diff --git a/.changeset/inbound-decode-parity.md b/.changeset/inbound-decode-parity.md new file mode 100644 index 000000000..05b88c8c0 --- /dev/null +++ b/.changeset/inbound-decode-parity.md @@ -0,0 +1,19 @@ +--- +--- + +fix(core): accept a zero-fraction float timestamp and refuse an over-long client frame + +Two inbound decode divergences from the capture, both measured against it: + +- An event's `created_at` or `kind` written in float spelling with a zero fractional + part (`1700000000.0`, `1.7e9`, `1.0`) is now the integer it denotes. JavaScript has + no float/int distinction after `JSON.parse`, so the capture accepts these and the + port refused them. A genuine fraction now answers `Expected integer, received float` + on `created_at` too, which is the capture's message (#960). +- `EVENT`, `AUTH` and `CLOSE` refuse a frame longer than their tuple with the capture's + `Array must contain at most 2 element(s)`, after the element checks, so a frame that + is also wrong-typed keeps the message it answered before (#961). `REQ`/`COUNT` keep + their filter bounds. + +The TypeScript tree is untouched: no `src/` or `test/` file changes, so the published +package has no user-visible change. diff --git a/crates/nostream-core/src/filter.rs b/crates/nostream-core/src/filter.rs index 5b3cbd902..507ad692a 100644 --- a/crates/nostream-core/src/filter.rs +++ b/crates/nostream-core/src/filter.rs @@ -8,6 +8,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use crate::event::{Event, Kind}; +use crate::validation::CREATED_AT_LIMIT; /// A subscription id (`src/@types/subscription.ts` `SubscriptionId`). pub type SubscriptionId = String; @@ -25,8 +26,16 @@ const MAX_SAFE_INTEGER: f64 = 9_007_199_254_740_991.0; /// `2^64`, the first value a `u64` cannot hold. `u64::MAX as f64` **is** this number — the /// largest `f64` below it is `u64::MAX - 2047` — so the comparison must be `>=`, not `>`: -/// with `>`, exactly `2^64` passed and `as u64` silently saturated it to `u64::MAX`. -const U64_RANGE_END: f64 = 18_446_744_073_709_551_616.0; +/// with `>`, exactly `2^64` passed and `as u64` silently saturated it to `u64::MAX`. It bounds +/// both refusals ADR 0002 decides: +/// `docs/rewrite/decisions/0002-core-crypto-and-negentropy.md:125-136`. +pub(crate) const U64_RANGE_END: f64 = 18_446_744_073_709_551_616.0; + +/// The refusal for a `kind` no `u64` can hold. The capture's `kindSchema` has no ceiling, so +/// the message is a representability divergence rather than a zod rule, decided in +/// `docs/rewrite/decisions/0002-core-crypto-and-negentropy.md:125-136`. One statement of the +/// wording, shared with the event decoder. +pub(crate) const KIND_DOES_NOT_FIT: &str = "kind value does not fit in 64 bits"; /// A Nostr REQ filter (`src/@types/subscription.ts` `SubscriptionFilter`). /// @@ -445,7 +454,7 @@ fn validate_kinds(path: &str, value: &Value, issues: &mut Vec) -> O match item.as_u64() { Some(kind) => kinds.push(Kind(kind)), None if number >= U64_RANGE_END => { - issue(&element, "kind value does not fit in 64 bits", issues); + issue(&element, KIND_DOES_NOT_FIT, issues); } None => kinds.push(Kind(number as u64)), } @@ -454,7 +463,8 @@ fn validate_kinds(path: &str, value: &Value, issues: &mut Vec) -> O Some(kinds) } -/// `createdAtSchema`: integers `>= 0` below `10^10` (`src/schemas/base-schema.ts:25-30`). +/// `createdAtSchema`: integers `>= 0` below `10^10` (`src/schemas/base-schema.ts:25-30`), the +/// window `validation::CREATED_AT_LIMIT` holds for this check and the decoder's `created_at`. fn validate_timestamp(path: &str, value: &Value, issues: &mut Vec) -> Option { let Some(number) = value.as_f64() else { issue(path, invalid_type("number", value), issues); @@ -474,7 +484,7 @@ fn validate_timestamp(path: &str, value: &Value, issues: &mut Vec) } let safe_integer = number.fract() == 0.0 && number.abs() <= MAX_SAFE_INTEGER; - let in_range = safe_integer && (0.0..1e10).contains(&number); + let in_range = safe_integer && (0.0..CREATED_AT_LIMIT as f64).contains(&number); if !in_range { issue(path, "Invalid timestamp", issues); @@ -484,7 +494,8 @@ fn validate_timestamp(path: &str, value: &Value, issues: &mut Vec) if valid { Some(number as i64) } else { None } } -/// `z.number().int().min(0)` for `limit`. +/// `z.number().int().min(0)` for `limit`, whose representability refusal is ADR 0002's +/// (`docs/rewrite/decisions/0002-core-crypto-and-negentropy.md:125-136`). fn validate_limit(path: &str, value: &Value, issues: &mut Vec) -> Option { let Some(number) = value.as_f64() else { issue(path, invalid_type("number", value), issues); diff --git a/crates/nostream-core/src/messages.rs b/crates/nostream-core/src/messages.rs index 6abd0b04f..1940c3e71 100644 --- a/crates/nostream-core/src/messages.rs +++ b/crates/nostream-core/src/messages.rs @@ -8,15 +8,34 @@ //! //! The positional decoder replaces the capture's `attemptValidation(messageSchema)` call //! (`src/adapters/web-socket-adapter.ts:199`). The zod messages it reports are reproduced -//! from `src/schemas/{message,event,filter}-schema.ts`; the one deliberate difference is the -//! detail after `invalid: ` for unparseable JSON, where V8's `JSON.parse` diagnosis is not -//! reproducible (ADR 0004 records the tolerance). +//! from `src/schemas/{message,event,filter}-schema.ts`. This list is the decoder's own +//! statement of where it differs from the capture, and which of those the tree decided. +//! +//! Decided, so reproducing it is the job: +//! +//! - The detail after `invalid: ` for unparseable JSON, where V8's `JSON.parse` diagnosis is +//! not reproducible (tolerance T1, ADR 0004 records it). +//! - A `kind` — an event's, or a filter's `kinds` element — or a filter's `limit` that no `u64` +//! can hold is refused rather than accepted: the capture admits it and this cannot represent +//! it, so the refusal is decided, not borrowed. `filter::U64_RANGE_END` bounds both values; +//! `filter::KIND_DOES_NOT_FIT` and `validate_limit` own the two wordings (ADR 0002, +//! `docs/rewrite/decisions/0002-core-crypto-and-negentropy.md:125-136`). +//! +//! Open, so the capture is authoritative and this decoder is the defect (`CLAUDE.md` +//! §"Porting discipline"): zod separates a branch that aborted from one that recorded an +//! issue, and this decoder reports the issue either way. Where a member of the capture's +//! `z.union` (`src/schemas/message-schema.ts:61`) aborts, the union reports one +//! `invalid_union` issue whose message is `Invalid input`, and this decoder reports the +//! field's own message instead; a frame that is both over-long and carries a dirty element +//! likewise answers the element's message here and the tuple's `Array must contain at most N +//! element(s)` there (`["CLOSE","","junk"]`). Tracked in #979. use serde::{Deserialize, Serialize}; -use serde_json::Value; +use serde_json::{Number, Value}; use crate::event::{Event, EventId, Pubkey}; -use crate::filter::{Filter, SubscriptionId, validate_filter}; +use crate::filter::{Filter, KIND_DOES_NOT_FIT, SubscriptionId, U64_RANGE_END, validate_filter}; +use crate::validation::CREATED_AT_LIMIT; /// The wire message kinds (`src/@types/messages.ts` `MessageType`). #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -337,8 +356,11 @@ pub fn decode(text: &str) -> Result { match verb { Some("EVENT") => { let event = items.get(1).ok_or_else(|| invalid("Invalid input"))?; + let event = validate_event_value(event)?; + + exactly(&items, 2)?; - Ok(ClientMessage::Event(validate_event_value(event)?)) + Ok(ClientMessage::Event(event)) } Some("REQ") => { let (subscription_id, filters) = decode_request(&items, "REQ")?; @@ -365,21 +387,43 @@ pub fn decode(text: &str) -> Result { return Err(invalid("String must contain at least 1 character(s)")); } + exactly(&items, 2)?; + Ok(ClientMessage::Unsubscribe(UnsubscribeMessage { subscription_id: subscription_id.clone(), })) } Some("AUTH") => { let event = items.get(1).ok_or_else(|| invalid("Invalid input"))?; + let event = validate_event_value(event)?; - Ok(ClientMessage::Auth(AuthMessage { - event: validate_event_value(event)?, - })) + exactly(&items, 2)?; + + Ok(ClientMessage::Auth(AuthMessage { event })) } _ => Err(invalid("Invalid input")), } } +/// A fixed-arity client verb refuses a frame longer than its tuple: `EVENT`, `AUTH` and +/// `CLOSE` are `z.tuple`s with no `.rest()` (`src/schemas/message-schema.ts:8,56,59`), and +/// zod reports the tuple's `too_big` issue, whose message is this one. The lower bound stays +/// each arm's own `Invalid input` — what an aborted union answers for a missing element — so +/// this never reports one. +/// +/// Call it after the arm's element checks: an element's own message answers before the length +/// one, which is what the capture's aborted union does, so `["CLOSE",42,"junk"]` stays +/// `Invalid input` while `["CLOSE","c1","junk"]` is refused here. +fn exactly(items: &[Value], expected: usize) -> Result<(), WireError> { + if items.len() > expected { + return Err(invalid(&format!( + "Array must contain at most {expected} element(s)" + ))); + } + + Ok(()) +} + /// `reqMessageSchema`/`countMessageSchema`: the id, then one to eleven filters. fn decode_request(items: &[Value], verb: &str) -> Result<(String, Vec), WireError> { let Value::String(subscription_id) = items.get(1).unwrap_or(&Value::Null) else { @@ -427,6 +471,31 @@ fn decode_request(items: &[Value], verb: &str) -> Result<(String, Vec), Ok((subscription_id.clone(), filters)) } +/// `z.number().int().min(0)` for an event's numeric fields (`src/schemas/base-schema.ts:17`), +/// in zod's order: a fractional value answers `Expected integer, received float`, then a +/// negative one answers `Number must be greater than or equal to 0`. A zero-fraction float +/// spelling is the integer it denotes — JavaScript has no float/int distinction after +/// `JSON.parse`, so the capture admits `1700000000.0` — and a value that is not a number +/// aborts the union with `Invalid input`. +/// +/// The value comes back as an `f64`; each caller bounds its own domain and casts only after +/// that check, which is what keeps the cast exact. +fn integer_value(number: &Number) -> Result { + let Some(value) = number.as_f64() else { + return Err(invalid("Invalid input")); + }; + + if value.fract() != 0.0 { + return Err(invalid("Expected integer, received float")); + } + + if value < 0.0 { + return Err(invalid("Number must be greater than or equal to 0")); + } + + Ok(value) +} + /// `eventSchema.parse` at the value level: the shape checks in field order, the `.strict()` /// rule, and the kind-7/17/10002/geohash refinements of `src/schemas/event-schema.ts`. /// @@ -473,13 +542,15 @@ pub fn validate_event_value(value: &Value) -> Result { }; let created_at = match object.get("created_at") { - Some(Value::Number(number)) => match number.as_i64() { - Some(value) if (0..10_000_000_000).contains(&value) => value, - Some(value) if value < 0 => { - return Err(invalid("Number must be greater than or equal to 0")); + Some(Value::Number(number)) => { + let value = integer_value(number)?; + + if value >= CREATED_AT_LIMIT as f64 { + return Err(invalid("Invalid timestamp")); } - _ => return Err(invalid("Invalid timestamp")), - }, + + value as i64 + } other => { return Err(invalid(&format!( "Expected number, received {}", @@ -491,13 +562,15 @@ pub fn validate_event_value(value: &Value) -> Result { let kind = match object.get("kind") { Some(Value::Number(number)) => match number.as_u64() { Some(value) => crate::event::Kind(value), - None if number.as_f64().is_some_and(|value| value.fract() != 0.0) => { - return Err(invalid("Expected integer, received float")); - } - None if number.as_i64().is_some_and(|value| value < 0) => { - return Err(invalid("Number must be greater than or equal to 0")); + None => { + let value = integer_value(number)?; + + if value >= U64_RANGE_END { + return Err(invalid(KIND_DOES_NOT_FIT)); + } + + crate::event::Kind(value as u64) } - None => return Err(invalid("Number must be greater than or equal to 0")), }, other => { return Err(invalid(&format!( diff --git a/crates/nostream-core/src/validation.rs b/crates/nostream-core/src/validation.rs index a1704e2a3..ce6d77a74 100644 --- a/crates/nostream-core/src/validation.rs +++ b/crates/nostream-core/src/validation.rs @@ -14,8 +14,10 @@ const GEOHASH_BASE32: &str = "0123456789bcdefghjkmnpqrstuvwxyz"; /// The largest integer JavaScript represents exactly (`Number.MAX_SAFE_INTEGER`). const MAX_SAFE_INTEGER: i64 = 9_007_199_254_740_991; -/// The exclusive upper bound of `createdAtSchema` (`Math.log10(value) < 10`). -const CREATED_AT_LIMIT: i64 = 10_000_000_000; +/// The exclusive upper bound of `createdAtSchema` (`Math.log10(value) < 10`), shared with the +/// decoder's `created_at` check and the filter's `since`/`until` validation, so the window is +/// written once. +pub(crate) const CREATED_AT_LIMIT: i64 = 10_000_000_000; /// Kind 7, `EventKinds.REACTION`. const REACTION_KIND: Kind = Kind(7); diff --git a/crates/nostream-core/tests/wire_codec.rs b/crates/nostream-core/tests/wire_codec.rs index 0ea156e06..040e9a04c 100644 --- a/crates/nostream-core/tests/wire_codec.rs +++ b/crates/nostream-core/tests/wire_codec.rs @@ -48,6 +48,21 @@ fn frame_document(event: &Event) -> String { serde_json::to_string(event).unwrap() } +/// The `event()` payload with `created_at` respelled. `serde_json` writes `1700000000.0` for +/// the value but never `1.7e9`, and the measured row is the spelling, so the number is +/// substituted as raw JSON text. +fn event_timestamp_spelled_as(spelling: &str) -> String { + frame_document(&event()).replace( + r#""created_at":1648339664"#, + &format!(r#""created_at":{spelling}"#), + ) +} + +/// The `event()` payload with `kind` respelled, for the same reason. +fn event_kind_spelled_as(spelling: &str) -> String { + frame_document(&event()).replace(r#""kind":1"#, &format!(r#""kind":{spelling}"#)) +} + #[test] fn notice_frame_is_a_two_element_array() { let frame = RelayMessage::Notice(NoticeMessage { @@ -483,3 +498,130 @@ fn signature_and_pubkey_hex_rules_are_reported() { let _: Pubkey = PUBKEY.parse().unwrap(); let _: Signature = SIG.parse().unwrap(); } + +#[test] +fn decode_accepts_an_integer_valued_float_timestamp() { + // JavaScript has no float/int distinction after `JSON.parse`, so the capture admits + // these spellings and holds the integer each denotes. + for (spelling, expected) in [ + ("1700000000.0", 1_700_000_000), + ("1.7e9", 1_700_000_000), + ("0.0", 0), + ] { + let document = event_timestamp_spelled_as(spelling); + let frame = format!(r#"["EVENT",{document}]"#); + let ClientMessage::Event(event) = decode(&frame).unwrap() else { + panic!("{frame} did not decode as EVENT"); + }; + + assert_eq!(event.created_at, expected, "{spelling}"); + } + + let document = event_timestamp_spelled_as("1700000000.0"); + let frame = format!(r#"["AUTH",{document}]"#); + let ClientMessage::Auth(auth) = decode(&frame).unwrap() else { + panic!("{frame} did not decode as AUTH"); + }; + + assert_eq!(auth.event.created_at, 1_700_000_000); +} + +#[test] +fn decode_refuses_a_fractional_timestamp_with_the_capture_message() { + for (spelling, detail) in [ + ("1700000000.5", "Expected integer, received float"), + ("-1.5", "Expected integer, received float"), + ("-1", "Number must be greater than or equal to 0"), + ("1672295751103", "Invalid timestamp"), + ("10000000000", "Invalid timestamp"), + ] { + let document = event_timestamp_spelled_as(spelling); + let frame = format!(r#"["EVENT",{document}]"#); + + assert_eq!( + decode(&frame).unwrap_err(), + WireError::Validation(detail.to_owned()), + "{spelling}" + ); + } +} + +#[test] +fn decode_accepts_an_integer_valued_float_kind() { + let document = event_kind_spelled_as("1.0"); + let frame = format!(r#"["EVENT",{document}]"#); + let ClientMessage::Event(event) = decode(&frame).unwrap() else { + panic!("{frame} did not decode as EVENT"); + }; + + assert_eq!(event.kind, Kind(1)); + + for (spelling, detail) in [ + ("1.5", "Expected integer, received float"), + ("-1", "Number must be greater than or equal to 0"), + // Representability, not a capture string: the capture accepts a kind at or above + // `2^64` and `Kind` is a `u64`, so the value is refused rather than saturated + // (ADR 0002). + ("18446744073709551616", "kind value does not fit in 64 bits"), + ] { + let document = event_kind_spelled_as(spelling); + let frame = format!(r#"["EVENT",{document}]"#); + + assert_eq!( + decode(&frame).unwrap_err(), + WireError::Validation(detail.to_owned()), + "{spelling}" + ); + } +} + +#[test] +fn decode_refuses_an_over_long_fixed_arity_frame() { + let document = frame_document(&event()); + + for frame in [ + format!(r#"["EVENT",{document},"junk"]"#), + format!(r#"["EVENT",{document},"junk","junk"]"#), + format!(r#"["AUTH",{document},"junk"]"#), + format!(r#"["AUTH",{document},"junk","junk"]"#), + r#"["CLOSE","c1","junk"]"#.to_owned(), + r#"["CLOSE","c1","junk","junk"]"#.to_owned(), + ] { + assert_eq!( + decode(&frame).unwrap_err(), + WireError::Validation("Array must contain at most 2 element(s)".to_owned()), + "{frame}" + ); + } + + // The lower bound is already right on all three verbs, and an element check still + // answers before the length check, which is what the capture's aborted union does. + for (frame, detail) in [ + (r#"["EVENT"]"#, "Invalid input"), + (r#"["CLOSE"]"#, "Invalid input"), + (r#"["AUTH"]"#, "Invalid input"), + ( + r#"["CLOSE",""]"#, + "String must contain at least 1 character(s)", + ), + (r#"["CLOSE",42,"junk"]"#, "Invalid input"), + ( + r#"["REQ","s1"]"#, + "REQ message must contain at least one filter", + ), + ( + r#"["COUNT","c1"]"#, + "COUNT message must contain at least one filter", + ), + ( + r#"["REQ","s1",{"kinds":[1]},"junk"]"#, + "Expected object, received string", + ), + ] { + assert_eq!( + decode(frame).unwrap_err(), + WireError::Validation(detail.to_owned()), + "{frame}" + ); + } +}