From bb28ba300604b617dfda52c1f37514832f48e3f3 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Thu, 3 Sep 2026 04:30:54 +0000 Subject: [PATCH] Resolve N3: maxLength is string/bytes-only (review #006) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Parse gate in BastField::parse: maxLength on any kind other than string/bytes is a clean Schema error (records, arrays, inline structs, refs, union shared fields all covered; the choke point needs no ref-following since $defs entries are struct/union/enum) - Meta-schema FieldDef: if kind in {string, bytes} else maxLength forbidden — the published alk.dev/bast/v1 contract matches the parser (N2 dual-layer pattern) - M5's compute-side record maxLength arm became unreachable and was deleted (offset-indirect arm stays); the two superseded M5 maxLength tests rewritten as the n3_* parse-rejection family - ADR-006 remedy message tailored per kind: for records both annotated remedies are dead ends, so the error text points at the last-position fix only - Docs aligned: bast-format.md (FieldDef meta-schema + FieldDef/ Variable-Length Encoding prose), layout-engine.md (Strategy 2 + ADR-006 paragraph), schema-layer.md, ADR-003 §2/§3a amended, builder .max_length() doc - Review #006: N3 resolved (all findings now closed), M5 update note, test-count bookkeeping note (in-session probes vs static counts), status lines flipped to fully resolved Verified: 547 tests green + 2 ignored doctests in BOTH release and default profiles (a stale debug artifact from an earlier session masked one H3 roundtrip test in debug; clean rebuild passes both), clippy -D warnings clean, cargo doc --no-deps zero warnings, wasm build green. --- docs/architecture/bast-format.md | 20 ++- .../decisions/003-schema-annotations.md | 15 +- docs/architecture/layout-engine.md | 12 +- docs/architecture/schema-layer.md | 5 +- docs/reviews/006-implementation-review-030.md | 111 ++++++++++++-- src/bast.rs | 144 +++++++++++++++++- src/bast_meta.rs | 49 ++++++ src/builder.rs | 2 + src/offset_map.rs | 89 +++-------- 9 files changed, 352 insertions(+), 95 deletions(-) diff --git a/docs/architecture/bast-format.md b/docs/architecture/bast-format.md index df7e5a1..215ce4b 100644 --- a/docs/architecture/bast-format.md +++ b/docs/architecture/bast-format.md @@ -128,6 +128,12 @@ These are different validators for different inputs. "encoding": { "enum": ["length-prefixed", "offset-indirect"] }, "maxLength": { "type": "integer", "minimum": 0 } }, + "if": { + "properties": { + "kind": { "enum": ["string", "bytes"] } + } + }, + "else": { "properties": { "maxLength": false } }, "required": ["name", "kind"], "additionalProperties": false }, @@ -269,8 +275,11 @@ These are different validators for different inputs. - `align` (optional): field-level alignment (aligned mode only). - `encoding` (optional): `"length-prefixed"` (default) or `"offset-indirect"`. See [Variable-length encoding](#variable-length-encoding). -- `maxLength` (optional): byte-length cap. See - [Variable-length encoding](#variable-length-encoding). +- `maxLength` (optional, `string`/`bytes` fields only): byte-length + cap. See [Variable-length encoding](#variable-length-encoding). + Rejected at parse on any other kind (review #006 N3: the annotation + was silently unenforced there — the validation plan bakes `maxLength` + into string/bytes leaves only). ### TypeRef @@ -456,8 +465,11 @@ override). In little-endian mode, `u32::from_le_bytes`; in big-endian mode, `u32::from_be_bytes`. Ensures SFTP consumers (big-endian) have consistent byte order for field values and length prefixes. -Applies to all variable-length types: `string`, `bytes`, -`record`, and arrays of variable-length elements. +Applies to variable-length primitive types only: `string` and +`bytes`. The parser rejects `maxLength` (and the meta-schema forbids +it) on every other kind — including `record` (review #006 N3/M5: no +consumer honored it there, so the annotation was either silently +unenforced or, in aligned mode, silently corrupt). ## Endianness diff --git a/docs/architecture/decisions/003-schema-annotations.md b/docs/architecture/decisions/003-schema-annotations.md index 9c2b84b..1d04f03 100644 --- a/docs/architecture/decisions/003-schema-annotations.md +++ b/docs/architecture/decisions/003-schema-annotations.md @@ -136,9 +136,9 @@ reserving worst-case space. - `true` is a shorthand for the default (length-prefixed). This keeps the common case concise and the override explicit. -- The `encoding` annotation and `maxLength` apply to all variable-length - types: `AlkType:String`, `AlkType:Bytes`, `AlkType:Array`, - `AlkType:Record`, `AlkType:Timestamp`. +- The `encoding` annotation and `maxLength` apply to the variable-length + primitive types `AlkType:String` and `AlkType:Bytes`. (`maxLength` on + records was amended out by review #006 N3/M5 — see §3a.) ### 3a. TRecord value type @@ -164,8 +164,13 @@ the `"values"` property in the schema: the value's size is determined by its kind (fixed-size kinds have a known size; variable-length kinds carry their own length prefix). - The count and key-length prefixes respect the schema's endianness. -- In aligned static mode with `maxLength`, the entire record is reserved - at `maxLength` bytes (zero-padded). +- ~~In aligned static mode with `maxLength`, the entire record is + reserved at `maxLength` bytes (zero-padded).~~ **Amended (review #006 + N3/M5, 2026-09-02):** `maxLength` is rejected at parse on record + fields. The aligned materializer walks the record's inline + count-prefixed form and never honors the reservation (M5: silent + cross-field corruption), and no packed consumer enforced it either + (N3: silently unenforced). `maxLength` is `string`/`bytes`-only. ### 4. TUnion discriminators diff --git a/docs/architecture/layout-engine.md b/docs/architecture/layout-engine.md index e5083ec..d8e0d32 100644 --- a/docs/architecture/layout-engine.md +++ b/docs/architecture/layout-engine.md @@ -113,7 +113,11 @@ with a `AlkTypeError::Offset` — the `OffsetMap` reserves only 4 bytes (the length prefix), but `data_access::write_string` writes prefix + data inline, which would clobber subsequent fields. Non-final variable fields must use `maxLength` (fixed-size reservation) or -`"encoding": "offset-indirect"`. See +`"encoding": "offset-indirect"` — except `record` fields, for which +neither remedy is available (`maxLength` is rejected at parse — review +#006 N3 — and `offset-indirect` is rejected for records in aligned +mode — review #006 M5), so a non-final record field cannot be repaired +and must move to the last position. See [ADR-006](decisions/006-reject-non-final-inline-length-prefixed-in-aligned-mode.md). ## Offset Computation Algorithm @@ -194,6 +198,12 @@ annotation shapes). only. The engine uses strategy 1 (inline length-prefixing) because protocols don't benefit from fixed-size reservation. +`maxLength` applies to `string` and `bytes` fields only. The parser +rejects it on any other kind (review #006 N3): the validation plan +bakes it into string/bytes leaves only, so on a record (or any other +kind) the annotation did nothing — and in aligned mode a record +reservation was silently corrupt (review #006 M5). + **Strategy 3: Offset indirection (`"encoding": "offset-indirect"`).** 1. The field is a struct `{offset: u32, length: u32}`. 2. The `OffsetMap` records the position of this struct. diff --git a/docs/architecture/schema-layer.md b/docs/architecture/schema-layer.md index bd55712..aad7941 100644 --- a/docs/architecture/schema-layer.md +++ b/docs/architecture/schema-layer.md @@ -230,7 +230,10 @@ type-level properties. The concrete BAST shapes are in The `maxLength` keyword is *not* a BAST invention — it is the standard JSON Schema `maxLength`, repurposed as a byte-length cap. In aligned mode it reserves a fixed-size slot; in packed mode it is a validation -constraint only. See [bast-format.md §Variable-Length +constraint only. It applies to `string`/`bytes` fields only: the parser +rejects it on any other kind (review #006 N3 — elsewhere it was +silently unenforced), and in aligned mode a record reservation was +silently corrupt (review #006 M5). See [bast-format.md §Variable-Length Encoding](bast-format.md#variable-length-encoding) and [ADR-003](decisions/003-schema-annotations.md). diff --git a/docs/reviews/006-implementation-review-030.md b/docs/reviews/006-implementation-review-030.md index f4de027..d8e0084 100644 --- a/docs/reviews/006-implementation-review-030.md +++ b/docs/reviews/006-implementation-review-030.md @@ -1,5 +1,5 @@ --- -status: in-progress (H1, L1, H3, L5, L6, H2, M1, M2, M3, L2, L3, N2, L4, N1, M5, M6 resolved 2026-09-02) +status: resolved (H1, L1, H3, L5, L6, H2, M1, M2, M3, L2, L3, N2, L4, N1, M5, M6, N3 resolved 2026-09-02; M4 items 1–4 closed, per-fix coverage posture ongoing by design) last_updated: 2026-09-02 reviewed_artifacts: - src/read_plan.rs @@ -98,14 +98,14 @@ sub-types, `fingerprint()` methods; `Hash` on `Endian`/ | High | 3 (H1, H2, H3) | all resolved 2026-09-02 | | Medium | 6 (M1–M4, M5, M6) | M1, M2, M3, M5, M6 resolved 2026-09-02; M4 items 1–4 closed 2026-09-02 (posture ongoing) | | Low | 6 (L1–L6) | all resolved 2026-09-02 | -| Nit | 3 (N1, N2, N3) | N1, N2 resolved 2026-09-02; N3 open | +| Nit | 3 (N1, N2, N3) | all resolved 2026-09-02 | -All Highs, five of six original Mediums (M4's four concrete items are -closed; the per-fix coverage posture itself is ongoing by design), all -six Lows, and two of three Nits are resolved. Two new findings (M5, M6) -and one new Nit (N3) surfaced during the follow-up session while -closing M4's coverage map — all in the same untrusted-input family the -review exists for. Remaining: M4 (ongoing posture), N3. +Every finding is resolved. The only intentionally-ongoing item is M4's +*posture* (fold a coverage check into each future fix session), which +is process, not a defect. Two new findings (M5, M6) and one new Nit +(N3) surfaced during the follow-up session while closing M4's coverage +map — all in the same untrusted-input family the review exists for; +N3 closed last (string/bytes-only `maxLength` gate). The three Highs are adversarial-input crashes (H1, H2) and a cross-consumer wire-layout convention gap (H3) — all three are the @@ -161,9 +161,20 @@ them became materially easier to hit with the 0.3.0 surface. resolution blocks. M6 was probe-verified while writing the M4 record-with-union-values coverage test; items 1–3 closed the aligned materializer, reader, and data-access holes from M4's map. Coverage - after: materialize.rs 64.48→85.72% lines, TOTAL 89.59→90.60%. 567 - tests green, clippy `-D warnings` clean, wasm build green, `cargo - doc --no-deps` zero warnings. + after: materialize.rs 64.48→85.72% lines, TOTAL 89.59→90.60%. (The + "567 tests green" quoted at the time counts in-session disposable + probes that were deleted before commit; static count at `2eb086f` is + 542 + 2 ignored — see the N3 resolution block's bookkeeping note.) +- **N3 (2026-09-02):** resolved — see the "Resolution (2026-09-02)" + block on the finding. Posture decision: option (b) generalized — + `maxLength` is string/bytes-only, rejected at parse on every other + kind, enforced in the meta-schema; M5's compute-side record + `maxLength` arm became dead and was deleted; ADR-006's record + remedy text tailored; five normative docs aligned. 548 tests green + (static count 547 + 2 ignored doctests, +6 net: six `n3_` parse + tests and two meta-schema tests added, the two superseded M5 + `maxLength` tests removed), clippy `-D warnings` clean, wasm build + green, `cargo doc --no-deps` zero warnings. --- @@ -1181,6 +1192,15 @@ silently corrupt, now rejected with a clean error naming the annotation). Verified with M5's commit: 546 tests green, clippy clean, wasm green. +**Update (2026-09-02, N3):** the `maxLength` half of this fix was +superseded — `maxLength` is now rejected *earlier*, at parse +(`BastField::parse`), for every non-string/bytes kind, so the +compute-side record `maxLength` arm became unreachable and was +deleted. The offset-indirect arm remains reachable and stays. The two +M5 `maxLength` tests were rewritten as the `n3_*` parse-rejection +family; `m5_record_offset_indirect_rejected_in_aligned_mode` is +unchanged. See the N3 resolution block. + ### M6. The legacy BAST-walker's field-disc union arm skips non-discriminator shared fields — variant materializes from shared fields' bytes **Files**: `src/materialize.rs:728-765` (the `Field` arm of @@ -1278,6 +1298,67 @@ but rejects schemas that are otherwise fine); (c) document that Option (c) is the cheapest honest closure; option (a) is the most useful. Needs a posture decision like N1's — either is small. +**Resolution (2026-09-02) — posture decision taken: option (b), +generalized to string/bytes-only.** `maxLength` is rejected at parse on +every non-`string`/`bytes` field kind, not just records — the +"silently unenforced" argument (the validation plan bakes `maxLength` +into string/bytes leaves only, validation_plan.rs:338-339) applies +identically to every other kind, so one gate closes the whole family: + +1. **Parse gate** (`BastField::parse`, the choke point every consumer + inherits — struct fields and union `fields` both parse through it; + records can only appear as inline field kinds since `$defs` entries + are struct/union/enum only, so no ref-following is needed): a + `maxLength` on any kind other than the `string`/`bytes` primitives + is a clean `Schema` error naming the field, the path, the kind, and + the review reference. `BastType::Ref` fields are rejected too — a + ref resolves to a struct/union/enum, never to a string/bytes + primitive, so the annotation is equally dead there and the + resolved shape does not need to be known to reject it. +2. **Meta-schema** (`bast_meta.rs` FieldDef): `if kind ∈ {string, + bytes}` / `else: maxLength: false` — the published + `https://alk.dev/bast/v1/schema` contract now matches the parser + (the N2 dual-layer pattern). +3. **Dead compute arm deleted** (repo precedent M3/M4-item-4): M5's + aligned-mode `maxLength` rejection in `compute_field`'s Record arm + is unreachable after the parse gate and was removed; the + offset-indirect arm stays (still reachable — the parser accepts + offset-indirect on any field). The two M5 `maxLength` tests + (`m5_record_max_length_rejected_in_aligned_mode`, + `m5_record_max_length_rejected_even_as_last_field`) were rewritten + to the parse-rejection posture as the `n3_*` family in `bast.rs`; + `m5_record_offset_indirect_rejected_in_aligned_mode` is unchanged. +4. **ADR-006 remedy message tailored per kind**: for a non-final + *record* field, both annotated remedies are now dead ends + (`maxLength` → N3 parse rejection, `offset-indirect` → M5 compute + rejection), so the error text now tells the consumer to move the + field to the last position and says why; string/bytes fields keep + the original remedy list. +5. **Docs aligned** (the "schema is the format" principle): ADR-003 + §2's kind list and §3a's record-reservation sentence amended with + strikethrough + review reference; bast-format.md FieldDef + + meta-schema listing + Variable-Length Encoding section updated; + layout-engine.md Strategy 2 and the ADR-006 summary paragraph + updated; schema-layer.md annotation semantics paragraph updated; + builder `.max_length()` doc states the string/bytes-only posture. + +Breaking constraint for 0.2.0-era schemas that put `maxLength` on any +non-string/bytes field — same class as M5's aligned-record rejection +(previously silently unenforced, now rejected with a clean error). +Tests: six `n3_*` parse tests in `bast.rs` (record, fixed primitive, +array, inline struct, union shared field rejected; string/bytes still +accepted) and two meta-schema tests (`maxLength` on record rejected by +the published contract; string/bytes accepted). + +Session note (test-count bookkeeping): the resolution log below quotes +567 tests green for the M6/M4 commit (`2eb086f`); the static +`#[test]` count at that commit is 542 (+2 ignored doctests). The +difference matches the disposable probe tests that session ran in-tree +during verification and deleted before commit (per the Methodology +no-reproducer rule). Static counts at the other 0.3.0 commits: 475 +(release `9949f91`), 489 (H3 `05a2a42`), 507 (L2/L3), 510 (N2), 523 +(L4/N1/M5), 542 (M6/M4). + --- ## What's Good @@ -1336,13 +1417,15 @@ Worth recording, because the findings shouldn't eclipse it: `field_endian_for_element` verdict: dead arms deleted); the per-fix coverage posture stays ongoing by design. The M4 session surfaced **M5** and **M6** (both fixed same-day, see their - findings) and **N3** (open). + findings) and **N3** (resolved, see below). 9. ~~**M5**~~ **resolved 2026-09-02** (aligned record `maxLength`/`offset-indirect` rejected at compute). 10. ~~**M6**~~ **resolved 2026-09-02** (legacy walker's field-disc union arm walks shared fields first). -11. **N3** — open: pick the posture (enforce record `maxLength` in - `validate_bytes`, or document string/bytes-only) and close. +11. ~~**N3**~~ **resolved 2026-09-02** (posture decision: `maxLength` + is string/bytes-only — rejected at parse on every other kind, + enforced in the meta-schema; supersedes M5's compute-side record + `maxLength` arm, which became dead and was deleted). ## Notes diff --git a/src/bast.rs b/src/bast.rs index 3a0f82f..9360084 100644 --- a/src/bast.rs +++ b/src/bast.rs @@ -393,7 +393,9 @@ impl BastField { /// The byte-length cap. Enforced as a validation constraint /// (packed mode) or a fixed-size reservation (aligned mode) per - /// ADR-003. + /// ADR-003. Only accepted on `string` and `bytes` fields (review + /// #006 N3: on any other kind the annotation was silently + /// unenforced by `validate_bytes`; rejected at parse now). pub fn max_length(&self) -> Option { self.max_length } @@ -426,10 +428,22 @@ impl BastField { )) })?; let ty = BastType::parse(raw_kind, path, doc_root)?; + let max_length = parse_max_length(node); + if max_length.is_some() && !matches!( + &ty, + BastType::Primitive(AlkTypeKind::String) | BastType::Primitive(AlkTypeKind::Bytes) + ) { + return Err(AlkTypeError::Schema(format!( + "bast: field {name:?} at {path} declares `maxLength` but its kind \ + ({ty}) is not a variable-length primitive — `maxLength` is only \ + honored on `string` and `bytes` fields (review #006 N3: on any \ + other kind the annotation was silently unenforced by \ + validate_bytes)" + ))); + } let endian = parse_endian_opt(node); let align = parse_align(node, path)?; let encoding = parse_encoding(node); - let max_length = parse_max_length(node); Ok(Self { name: name.to_string(), ty, @@ -1240,6 +1254,132 @@ mod tests { assert_eq!(s.fields()[0].encoding(), VariableEncoding::LengthPrefixed); } + // ----- N3: maxLength is string/bytes-only -------------------------- + + #[test] + fn n3_max_length_on_record_field_rejected_at_parse() { + // N3's probe shape: on a record field the annotation did + // nothing — validate_bytes never bounded the entry region by + // it. Rejected at parse now, mirroring M5's aligned-mode + // posture. + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "counts", "kind": { "kind": "record", "values": "uint16" }, "maxLength": 8 } + ] + } + } + }); + let err = BastDoc::new(&root, "S").unwrap_err(); + match err { + AlkTypeError::Schema(msg) => { + assert!(msg.contains("maxLength"), "msg: {msg}"); + assert!(msg.contains("counts"), "msg: {msg}"); + assert!(msg.contains("string"), "msg: {msg}"); + } + other => panic!("expected Schema, got {other:?}"), + } + } + + #[test] + fn n3_max_length_on_fixed_primitive_rejected_at_parse() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "id", "kind": "uint32", "maxLength": 8 } + ] + } + } + }); + let err = BastDoc::new(&root, "S").unwrap_err(); + match err { + AlkTypeError::Schema(msg) => { + assert!(msg.contains("maxLength"), "msg: {msg}"); + assert!(msg.contains("id"), "msg: {msg}"); + } + other => panic!("expected Schema, got {other:?}"), + } + } + + #[test] + fn n3_max_length_on_array_field_rejected_at_parse() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "tags", "kind": { "kind": "array", "element": "string", "count": 4 }, "maxLength": 16 } + ] + } + } + }); + let err = BastDoc::new(&root, "S").unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + } + + #[test] + fn n3_max_length_on_struct_kind_rejected_at_parse() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "nested", "kind": { "kind": "struct", "fields": [ { "name": "a", "kind": "uint8" } ] }, "maxLength": 8 } + ] + } + } + }); + let err = BastDoc::new(&root, "S").unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + } + + #[test] + fn n3_max_length_on_string_and_bytes_still_accepted() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "name", "kind": "string", "maxLength": 256 }, + { "name": "blob", "kind": "bytes", "maxLength": 64 } + ] + } + } + }); + let d = doc_from(&root, "S"); + let s = match d.root_def().kind() { + BastDefKind::Struct(s) => s, + _ => unreachable!(), + }; + assert_eq!(s.fields()[0].max_length(), Some(256)); + assert_eq!(s.fields()[1].max_length(), Some(64)); + } + + #[test] + fn n3_max_length_on_union_field_disc_shared_field_rejected_at_parse() { + // The union's `fields` parse through the same BastField::parse + // choke point, so the gate applies there too. + let root = json!({ + "$defs": { + "U": { + "kind": "union", + "discriminator": { "kind": "field", "name": "type" }, + "fields": [ + { "name": "type", "kind": "uint8" }, + { "name": "payload", "kind": { "kind": "record", "values": "uint8" }, "maxLength": 16 } + ], + "mapping": { "1": { "kind": "struct", "fields": [] } } + } + } + }); + let err = BastDoc::new(&root, "U").unwrap_err(); + assert!(matches!(err, AlkTypeError::Schema(_)), "got {err:?}"); + } + // ----- TypeRef: primitives, $ref, array, record ------------------- #[test] diff --git a/src/bast_meta.rs b/src/bast_meta.rs index c5a71a6..9b8ad3d 100644 --- a/src/bast_meta.rs +++ b/src/bast_meta.rs @@ -80,6 +80,12 @@ pub static BAST_META_SCHEMA: LazyLock = LazyLock::new(|| { "encoding": { "enum": ["length-prefixed", "offset-indirect"] }, "maxLength": { "type": "integer", "minimum": 0 } }, + "if": { + "properties": { + "kind": { "enum": ["string", "bytes"] } + } + }, + "else": { "properties": { "maxLength": false } }, "required": ["name", "kind"], "additionalProperties": false }, @@ -518,4 +524,47 @@ mod tests { }); assert!(validator.validate(&doc).is_ok(), "inline struct in union mapping rejected"); } + + #[test] + fn meta_schema_rejects_max_length_on_non_string_bytes_field() { + let validator = jsonschema::options() + .build(&BAST_META_SCHEMA) + .expect("meta-schema compiles"); + let doc = serde_json::json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "counts", "kind": { "kind": "record", "values": "uint16" }, "maxLength": 8 } + ] + } + } + }); + assert!( + validator.validate(&doc).is_err(), + "maxLength on a record field accepted by the meta-schema" + ); + } + + #[test] + fn meta_schema_accepts_max_length_on_string_and_bytes_field() { + let validator = jsonschema::options() + .build(&BAST_META_SCHEMA) + .expect("meta-schema compiles"); + let doc = serde_json::json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "name", "kind": "string", "maxLength": 256 }, + { "name": "blob", "kind": "bytes", "maxLength": 64 } + ] + } + } + }); + assert!( + validator.validate(&doc).is_ok(), + "maxLength on string/bytes fields rejected" + ); + } } \ No newline at end of file diff --git a/src/builder.rs b/src/builder.rs index 9997ea6..874229f 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -372,6 +372,8 @@ impl Schema { /// Schema type, emitted as the standard `maxLength` keyword. /// In aligned mode with a variable-length type, reserves this many /// bytes (strategy 2). In packed mode, validation constraint only. + /// Only honored on `string`/`bytes` fields — the BAST parser rejects + /// it on any other kind (review #006 N3). pub fn max_length(mut self, max: usize) -> Self { match &mut self.repr { Repr::Standard(map) => { diff --git a/src/offset_map.rs b/src/offset_map.rs index c6eb855..8ef0b1c 100644 --- a/src/offset_map.rs +++ b/src/offset_map.rs @@ -259,15 +259,29 @@ impl<'d> ComputeCtx<'d> { let is_inline_length_prefixed = encoding == VariableEncoding::LengthPrefixed && max_length.is_none(); if is_inline_length_prefixed { + let remedy = if kind == AlkTypeKind::Record { + // For records both annotated remedies are + // dead ends: `maxLength` is rejected at + // parse (review #006 N3) and + // `offset-indirect` is rejected right + // below (review #006 M5) — moving to the + // last position is the only fix. + "move this field to the last position in the struct (for a \ + record field, neither `maxLength` nor `offset-indirect` \ + is available: `maxLength` is rejected at parse — review \ + #006 N3 — and `offset-indirect` is rejected for records \ + in aligned mode — review #006 M5)" + } else { + "Use `maxLength` (fixed-size reservation) or \ + `\"encoding\": \"offset-indirect\"`, or move this \ + field to the last position in the struct" + }; return Err(AlkTypeError::Offset { field_path: field_path.clone(), reason: format!( "non-final inline length-prefixed variable field \ ({kind}) in aligned mode: the variable data would \ - clobber subsequent fields. Use `maxLength` \ - (fixed-size reservation) or \ - `\"encoding\": \"offset-indirect\"`, or move this \ - field to the last position in the struct. (ADR-006)" + clobber subsequent fields. {remedy}. (ADR-006)" ), }); } @@ -320,18 +334,6 @@ impl<'d> ComputeCtx<'d> { .to_string(), }); } - if field.max_length().is_some() { - return Err(AlkTypeError::Offset { - field_path: field_path.to_string(), - reason: "record field with `maxLength` is not supported in aligned mode: \ - the materializer walks the record's inline count-prefixed form \ - from the entry start and does not bound it by the reservation, \ - so the record data would silently cross the maxLength boundary \ - into subsequent fields (review #006 M5). Use the default inline \ - length-prefixing (as the last field), or packed mode." - .to_string(), - }); - } self.compute_variable_field(field, field_path, struct_default_align, resolved.alk_kind(), field_endian) } BastType::Primitive(k) if k.is_variable_length() => { @@ -1138,37 +1140,9 @@ mod tests { assert_eq!(m.total_size(), 8); } - // ----- M5: record maxLength / offset-indirect rejected in aligned mode - - #[test] - fn m5_record_max_length_rejected_in_aligned_mode() { - // Probe-verified silent corruption: the materializer walks the - // record's inline count-prefixed form from the entry start and - // never bounds it by the maxLength reservation, so the record - // data crosses the reservation into the next field's bytes and - // validate_bytes accepts the corrupt buffer. Reject at compute. - let root = json!({ - "$defs": { - "S": { - "kind": "struct", - "fields": [ - { "name": "counts", "kind": { "kind": "record", "values": "uint16" }, "maxLength": 8 }, - { "name": "id", "kind": "uint32" } - ] - } - } - }); - let doc = BastDoc::new(&root, "S").expect("doc parses"); - let err = OffsetMap::compute(&doc).unwrap_err(); - match err { - AlkTypeError::Offset { field_path, reason } => { - assert_eq!(field_path, "counts"); - assert!(reason.contains("maxLength"), "reason: {reason}"); - assert!(reason.contains("record"), "reason: {reason}"); - } - other => panic!("expected Offset, got {other:?}"), - } - } + // ----- M5: record offset-indirect rejected in aligned mode; record + // maxLength now rejected earlier, at parse (N3) — see the n3_ family + // in bast.rs #[test] fn m5_record_offset_indirect_rejected_in_aligned_mode() { @@ -1198,27 +1172,6 @@ mod tests { } } - #[test] - fn m5_record_max_length_rejected_even_as_last_field() { - // Position doesn't rescue it: the reservation would fix - // total_size, but the walk still reads the inline form — a - // different wire shape than any writer produces. - let root = json!({ - "$defs": { - "S": { - "kind": "struct", - "fields": [ - { "name": "id", "kind": "uint32" }, - { "name": "counts", "kind": { "kind": "record", "values": "uint16" }, "maxLength": 64 } - ] - } - } - }); - let doc = BastDoc::new(&root, "S").expect("doc parses"); - let err = OffsetMap::compute(&doc).unwrap_err(); - assert!(matches!(err, AlkTypeError::Offset { .. }), "got {err:?}"); - } - #[test] fn non_final_inline_string_rejected_in_aligned_mode() { let root = json!({