From 80fbb17b9bbe1ea6cfeb6d5c82cf9a31b655ee4a Mon Sep 17 00:00:00 2001 From: Sven Slootweg Date: Sun, 19 Jun 2022 18:57:35 +0200 Subject: [PATCH] WIP, make strict-mode response validation optional --- src/packages/is-device-event/index.js | 8 +- src/packages/is-event/index.js | 16 +- src/packages/is-messages-response/index.js | 19 +- src/packages/is-presence-event/index.js | 8 +- src/packages/is-room-event/index.js | 102 +++++----- src/packages/is-state-event/index.js | 15 +- src/packages/is-stripped-event/index.js | 10 +- src/packages/is-sync-response/index.js | 177 +++++++++--------- src/packages/is-timeline-event/index.js | 11 +- src/packages/parse-messages-response/index.js | 4 +- src/packages/parse-sync-response/index.js | 4 +- src/packages/stream-events/test.js | 14 +- 12 files changed, 211 insertions(+), 177 deletions(-) diff --git a/src/packages/is-device-event/index.js b/src/packages/is-device-event/index.js index 2a43619..7cf3a0f 100644 --- a/src/packages/is-device-event/index.js +++ b/src/packages/is-device-event/index.js @@ -5,7 +5,9 @@ const required = require("@validatem/required"); const isEvent = require("../is-event"); const isMatrixID = require("../is-matrix-id"); -module.exports = { - ... isEvent, - sender: [ required, isMatrixID ] +module.exports = function isDeviceEvent(strict = false, extraFields = {}) { + return isEvent(strict, { + sender: [ required, isMatrixID ], + ... extraFields + }); }; diff --git a/src/packages/is-event/index.js b/src/packages/is-event/index.js index 7ff9669..cc34408 100644 --- a/src/packages/is-event/index.js +++ b/src/packages/is-event/index.js @@ -3,8 +3,18 @@ const required = require("@validatem/required"); const isString = require("@validatem/is-string"); const isPlainObject = require("@validatem/is-plain-object"); +const allowExtraProperties = require("@validatem/allow-extra-properties"); -module.exports = { - type: [ required, isString ], - content: [ required, isPlainObject ] +module.exports = function isEvent(strict = false, extraFields = {}) { + let fields = { + type: [ required, isString ], + content: [ required, isPlainObject ], + ... extraFields + }; + + if (strict) { + return fields; + } else { + return allowExtraProperties(fields); + } }; diff --git a/src/packages/is-messages-response/index.js b/src/packages/is-messages-response/index.js index 885f881..6551b14 100644 --- a/src/packages/is-messages-response/index.js +++ b/src/packages/is-messages-response/index.js @@ -8,12 +8,15 @@ const isTimelineEvent = require("../is-timeline-event"); const isStateEvent = require("../is-state-event"); const optionalArray = require("../optional-array"); -let isTimelineList = arrayOf([ required, isTimelineEvent ]); -let isStateList = arrayOf([ required, isStateEvent ]); - -module.exports = { - start: [ required, isString ], - end: [ isString ], - chunk: optionalArray(isTimelineList), - state: optionalArray(isStateList) +module.exports = function isMessagesResponse(strict = false, extraFields = {}) { + // FIXME: Expand validation rules affected by 'strict' setting? eg. allowing extra properties in the messages response itself + let isTimelineList = arrayOf([ required, isTimelineEvent(strict, extraFields) ]); + let isStateList = arrayOf([ required, isStateEvent(strict, extraFields) ]); + + return { + start: [ required, isString ], + end: [ isString ], + chunk: optionalArray(isTimelineList), + state: optionalArray(isStateList) + }; }; diff --git a/src/packages/is-presence-event/index.js b/src/packages/is-presence-event/index.js index 6d91c67..a7245e7 100644 --- a/src/packages/is-presence-event/index.js +++ b/src/packages/is-presence-event/index.js @@ -6,7 +6,9 @@ const isEvent = require("../is-event"); const isMatrixID = require("../is-matrix-id"); // NOTE: Unspecced, see https://github.com/matrix-org/matrix-doc/issues/2680 - this would normally just be `isEvent` as per the spec -module.exports = { - ... isEvent, - sender: [ required, isMatrixID ] +module.exports = function isPresenceEvent(strict = false, extraFields = {}) { + return isEvent(strict, { + sender: [ required, isMatrixID ], + ... extraFields + }); }; diff --git a/src/packages/is-room-event/index.js b/src/packages/is-room-event/index.js index ae83292..6d53054 100644 --- a/src/packages/is-room-event/index.js +++ b/src/packages/is-room-event/index.js @@ -12,57 +12,59 @@ const isRoomID = require("../is-room-id"); const optionalObject = require("../optional-object"); const isPaginatedChunkOf = require("../is-paginated-chunk-of"); -module.exports = { - ... isEvent, - event_id: [ required, isString ], - sender: [ required, isMatrixID ], - origin_server_ts: [ required, isInteger ], - // In spec, but missing from Room Event format: https://github.com/matrix-org/matrix-doc/issues/2684 - redacts: isEventID, // FIXME: Make required when redaction-type event - // Spec omission: https://github.com/matrix-org/matrix-doc/issues/2685 - age_ts: isInteger, - room_id: [ isRoomID ], // FIXME: Not present on /sync, but will need to be required-checked for event validation elsewhere - // Synapse bug: https://github.com/matrix-org/synapse/issues/7925 - age: isInteger, - // Synapse bug: https://github.com/matrix-org/synapse/issues/7924 - user_id: isMatrixID, - // Synapse bug: https://github.com/matrix-org/synapse/issues/7925#issuecomment-662089208 - replaces_state: isEventID, - // Synapse bug: https://github.com/matrix-org/synapse/issues/7925#issuecomment-663247760 - redacted_because: isPlainObject, - // Obsolete field originating from a now-defunct Synapse fork running on ponies.im - origin_server_ipts: [ isInteger ], - unsigned: optionalObject({ +module.exports = function isRoomEvent(strict = false, extraFields = {}) { + return isEvent(strict, { + event_id: [ required, isString ], + sender: [ required, isMatrixID ], + origin_server_ts: [ required, isInteger ], + // In spec, but missing from Room Event format: https://github.com/matrix-org/matrix-doc/issues/2684 + redacts: isEventID, // FIXME: Make required when redaction-type event + // Spec omission: https://github.com/matrix-org/matrix-doc/issues/2685 + age_ts: isInteger, + room_id: [ isRoomID ], // FIXME: Not present on /sync, but will need to be required-checked for event validation elsewhere + // Synapse bug: https://github.com/matrix-org/synapse/issues/7925 age: isInteger, - transaction_id: isString, - redacted_because: isPlainObject, // FIXME: Cannot do recursion with isRoomEvent (/isStateEvent) -- fixable via `dynamic` wrapper, so that the rules are only generated *after* the validator has finished being declared? - // Spec omission: https://github.com/matrix-org/matrix-doc/issues/2690 - redacted_by: isEventID, - // Spec omission: https://github.com/matrix-org/matrix-doc/issues/1167 + // Synapse bug: https://github.com/matrix-org/synapse/issues/7924 + user_id: isMatrixID, + // Synapse bug: https://github.com/matrix-org/synapse/issues/7925#issuecomment-662089208 replaces_state: isEventID, - // Spec omission and/or Synapse bug: https://github.com/matrix-org/matrix-doc/issues/877 - prev_content: isPlainObject, - // Spec omission: https://github.com/matrix-org/matrix-doc/issues/684 - prev_sender: isMatrixID, - // MSC 1849, not merged yet: https://github.com/matrix-org/matrix-doc/blob/matthew/msc1849/proposals/1849-aggregations.md - "m.relations": { - "m.annotation": isPaginatedChunkOf({ - type: [ required, isString ], - key: [ required, isString ], - // Should be required according to MSC, but currently missing in Synapse: https://github.com/matrix-org/synapse/issues/7941#issuecomment-663238820 - origin_server_ts: [ /* required, */ isInteger ], - count: [ required, isInteger ] - }), - "m.reference": isPaginatedChunkOf({ - // Should be required according to MSC, but currently missing in Synapse: https://github.com/matrix-org/synapse/issues/7941 - type: [ /* required, */ isString ], - event_id: [ required, isEventID ] - }), - "m.replace": { - event_id: [ required, isEventID ], - origin_server_ts: [ required, isInteger ], - sender: [ required, isMatrixID ] + // Synapse bug: https://github.com/matrix-org/synapse/issues/7925#issuecomment-663247760 + redacted_because: isPlainObject, + // Obsolete field originating from a now-defunct Synapse fork running on ponies.im + origin_server_ipts: [ isInteger ], + unsigned: optionalObject({ + age: isInteger, + transaction_id: isString, + redacted_because: isPlainObject, // FIXME: Cannot do recursion with isRoomEvent (/isStateEvent) -- fixable via `dynamic` wrapper, so that the rules are only generated *after* the validator has finished being declared? + // Spec omission: https://github.com/matrix-org/matrix-doc/issues/2690 + redacted_by: isEventID, + // Spec omission: https://github.com/matrix-org/matrix-doc/issues/1167 + replaces_state: isEventID, + // Spec omission and/or Synapse bug: https://github.com/matrix-org/matrix-doc/issues/877 + prev_content: isPlainObject, + // Spec omission: https://github.com/matrix-org/matrix-doc/issues/684 + prev_sender: isMatrixID, + // MSC 1849, not merged yet: https://github.com/matrix-org/matrix-doc/blob/matthew/msc1849/proposals/1849-aggregations.md + "m.relations": { + "m.annotation": isPaginatedChunkOf({ + type: [ required, isString ], + key: [ required, isString ], + // Should be required according to MSC, but currently missing in Synapse: https://github.com/matrix-org/synapse/issues/7941#issuecomment-663238820 + origin_server_ts: [ /* required, */ isInteger ], + count: [ required, isInteger ] + }), + "m.reference": isPaginatedChunkOf({ + // Should be required according to MSC, but currently missing in Synapse: https://github.com/matrix-org/synapse/issues/7941 + type: [ /* required, */ isString ], + event_id: [ required, isEventID ] + }), + "m.replace": { + event_id: [ required, isEventID ], + origin_server_ts: [ required, isInteger ], + sender: [ required, isMatrixID ] + } } - } - }) + }), + ... extraFields + }); }; diff --git a/src/packages/is-state-event/index.js b/src/packages/is-state-event/index.js index 442e634..14662f0 100644 --- a/src/packages/is-state-event/index.js +++ b/src/packages/is-state-event/index.js @@ -3,14 +3,15 @@ const required = require("@validatem/required"); const isString = require("@validatem/is-string"); const isPlainObject = require("@validatem/is-plain-object"); -const isInteger = require("@validatem/is-integer"); const isRoomEvent = require("../is-room-event"); -module.exports = { - ... isRoomEvent, - state_key: [ required, isString ], - prev_content: isPlainObject, - // Spec violation by Synapse: https://github.com/matrix-org/synapse/issues/6226 - membership: [ isString ] +module.exports = function isStateEvent(strict = false, extraFields = {}) { + return isRoomEvent(strict, { + state_key: [ required, isString ], + prev_content: isPlainObject, + // Spec violation by Synapse: https://github.com/matrix-org/synapse/issues/6226 + membership: [ isString ], + ... extraFields + }); }; diff --git a/src/packages/is-stripped-event/index.js b/src/packages/is-stripped-event/index.js index 82f3a2d..922b0d4 100644 --- a/src/packages/is-stripped-event/index.js +++ b/src/packages/is-stripped-event/index.js @@ -6,8 +6,10 @@ const isString = require("@validatem/is-string"); const isEvent = require("../is-event"); const isMatrixID = require("../is-matrix-id"); -module.exports = { - ... isEvent, - state_key: [ required, isString ], - sender: [ required, isMatrixID ] +module.exports = function isDeviceEvent(strict = false, extraFields = {}) { + return isEvent(strict, { + state_key: [ required, isString ], + sender: [ required, isMatrixID ], + ... extraFields + }); }; diff --git a/src/packages/is-sync-response/index.js b/src/packages/is-sync-response/index.js index 7d243ba..c954eac 100644 --- a/src/packages/is-sync-response/index.js +++ b/src/packages/is-sync-response/index.js @@ -21,92 +21,95 @@ const isMatrixID = require("../is-matrix-id"); const optionalObject = require("../optional-object"); const optionalArray = require("../optional-array"); -let isStateList = arrayOf([ required, isStateEvent ]); -let isEventList = arrayOf([ required, isEvent ]); -let isPresenceEventList = arrayOf([ required, isPresenceEvent ]); -let isStrippedEventList = arrayOf([ required, isStrippedEvent ]); -let isDeviceEventList = arrayOf([ required, isDeviceEvent ]); -let isTimelineList = arrayOf([ required, isTimelineEvent ]); - -module.exports = { - next_batch: [ required, isString ], - // FIXME: also optionalObject for `rooms` - rooms: { - join: optionalObject(anyProperty({ - key: [ required, isRoomID ], - value: { - summary: optionalObject({ - "m.heroes": arrayOf([ required, isString ]), - "m.joined_member_count": isInteger, - "m.invited_member_count": isInteger, - }), - // NOTE: Despite what the spec currently says, state.events *can* contain membership events when the timeline isn't limited, when lazy-loading is enabled - state: optionalObject({ - events: optionalArray(isStateList) - }), - timeline: optionalObject({ - events: optionalArray(isTimelineList), - limited: isBoolean, - prev_batch: isString - }), - ephemeral: optionalObject({ - events: optionalArray(isEventList) - }), - account_data: optionalObject({ - events: optionalArray(isEventList) - }), - unread_notifications: { - highlight_count: isInteger, - notification_count: isInteger - }, - // FIXME: expose the below - "org.matrix.msc2654.unread_count": [ isInteger ], // NOTE: https://github.com/matrix-org/matrix-doc/pull/2654 - } - })), - invite: optionalObject(anyProperty({ - key: [ required, isRoomID ], - value: { - // NOTE: This state needs to be maintained separately from known room state (see spec). FIXME: Represent this in the event list output? - invite_state: optionalObject({ - events: optionalArray(isStrippedEventList) - }) - } +module.exports = function isSyncResponse(strict = false) { + // FIXME: Expand validation rules affected by 'strict' setting? eg. allowing extra properties in the sync response itself + let isStateList = arrayOf([ required, isStateEvent(strict) ]); + let isEventList = arrayOf([ required, isEvent(strict) ]); + let isPresenceEventList = arrayOf([ required, isPresenceEvent(strict) ]); + let isStrippedEventList = arrayOf([ required, isStrippedEvent(strict) ]); + let isDeviceEventList = arrayOf([ required, isDeviceEvent(strict) ]); + let isTimelineList = arrayOf([ required, isTimelineEvent(strict) ]); + + return { + next_batch: [ required, isString ], + // FIXME: also optionalObject for `rooms` + rooms: { + join: optionalObject(anyProperty({ + key: [ required, isRoomID ], + value: { + summary: optionalObject({ + "m.heroes": arrayOf([ required, isString ]), + "m.joined_member_count": isInteger, + "m.invited_member_count": isInteger, + }), + // NOTE: Despite what the spec currently says, state.events *can* contain membership events when the timeline isn't limited, when lazy-loading is enabled + state: optionalObject({ + events: optionalArray(isStateList) + }), + timeline: optionalObject({ + events: optionalArray(isTimelineList), + limited: isBoolean, + prev_batch: isString + }), + ephemeral: optionalObject({ + events: optionalArray(isEventList) + }), + account_data: optionalObject({ + events: optionalArray(isEventList) + }), + unread_notifications: { + highlight_count: isInteger, + notification_count: isInteger + }, + // FIXME: expose the below + "org.matrix.msc2654.unread_count": [ isInteger ], // NOTE: https://github.com/matrix-org/matrix-doc/pull/2654 + } + })), + invite: optionalObject(anyProperty({ + key: [ required, isRoomID ], + value: { + // NOTE: This state needs to be maintained separately from known room state (see spec). FIXME: Represent this in the event list output? + invite_state: optionalObject({ + events: optionalArray(isStrippedEventList) + }) + } + })), + leave: optionalObject(anyProperty({ + key: [ required, isRoomID ], + value: { + state: optionalObject({ + events: optionalArray(isStateList) + }), + timeline: optionalObject({ + events: optionalArray(isTimelineList), + limited: isBoolean, + prev_batch: isString + }), + account_data: optionalObject({ + events: optionalArray(isEventList) + }), + } + })) + }, + presence: optionalObject({ + events: optionalArray(isPresenceEventList) + }), + account_data: optionalObject({ + events: optionalArray(isEventList) + }), + to_device: optionalObject({ + events: optionalArray(isDeviceEventList) + }), + device_lists: optionalObject({ + changed: arrayOf([ required, isMatrixID ]), + left: arrayOf([ required, isMatrixID ]) + }), + device_one_time_keys_count: optionalObject(anyProperty({ + key: [ required, isString ], // algorithm name + value: [ required, isInteger ] // key count })), - leave: optionalObject(anyProperty({ - key: [ required, isRoomID ], - value: { - state: optionalObject({ - events: optionalArray(isStateList) - }), - timeline: optionalObject({ - events: optionalArray(isTimelineList), - limited: isBoolean, - prev_batch: isString - }), - account_data: optionalObject({ - events: optionalArray(isEventList) - }), - } - })) - }, - presence: optionalObject({ - events: optionalArray(isPresenceEventList) - }), - account_data: optionalObject({ - events: optionalArray(isEventList) - }), - to_device: optionalObject({ - events: optionalArray(isDeviceEventList) - }), - device_lists: optionalObject({ - changed: arrayOf([ required, isMatrixID ]), - left: arrayOf([ required, isMatrixID ]) - }), - device_one_time_keys_count: optionalObject(anyProperty({ - key: [ required, isString ], // algorithm name - value: [ required, isInteger ] // key count - })), - groups: anything, // NOTE: Non-standard - // TODO: Validate algorithm names below? - "org.matrix.msc2732.device_unused_fallback_key_types": optionalArray(arrayOf([ required, isString ])), // NOTE: https://github.com/matrix-org/matrix-doc/pull/2732 + groups: anything, // NOTE: Non-standard + // TODO: Validate algorithm names below? + "org.matrix.msc2732.device_unused_fallback_key_types": optionalArray(arrayOf([ required, isString ])), // NOTE: https://github.com/matrix-org/matrix-doc/pull/2732 + }; }; diff --git a/src/packages/is-timeline-event/index.js b/src/packages/is-timeline-event/index.js index 78f7456..231f4d7 100644 --- a/src/packages/is-timeline-event/index.js +++ b/src/packages/is-timeline-event/index.js @@ -6,7 +6,10 @@ const isRoomEvent = require("../is-room-event"); const isStateEvent = require("../is-state-event"); // State events are a more specific version of room events: https://github.com/matrix-org/matrix-doc/issues/2681 -module.exports = either([ - [ isRoomEvent ], - [ isStateEvent ] -]); +module.exports = function (strict = false, extraFields = {}) { + return either([ + [ isRoomEvent(strict, extraFields) ], + [ isStateEvent(strict, extraFields) ] + ]); +}; + diff --git a/src/packages/parse-messages-response/index.js b/src/packages/parse-messages-response/index.js index 0a71568..4966730 100644 --- a/src/packages/parse-messages-response/index.js +++ b/src/packages/parse-messages-response/index.js @@ -7,10 +7,10 @@ const itemDeduplicator = require("../item-deduplicator"); const isMessagesResponse = require("../is-messages-response"); -module.exports = function parseMessagesResponse(_response) { +module.exports = function parseMessagesResponse(_response, strict = false) { // FIXME: Figure out a way to soft-fail, and turn the validation error into a warning event let [ response ] = validateArguments(arguments, { - response: [ required, isMessagesResponse ] + response: [ required, isMessagesResponse(strict) ] }); let deduplicateEvent = itemDeduplicator((event) => event.event_id); diff --git a/src/packages/parse-sync-response/index.js b/src/packages/parse-sync-response/index.js index dbcf63d..083c4ef 100644 --- a/src/packages/parse-sync-response/index.js +++ b/src/packages/parse-sync-response/index.js @@ -31,10 +31,10 @@ function maybeMapObject(object, mappingFunction) { } -module.exports = function syncResponseToEvents(_syncResponse) { +module.exports = function parseSyncResponse(_syncResponse, strict = false) { // require("fs").writeFileSync("private/dump.json", JSON.stringify(_syncResponse)); let [ syncResponse ] = validateArguments(arguments, { - syncResponseBody: [ required, isSyncResponse ], // TODO: Validate and normalize the response body, including setting defaults, and allowing extra properties + syncResponseBody: [ required, isSyncResponse(strict) ], // TODO: Validate and normalize the response body, including setting defaults, and allowing extra properties }); // We keep an event ID -> event body mapping, to ensure that the same event in different places in the response maps to the same in-memory object in our resulting event list; this is useful both to save memory, and to make equality-checking operations work diff --git a/src/packages/stream-events/test.js b/src/packages/stream-events/test.js index 43fd365..8e3f9c5 100644 --- a/src/packages/stream-events/test.js +++ b/src/packages/stream-events/test.js @@ -7,11 +7,13 @@ const simpleSink = require("@promistream/simple-sink"); const filter = require("@promistream/filter"); const mapEvent = require("../map-event"); +const createSession = require("../../../../create-session"); -let session = { - homeserver: "https://pixie.town", - accessToken: require("../../../private/access-token") -}; + +// let session = { +// homeserver: "https://pixie.town", +// accessToken: require("../../../private/access-token") +// }; // let since = "s14011802_60514432_167714_6077759_745604_385_21833_2919406_36"; // let since = "s14886247_62932075_392219_6368720_764854_404_25467_3352546_36"; @@ -19,6 +21,10 @@ let session = { let since = "s15508519_65086739_41448_6548141_783165_404_27201_3802507_36"; return Promise.try(() => { + return createSession("https://pixie.town", { + accessToken: require("../../../private/access-token") + }); +}).then((session) => { return pipe([ mmStreamEvents(session, since, { initialLimit: 50, eventMapper: mapEvent }), filter((event) => event.type === "roomTimelineEvent"),