From e5c7cc1ca21c19ba8a795b6a41dbbc352e3f4c89 Mon Sep 17 00:00:00 2001 From: deepseek-v4-pro Date: Sun, 16 Aug 2026 08:38:26 +0000 Subject: [PATCH] Fix offset-indirect, field-level endian, and aligned materialization Resolves review #003 findings M1, M2, L1, and L2. - offset-indirect (L1 + M2 arm): rework read_*_indirect to same-buffer absolute offsets (safetensors-style, per the metatensor model) and add write_*_indirect. Wire the encoding dispatch into engine.read_field/ write_field and the aligned materializer. Previously the encoding was laid out but never read back. - field-level endian override (M1): thread field.effective_endian through sequential_reader, engine.read_field/write_field, and materialize_struct_aligned. Previously a per-field endian override was silently ignored, misreading multi-byte values. - aligned-mode materialization (M2): materialize fixed-size arrays via their vals[i] offset-map entries and maxLength reservations as zero-padded fixed-size slices (trailing NULs trimmed). Previously both read garbage or errored. - dead endian param (L2): drop the ignored endian argument from materialize_packed/materialize_aligned; endianness is read from the root struct. Verification: cargo test --release (404 pass), cargo clippy --all-targets -- -D warnings (clean), cargo build --target wasm32-unknown-unknown --release (clean), cargo llvm-cov --release (89.60% lines). --- src/bast_validation.rs | 3 +- src/data_access.rs | 167 +++++++++++++++++----- src/engine.rs | 202 +++++++++++++++++++++++--- src/materialize.rs | 301 +++++++++++++++++++++++++++++++++++---- src/sequential_reader.rs | 20 +++ tests/error_paths.rs | 21 ++- 6 files changed, 612 insertions(+), 102 deletions(-) diff --git a/src/bast_validation.rs b/src/bast_validation.rs index 19575ba..ed146f1 100644 --- a/src/bast_validation.rs +++ b/src/bast_validation.rs @@ -442,7 +442,6 @@ fn is_rfc3339_timestamp(s: &str) -> bool { #[cfg(test)] mod tests { use super::*; - use crate::schema::Endian; use serde_json::json; fn doc_from<'a>(root: &'a Value, name: &'a str) -> BastDoc<'a> { @@ -463,7 +462,7 @@ mod tests { doc: &BastDoc<'_>, buffer: &[u8], ) -> Result<(), AlkTypeError> { - let value = crate::materialize::materialize_packed(doc, buffer, Endian::Little)?; + let value = crate::materialize::materialize_packed(doc, buffer)?; validate_value(doc, &value) } diff --git a/src/data_access.rs b/src/data_access.rs index 863a2d8..2a16dec 100644 --- a/src/data_access.rs +++ b/src/data_access.rs @@ -294,24 +294,23 @@ pub fn write_bytes( } // --------------------------------------------------------------------------- -// Variable-length read (offset indirection) +// Variable-length read/write (offset indirection) // --------------------------------------------------------------------------- /// Read an offset-indirect string. /// /// The 8-byte struct at `buffer[offset..offset+8]` is /// `{ data_offset: u32, data_length: u32 }` (endian-aware). The actual UTF-8 -/// bytes live in `data_region[data_offset..data_offset+data_length]`. Returns -/// a `&'a str` borrowing from `data_region`. Invalid UTF-8 produces -/// [`AlkTypeError::Access`]. +/// bytes live at `buffer[data_offset..data_offset+data_length]` — offsets are +/// absolute into the same buffer (safetensors-style). Returns a `&'a str` +/// borrowing from `buffer`. Invalid UTF-8 produces [`AlkTypeError::Access`]. pub fn read_string_indirect<'a>( buffer: &'a [u8], offset: usize, - data_region: &'a [u8], field_path: &str, endian: Endian, ) -> Result<&'a str, AlkTypeError> { - let bytes = read_bytes_indirect(buffer, offset, data_region, field_path, endian)?; + let bytes = read_bytes_indirect(buffer, offset, field_path, endian)?; std::str::from_utf8(bytes).map_err(|e| { access_err( field_path, @@ -324,34 +323,97 @@ pub fn read_string_indirect<'a>( /// /// The 8-byte struct at `buffer[offset..offset+8]` is /// `{ data_offset: u32, data_length: u32 }` (endian-aware). Returns a -/// `&'a [u8]` slice of `data_region[data_offset..data_offset+data_length]`. +/// `&'a [u8]` slice of `buffer[data_offset..data_offset+data_length]` — +/// offsets are absolute into the same buffer. pub fn read_bytes_indirect<'a>( buffer: &'a [u8], offset: usize, - data_region: &'a [u8], field_path: &str, endian: Endian, ) -> Result<&'a [u8], AlkTypeError> { - let struct_end = offset - .checked_add(8) - .ok_or_else(|| access_err(field_path, format!("offset {offset} + 8 overflows usize")))?; - check_bounds(buffer.len(), offset, struct_end, field_path)?; - let off_bytes: [u8; U32_SIZE] = buffer[offset..offset + U32_SIZE] - .try_into() - .map_err(|_| access_err(field_path, "internal: try_into failed for data_offset"))?; - let len_bytes: [u8; U32_SIZE] = buffer[offset + U32_SIZE..offset + 8] - .try_into() - .map_err(|_| access_err(field_path, "internal: try_into failed for data_length"))?; - let data_offset = u32_from(off_bytes, endian) as usize; - let data_length = u32_from(len_bytes, endian) as usize; + let data_offset = read_u32(buffer, offset, field_path, endian)? as usize; + let len_offset = offset.checked_add(U32_SIZE).ok_or_else(|| { + access_err( + field_path, + format!("offset {offset} + {U32_SIZE} overflows usize"), + ) + })?; + let data_length = read_u32(buffer, len_offset, field_path, endian)? as usize; let data_end = data_offset.checked_add(data_length).ok_or_else(|| { access_err( field_path, format!("data_offset {data_offset} + data_length {data_length} overflows usize"), ) })?; - check_bounds(data_region.len(), data_offset, data_end, field_path)?; - Ok(&data_region[data_offset..data_end]) + check_bounds(buffer.len(), data_offset, data_end, field_path)?; + Ok(&buffer[data_offset..data_end]) +} + +/// Write an offset-indirect string. +/// +/// Writes the `{ data_offset: u32, data_length: u32 }` pair at `offset` and +/// the UTF-8 bytes at `data_offset` (absolute into the same buffer). Returns +/// the number of bytes written for the pair (`8`). +pub fn write_string_indirect( + buffer: &mut [u8], + offset: usize, + data_offset: usize, + value: &str, + field_path: &str, + endian: Endian, +) -> Result { + write_bytes_indirect(buffer, offset, data_offset, value.as_bytes(), field_path, endian) +} + +/// Write offset-indirect raw bytes. +/// +/// Writes the `{ data_offset: u32, data_length: u32 }` pair at `offset` and +/// the raw bytes at `data_offset` (absolute into the same buffer). Returns +/// the number of bytes written for the pair (`8`). +pub fn write_bytes_indirect( + buffer: &mut [u8], + offset: usize, + data_offset: usize, + value: &[u8], + field_path: &str, + endian: Endian, +) -> Result { + let data_len = value.len(); + let data_len_u32 = u32::try_from(data_len).map_err(|_| { + access_err( + field_path, + format!("data length {data_len} exceeds u32::MAX (length field width)"), + ) + })?; + let data_offset_u32 = u32::try_from(data_offset).map_err(|_| { + access_err( + field_path, + format!("data offset {data_offset} exceeds u32::MAX (offset field width)"), + ) + })?; + write_u32(buffer, offset, data_offset_u32, field_path, endian)?; + let len_offset = offset.checked_add(U32_SIZE).ok_or_else(|| { + access_err( + field_path, + format!("offset {offset} + {U32_SIZE} overflows usize"), + ) + })?; + write_u32(buffer, len_offset, data_len_u32, field_path, endian)?; + let data_end = data_offset.checked_add(data_len).ok_or_else(|| { + access_err( + field_path, + format!("data offset {data_offset} + length {data_len} overflows usize"), + ) + })?; + check_bounds(buffer.len(), data_offset, data_end, field_path)?; + let dest = buffer.get_mut(data_offset..data_end).ok_or_else(|| { + access_err( + field_path, + format!("mutable data slice [{data_offset}..{data_end}) unavailable"), + ) + })?; + dest.copy_from_slice(value); + Ok(8) } #[cfg(test)] @@ -583,39 +645,70 @@ mod tests { #[test] fn read_string_indirect_round_trip() { - let data_region = b"the quick brown fox"; - let mut index = [0u8; 8]; - write_u32(&mut index, 0, 4, "idx.off", LE).unwrap(); - write_u32(&mut index, 4, 11, "idx.len", LE).unwrap(); - let s = read_string_indirect(&index, 0, data_region, "msg", LE).unwrap(); + let data = b"the quick brown fox"; + let mut full = vec![0u8; 8 + data.len()]; + write_u32(&mut full, 0, 12, "idx.off", LE).unwrap(); + write_u32(&mut full, 4, 11, "idx.len", LE).unwrap(); + full[8..].copy_from_slice(data); + let s = read_string_indirect(&full, 0, "msg", LE).unwrap(); assert_eq!(s, "quick brown"); } #[test] fn read_bytes_indirect_round_trip() { - let data_region: &[u8] = b"HEADERbody-payloadTAIL"; - let mut index = [0u8; 8]; - write_u32(&mut index, 0, 6, "idx.off", BE).unwrap(); - write_u32(&mut index, 4, 12, "idx.len", BE).unwrap(); - let bytes = read_bytes_indirect(&index, 0, data_region, "blob", BE).unwrap(); + let data: &[u8] = b"HEADERbody-payloadTAIL"; + let mut full = vec![0u8; 8 + data.len()]; + write_u32(&mut full, 0, 14, "idx.off", BE).unwrap(); + write_u32(&mut full, 4, 12, "idx.len", BE).unwrap(); + full[8..].copy_from_slice(data); + let bytes = read_bytes_indirect(&full, 0, "blob", BE).unwrap(); assert_eq!(bytes, b"body-payload"); } + #[test] + fn write_bytes_indirect_round_trip() { + let mut buf = vec![0u8; 8 + 11]; + let written = write_bytes_indirect(&mut buf, 0, 8, b"quick brown", "blob", LE).unwrap(); + assert_eq!(written, 8); + assert_eq!(&buf[0..4], &8u32.to_le_bytes()); + assert_eq!(&buf[4..8], &11u32.to_le_bytes()); + assert_eq!(&buf[8..19], b"quick brown"); + let bytes = read_bytes_indirect(&buf, 0, "blob", LE).unwrap(); + assert_eq!(bytes, b"quick brown"); + } + + #[test] + fn write_string_indirect_round_trip() { + let mut buf = vec![0u8; 8 + 5]; + let written = write_string_indirect(&mut buf, 0, 8, "hello", "msg", BE).unwrap(); + assert_eq!(written, 8); + assert_eq!(&buf[0..4], &8u32.to_be_bytes()); + assert_eq!(&buf[4..8], &5u32.to_be_bytes()); + assert_eq!(&buf[8..13], b"hello"); + let s = read_string_indirect(&buf, 0, "msg", BE).unwrap(); + assert_eq!(s, "hello"); + } + #[test] fn read_bytes_indirect_bounds_failure_on_index() { let buf = [0u8; 4]; - let data_region = b"anything"; - let err = read_bytes_indirect(&buf, 0, data_region, "blob", LE).unwrap_err(); + let err = read_bytes_indirect(&buf, 0, "blob", LE).unwrap_err(); assert!(matches!(err, AlkTypeError::Access { .. })); } #[test] - fn read_bytes_indirect_bounds_failure_on_data_region() { + fn read_bytes_indirect_bounds_failure_on_data() { let mut buf = [0u8; 8]; write_u32(&mut buf, 0, 100, "idx.off", LE).unwrap(); write_u32(&mut buf, 4, 10, "idx.len", LE).unwrap(); - let data_region = b"too short"; - let err = read_bytes_indirect(&buf, 0, data_region, "blob", LE).unwrap_err(); + let err = read_bytes_indirect(&buf, 0, "blob", LE).unwrap_err(); + assert!(matches!(err, AlkTypeError::Access { .. })); + } + + #[test] + fn write_bytes_indirect_bounds_failure_on_data() { + let mut buf = vec![0u8; 8]; + let err = write_bytes_indirect(&mut buf, 0, 100, b"hello", "blob", LE).unwrap_err(); assert!(matches!(err, AlkTypeError::Access { .. })); } diff --git a/src/engine.rs b/src/engine.rs index bc031ea..6959132 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -23,7 +23,7 @@ use crate::error::AlkTypeError; use crate::layout_builder::LayoutBuilder; use crate::materialize; use crate::offset_map::OffsetMap; -use crate::schema::{Endian, AlkTypeKind}; +use crate::schema::{AlkTypeKind, Endian, VariableEncoding}; use crate::sequential_reader::{FieldValue, SequentialReader}; use crate::validation; use serde_json::Value; @@ -282,9 +282,9 @@ impl AlkTypeEngine { pub fn validate_bytes(&self, buffer: &[u8]) -> Result<(), AlkTypeError> { let doc = BastDoc::new(&self.bast_doc, &self.root_name)?; let value = match &self.layout { - Layout::Packed { .. } => materialize::materialize_packed(&doc, buffer, self.endian)?, + Layout::Packed { .. } => materialize::materialize_packed(&doc, buffer)?, Layout::Aligned { offset_map } => { - materialize::materialize_aligned(&doc, buffer, offset_map, self.endian)? + materialize::materialize_aligned(&doc, buffer, offset_map)? } }; bast_validation::validate_value(&doc, &value) @@ -331,11 +331,12 @@ impl AlkTypeEngine { reason: "field not found in offset map".to_string(), })?; let doc = BastDoc::new(&self.bast_doc, &self.root_name)?; - let kind = lookup_field_kind(&doc, field_path).ok_or_else(|| AlkTypeError::Offset { + let leaf = lookup_leaf_field(&doc, field_path).ok_or_else(|| AlkTypeError::Offset { field_path: field_path.to_string(), reason: "field schema not found in BAST tree or has no primitive kind".to_string(), })?; - let endian = self.endian; + let kind = leaf.kind; + let endian = leaf.endian; match kind { AlkTypeKind::Int8 => { let v = data_access::read_i8(buffer, range.start, field_path)?; @@ -386,11 +387,25 @@ impl AlkTypeEngine { Ok(FieldValue::Enum(v)) } AlkTypeKind::String => { - let v = data_access::read_string(buffer, range.start, field_path, endian)?; + let v = match leaf.encoding { + VariableEncoding::OffsetIndirect => { + data_access::read_string_indirect(buffer, range.start, field_path, endian)? + } + VariableEncoding::LengthPrefixed => { + data_access::read_string(buffer, range.start, field_path, endian)? + } + }; Ok(FieldValue::String(v)) } AlkTypeKind::Bytes => { - let v = data_access::read_bytes(buffer, range.start, field_path, endian)?; + let v = match leaf.encoding { + VariableEncoding::OffsetIndirect => { + data_access::read_bytes_indirect(buffer, range.start, field_path, endian)? + } + VariableEncoding::LengthPrefixed => { + data_access::read_bytes(buffer, range.start, field_path, endian)? + } + }; Ok(FieldValue::Bytes(v)) } AlkTypeKind::Timestamp => { @@ -452,7 +467,12 @@ impl AlkTypeEngine { field_path: field_path.to_string(), reason: "field not found in offset map".to_string(), })?; - let endian = self.endian; + let doc = BastDoc::new(&self.bast_doc, &self.root_name)?; + let leaf = lookup_leaf_field(&doc, field_path).ok_or_else(|| AlkTypeError::Offset { + field_path: field_path.to_string(), + reason: "field schema not found in BAST tree or has no primitive kind".to_string(), + })?; + let endian = leaf.endian; match value { FieldValue::I8(v) => data_access::write_i8(buffer, range.start, *v, field_path), FieldValue::I16(v) => { @@ -485,10 +505,28 @@ impl AlkTypeEngine { data_access::write_enum(buffer, range.start, *v, field_path, endian) } FieldValue::String(v) => { + if leaf.encoding == VariableEncoding::OffsetIndirect { + return Err(AlkTypeError::Access { + field_path: field_path.to_string(), + reason: "write_field cannot write offset-indirect fields; \ + use data_access::write_string_indirect with the \ + offset map range and a data offset" + .to_string(), + }); + } data_access::write_string(buffer, range.start, v, field_path, endian)?; Ok(()) } FieldValue::Bytes(v) => { + if leaf.encoding == VariableEncoding::OffsetIndirect { + return Err(AlkTypeError::Access { + field_path: field_path.to_string(), + reason: "write_field cannot write offset-indirect fields; \ + use data_access::write_bytes_indirect with the \ + offset map range and a data offset" + .to_string(), + }); + } data_access::write_bytes(buffer, range.start, v, field_path, endian)?; Ok(()) } @@ -515,31 +553,49 @@ impl fmt::Debug for AlkTypeEngine { } } -/// Walk the BAST typed tree to find the primitive/composite kind for a -/// dotted field path. Returns `None` if any segment is missing or the -/// resolved type is a `$ref` that can't be resolved. +/// The resolved leaf-field metadata needed by `read_field`/`write_field`: +/// the field's kind, its variable-length encoding, and its effective +/// endianness (field override, else the enclosing struct's default). +struct LeafFieldInfo { + kind: AlkTypeKind, + encoding: VariableEncoding, + endian: Endian, +} + +/// Walk the BAST typed tree to find the leaf field for a dotted field +/// path. Returns `None` if any segment is missing or the resolved type is +/// a `$ref` that can't be resolved. /// /// Splits `field_path` on `.` and descends into the root struct's /// `fields` at each step, resolving `$ref`s via [`BastDoc::resolve_typeref`]. /// Array element segments (`field[i]`) are not handled here — /// `read_field`/`write_field` only address leaf fields. -fn lookup_field_kind(doc: &BastDoc<'_>, field_path: &str) -> Option { +fn lookup_leaf_field(doc: &BastDoc<'_>, field_path: &str) -> Option { let root_def = doc.root_def(); let mut current_struct: BastStruct<'_> = match root_def.kind() { BastDefKind::Struct(s) => s.clone(), _ => return None, }; + let mut struct_endian = current_struct.endian(); let segments: Vec<&str> = field_path.split('.').collect(); let last = segments.len(); for (i, segment) in segments.iter().enumerate() { let field = current_struct.fields().iter().find(|f| f.name() == *segment)?; let ty = field.ty(); let resolved = doc.resolve_typeref(ty).ok()?; + let field_endian = field.effective_endian(struct_endian); if i + 1 == last { - return Some(resolved.alk_kind()); + return Some(LeafFieldInfo { + kind: resolved.alk_kind(), + encoding: field.encoding(), + endian: field_endian, + }); } match &resolved { - BastType::Struct(s) => current_struct = s.clone(), + BastType::Struct(s) => { + current_struct = s.clone(); + struct_endian = s.endian(); + } _ => return None, } } @@ -867,7 +923,7 @@ mod tests { } #[test] - fn lookup_field_kind_walks_dotted_path() { + fn lookup_leaf_field_walks_dotted_path() { let doc = json!({ "$defs": { "S": { @@ -888,10 +944,10 @@ mod tests { } }); let d = BastDoc::new(&doc, "S").expect("doc"); - assert_eq!(lookup_field_kind(&d, "header.magic"), Some(AlkTypeKind::Uint32)); - assert_eq!(lookup_field_kind(&d, "header.version"), Some(AlkTypeKind::Uint8)); - assert_eq!(lookup_field_kind(&d, "header.missing"), None); - assert_eq!(lookup_field_kind(&d, "missing"), None); + assert_eq!(lookup_leaf_field(&d, "header.magic").unwrap().kind, AlkTypeKind::Uint32); + assert_eq!(lookup_leaf_field(&d, "header.version").unwrap().kind, AlkTypeKind::Uint8); + assert!(lookup_leaf_field(&d, "header.missing").is_none()); + assert!(lookup_leaf_field(&d, "missing").is_none()); } #[test] @@ -980,6 +1036,114 @@ mod tests { } } + #[test] + fn read_field_aligned_honors_field_level_endian_override() { + let doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "crc", "kind": "uint32", "endian": "little" } + ] + } + } + }); + let engine = AlkTypeEngine::compile(&doc, "S", LayoutMode::Aligned, None).expect("compile"); + let buf = [0x04u8, 0x03, 0x02, 0x01]; + assert_eq!( + engine.read_field(&buf, "crc").unwrap(), + FieldValue::U32(0x01020304) + ); + } + + #[test] + fn write_field_aligned_honors_field_level_endian_override() { + let doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "crc", "kind": "uint32", "endian": "little" } + ] + } + } + }); + let engine = AlkTypeEngine::compile(&doc, "S", LayoutMode::Aligned, None).expect("compile"); + let mut buf = [0u8; 4]; + engine + .write_field(&mut buf, "crc", &FieldValue::U32(0x01020304)) + .unwrap(); + assert_eq!(buf, [0x04, 0x03, 0x02, 0x01]); + } + + #[test] + fn read_field_aligned_reads_offset_indirect_bytes() { + let doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "blob", "kind": "bytes", "encoding": "offset-indirect" } + ] + } + } + }); + let engine = AlkTypeEngine::compile(&doc, "S", LayoutMode::Aligned, None).expect("compile"); + let mut buf = vec![0u8; 8 + 3]; + buf[0..4].copy_from_slice(&8u32.to_le_bytes()); + buf[4..8].copy_from_slice(&3u32.to_le_bytes()); + buf[8..11].copy_from_slice(&[0xAA, 0xBB, 0xCC]); + match engine.read_field(&buf, "blob").unwrap() { + FieldValue::Bytes(b) => assert_eq!(b, &[0xAA, 0xBB, 0xCC]), + other => panic!("expected Bytes, got {other:?}"), + } + } + + #[test] + fn read_field_aligned_reads_offset_indirect_string() { + let doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "name", "kind": "string", "encoding": "offset-indirect" } + ] + } + } + }); + let engine = AlkTypeEngine::compile(&doc, "S", LayoutMode::Aligned, None).expect("compile"); + let mut buf = vec![0u8; 8 + 5]; + buf[0..4].copy_from_slice(&8u32.to_le_bytes()); + buf[4..8].copy_from_slice(&5u32.to_le_bytes()); + buf[8..13].copy_from_slice(b"hello"); + match engine.read_field(&buf, "name").unwrap() { + FieldValue::String(s) => assert_eq!(s, "hello"), + other => panic!("expected String, got {other:?}"), + } + } + + #[test] + fn write_field_aligned_rejects_offset_indirect() { + let doc = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "blob", "kind": "bytes", "encoding": "offset-indirect" } + ] + } + } + }); + let engine = AlkTypeEngine::compile(&doc, "S", LayoutMode::Aligned, None).expect("compile"); + let mut buf = vec![0u8; 16]; + let err = engine + .write_field(&mut buf, "blob", &FieldValue::Bytes(&[1, 2, 3])) + .unwrap_err(); + assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); + } + // ----- validate_bytes tests (ADR-010) ----- // // Note: step 4 wires the layout layer to BAST. The `validate_bytes` diff --git a/src/materialize.rs b/src/materialize.rs index bc69299..9f569c4 100644 --- a/src/materialize.rs +++ b/src/materialize.rs @@ -29,7 +29,7 @@ use crate::bast::{ }; use crate::data_access; use crate::error::AlkTypeError; -use crate::schema::{AlkTypeKind, Endian}; +use crate::schema::{AlkTypeKind, Endian, VariableEncoding}; use serde_json::{Map, Value}; const U32_SIZE: usize = 4; @@ -38,13 +38,11 @@ const DISCRIMINATOR_KEY: &str = "__discriminator"; /// Materialize a `Value` tree from `buffer` by walking the BAST root type /// in packed mode (sequential, from offset 0). /// -/// The root type must be a struct. `endian` is the schema's endianness -/// (parsed by the caller); a struct-level `endian` annotation overrides -/// it for the root and all nested structs. +/// The root type must be a struct. Endianness is read from the root +/// struct's `endian` annotation (defaults to little-endian). pub fn materialize_packed( doc: &BastDoc<'_>, buffer: &[u8], - endian: Endian, ) -> Result { let root_def = doc.root_def(); let struct_node = match root_def.kind() { @@ -57,7 +55,6 @@ pub fn materialize_packed( } }; let effective_endian = struct_node.endian(); - let _ = endian; materialize_struct_packed(doc, struct_node, "", effective_endian, buffer, &mut 0) } @@ -65,12 +62,12 @@ pub fn materialize_packed( /// in aligned mode (offsets from `offset_map`). /// /// The root type must be a struct. `offset_map` must have been computed -/// from the same BAST document. `endian` is the schema's endianness. +/// from the same BAST document. Endianness is read from the root struct's +/// `endian` annotation (defaults to little-endian). pub fn materialize_aligned( doc: &BastDoc<'_>, buffer: &[u8], offset_map: &crate::offset_map::OffsetMap, - endian: Endian, ) -> Result { let root_def = doc.root_def(); let struct_node = match root_def.kind() { @@ -83,7 +80,6 @@ pub fn materialize_aligned( } }; let effective_endian = struct_node.endian(); - let _ = endian; materialize_struct_aligned(doc, struct_node, "", offset_map, effective_endian, buffer) } @@ -456,8 +452,8 @@ fn number_from_f64(f: f64) -> Value { /// Aligned-mode materialization: walk the BAST typed tree, but read /// leaves at offsets from the `OffsetMap` and fall back to packed-style -/// sequential walks for composites (Array/Union/Record) whose internal -/// layout can't be precomputed into the static offset map. +/// sequential walks for composites (Record) whose internal layout can't +/// be precomputed into the static offset map. fn materialize_struct_aligned( doc: &BastDoc<'_>, struct_node: &BastStruct<'_>, @@ -476,21 +472,30 @@ fn materialize_struct_aligned( let ty = field.ty(); let resolved = doc.resolve_typeref(ty)?; let kind = resolved.alk_kind(); - let value = if kind.is_fixed_size() || kind.is_variable_length() { + let field_endian = field.effective_endian(endian); + let value = if kind.is_fixed_size() { let range = offset_map.get(&path).ok_or_else(|| AlkTypeError::Offset { field_path: path.clone(), reason: "field not found in offset map".to_string(), })?; - materialize_leaf_at(doc, &resolved, &path, range.start, endian, buffer)? + materialize_leaf_at(doc, &resolved, &path, range.start, field_endian, buffer)? } else if kind == AlkTypeKind::Struct { if let BastType::Struct(s) = &resolved { - materialize_struct_aligned(doc, s, &path, offset_map, endian, buffer)? + materialize_struct_aligned(doc, s, &path, offset_map, field_endian, buffer)? } else { return Err(AlkTypeError::Schema(format!( "materialize: resolved kind Struct but type was {resolved} at {path}" ))); } - } else { + } else if kind == AlkTypeKind::Array { + if let BastType::Array(a) = &resolved { + materialize_array_aligned(doc, a, &path, offset_map, field_endian, buffer)? + } else { + return Err(AlkTypeError::Schema(format!( + "materialize: resolved kind Array but type was {resolved} at {path}" + ))); + } + } else if kind == AlkTypeKind::Record { let range = offset_map.get(&path).ok_or_else(|| AlkTypeError::Offset { field_path: path.clone(), reason: "field not found in offset map".to_string(), @@ -501,16 +506,124 @@ fn materialize_struct_aligned( &resolved, field, &path, - endian, + field_endian, buffer, &mut offset, )? + } else if kind.is_variable_length() { + let range = offset_map.get(&path).ok_or_else(|| AlkTypeError::Offset { + field_path: path.clone(), + reason: "field not found in offset map".to_string(), + })?; + materialize_variable_aligned(doc, field, &resolved, &path, range, field_endian, buffer)? + } else { + return Err(AlkTypeError::Schema(format!( + "materialize: unsupported kind {kind} at {path} in aligned mode" + ))); }; obj.insert(field.name().to_string(), value); } Ok(Value::Object(obj)) } +/// Materialize a fixed-size array in aligned mode by reading each element +/// at its `vals[i]` offset-map entry. Array elements are always fixed-size +/// in aligned mode (the offset map rejects variable-length elements). +fn materialize_array_aligned( + doc: &BastDoc<'_>, + array: &BastArray<'_>, + field_path: &str, + offset_map: &crate::offset_map::OffsetMap, + endian: Endian, + buffer: &[u8], +) -> Result { + let element_ty = array.element(); + let resolved_elem = doc.resolve_typeref(element_ty)?; + let count = array.count(); + let mut arr = Vec::with_capacity(count); + for i in 0..count { + let elem_path = format!("{field_path}[{i}]"); + let range = offset_map.get(&elem_path).ok_or_else(|| AlkTypeError::Offset { + field_path: elem_path.clone(), + reason: "array element not found in offset map".to_string(), + })?; + let value = materialize_leaf_at(doc, &resolved_elem, &elem_path, range.start, endian, buffer)?; + arr.push(value); + } + Ok(Value::Array(arr)) +} + +/// Materialize a variable-length leaf (String/Bytes/Timestamp) in aligned +/// mode, dispatching on the field's encoding strategy: +/// +/// - `offset-indirect`: an 8-byte `{offset, length}` pair at `start`, +/// pointing absolutely into the same buffer. +/// - `maxLength` reservation: `end - start` bytes of zero-padded data; +/// trailing NUL bytes are trimmed. +/// - inline length-prefixed (default): a 4-byte length prefix at `start`. +fn materialize_variable_aligned( + doc: &BastDoc<'_>, + field: &BastField<'_>, + ty: &BastType<'_>, + field_path: &str, + range: &crate::offset_map::ByteRange, + endian: Endian, + buffer: &[u8], +) -> Result { + let start = range.start; + match field.encoding() { + VariableEncoding::OffsetIndirect => match ty { + BastType::Primitive(AlkTypeKind::String) + | BastType::Primitive(AlkTypeKind::Timestamp) => { + let s = data_access::read_string_indirect(buffer, start, field_path, endian)?; + Ok(Value::String(s.to_string())) + } + BastType::Primitive(AlkTypeKind::Bytes) => { + let b = data_access::read_bytes_indirect(buffer, start, field_path, endian)?; + let arr: Vec = b.iter().map(|&byte| Value::from(u32::from(byte))).collect(); + Ok(Value::Array(arr)) + } + _ => Err(AlkTypeError::Schema(format!( + "materialize: offset-indirect encoding on non-variable kind at {field_path}" + ))), + }, + VariableEncoding::LengthPrefixed => { + if let Some(_max_len) = field.max_length() { + let data = buffer.get(start..range.end).ok_or_else(|| AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!("reservation [{start}..{}) unavailable", range.end), + })?; + let trimmed_len = data + .iter() + .rposition(|&b| b != 0) + .map(|i| i + 1) + .unwrap_or(0); + let trimmed = &data[..trimmed_len]; + match ty { + BastType::Primitive(AlkTypeKind::String) + | BastType::Primitive(AlkTypeKind::Timestamp) => { + let s = std::str::from_utf8(trimmed).map_err(|e| AlkTypeError::Access { + field_path: field_path.to_string(), + reason: format!("invalid UTF-8 in maxLength string: {e}"), + })?; + Ok(Value::String(s.to_string())) + } + BastType::Primitive(AlkTypeKind::Bytes) => { + let arr: Vec = + trimmed.iter().map(|&byte| Value::from(u32::from(byte))).collect(); + Ok(Value::Array(arr)) + } + _ => Err(AlkTypeError::Schema(format!( + "materialize: maxLength on non-variable kind at {field_path}" + ))), + } + } else { + materialize_leaf_at(doc, ty, field_path, start, endian, buffer) + } + } + } +} + fn materialize_leaf_at( doc: &BastDoc<'_>, ty: &BastType<'_>, @@ -569,9 +682,8 @@ mod tests { fn materialize_packed_strict( doc: &BastDoc<'_>, buffer: &[u8], - endian: Endian, ) -> Result { - materialize_packed(doc, buffer, endian) + materialize_packed(doc, buffer) } #[test] @@ -590,7 +702,7 @@ mod tests { buf[4] = 0x41; buf[5] = 0x42; buf[6] = 0xC3; - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["blob"], json!([65, 66, 195])); } @@ -611,7 +723,7 @@ mod tests { buf[5] = 0xFE; buf[6] = 0x00; buf[7] = 0x80; - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["raw"], json!([255, 254, 0, 128])); } @@ -643,7 +755,7 @@ mod tests { buf[off] = b'a'; off += 1; buf[off..off + 4].copy_from_slice(&20u32.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["counts"], json!({ "b": 10, "a": 20 })); } @@ -662,7 +774,7 @@ mod tests { let doc = doc_from(&root, "S"); let mut buf = vec![0u8; 4]; buf[0..4].copy_from_slice(&0u32.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["counts"], json!({})); } @@ -680,7 +792,7 @@ mod tests { }); let doc = doc_from(&root, "S"); let buf = [0u8; 2]; - let err = materialize_packed_strict(&doc, &buf, Endian::Little).unwrap_err(); + let err = materialize_packed_strict(&doc, &buf).unwrap_err(); assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); } @@ -714,7 +826,7 @@ mod tests { let mut buf = vec![0u8; 5]; buf[0] = 5; buf[1..5].copy_from_slice(&42u32.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); let payload = &v["payload"]; assert_eq!(payload["__discriminator"], json!(5)); assert_eq!(payload["id"], json!(42)); @@ -752,7 +864,7 @@ mod tests { buf[0..4].copy_from_slice(&4u32.to_le_bytes()); buf[4..8].copy_from_slice(b"read"); buf[8..12].copy_from_slice(&7u32.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); let payload = &v["payload"]; assert_eq!(payload["__discriminator"], json!("read")); assert_eq!(payload["type"], json!("read")); @@ -793,7 +905,7 @@ mod tests { buf[4..8].copy_from_slice(b"read"); buf[8..12].copy_from_slice(&7u32.to_le_bytes()); buf[12..16].copy_from_slice(&99u32.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["trailer"], json!(99)); assert_eq!(v["payload"]["n"], json!(7)); } @@ -826,7 +938,7 @@ mod tests { }); let doc = doc_from(&root, "S"); let buf = [99u8]; - let err = materialize_packed_strict(&doc, &buf, Endian::Little).unwrap_err(); + let err = materialize_packed_strict(&doc, &buf).unwrap_err(); assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); } @@ -860,7 +972,7 @@ mod tests { let mut buf = vec![0u8; 5]; buf[0] = 5; buf[1..5].copy_from_slice(&42u32.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["payload"]["__discriminator"], json!(5)); assert_eq!(v["payload"]["id"], json!(42)); } @@ -897,7 +1009,7 @@ mod tests { buf[2..4].copy_from_slice(&2u16.to_le_bytes()); buf[4..6].copy_from_slice(&3u16.to_le_bytes()); buf[6..8].copy_from_slice(&4u16.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["items"], json!([{ "x": 1, "y": 2 }, { "x": 3, "y": 4 }])); } @@ -928,7 +1040,7 @@ mod tests { buf[0] = 1; buf[1] = 2; buf[2..6].copy_from_slice(&0x03030303u32.to_le_bytes()); - let v = materialize_packed_strict(&doc, &buf, Endian::Little).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["header"]["a"], json!(1)); assert_eq!(v["header"]["b"], json!(2)); assert_eq!(v["after"], json!(0x03030303u32)); @@ -950,8 +1062,135 @@ mod tests { }); let doc = doc_from(&root, "S"); let buf = [0u8, 0u8, 0u8, 42u8, 0u8, 0u8, 0u8, 7u8]; - let v = materialize_packed_strict(&doc, &buf, Endian::Big).expect("materialize"); + let v = materialize_packed_strict(&doc, &buf).expect("materialize"); assert_eq!(v["channel_id"], json!(42)); assert_eq!(v["length"], json!(7)); } + + // ----- Aligned-mode materialization (M2 fixes) ---------------------- + + fn materialize_aligned_strict( + doc: &BastDoc<'_>, + buffer: &[u8], + ) -> Result { + let offset_map = crate::offset_map::OffsetMap::compute(doc)?; + materialize_aligned(doc, buffer, &offset_map) + } + + #[test] + fn materialize_aligned_fixed_size_array() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "vals", "kind": { "kind": "array", "element": "uint32", "count": 2 } } + ] + } + } + }); + let doc = doc_from(&root, "S"); + let buf = [1u8, 0, 0, 0, 2, 0, 0, 0]; + let v = materialize_aligned_strict(&doc, &buf).expect("materialize"); + assert_eq!(v["vals"], json!([1, 2])); + } + + #[test] + fn materialize_aligned_max_length_string_trims_padding() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "name", "kind": "string", "maxLength": 8 } + ] + } + } + }); + let doc = doc_from(&root, "S"); + let mut buf = vec![0u8; 8]; + buf[..5].copy_from_slice(b"hello"); + let v = materialize_aligned_strict(&doc, &buf).expect("materialize"); + assert_eq!(v["name"], json!("hello")); + } + + #[test] + fn materialize_aligned_max_length_bytes_trims_padding() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "blob", "kind": "bytes", "maxLength": 8 } + ] + } + } + }); + let doc = doc_from(&root, "S"); + let mut buf = vec![0u8; 8]; + buf[..3].copy_from_slice(&[0xAA, 0xBB, 0xCC]); + let v = materialize_aligned_strict(&doc, &buf).expect("materialize"); + assert_eq!(v["blob"], json!([0xAA, 0xBB, 0xCC])); + } + + #[test] + fn materialize_aligned_offset_indirect_bytes() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "blob", "kind": "bytes", "encoding": "offset-indirect" } + ] + } + } + }); + let doc = doc_from(&root, "S"); + let mut buf = vec![0u8; 8 + 3]; + buf[0..4].copy_from_slice(&8u32.to_le_bytes()); + buf[4..8].copy_from_slice(&3u32.to_le_bytes()); + buf[8..11].copy_from_slice(&[0xAA, 0xBB, 0xCC]); + let v = materialize_aligned_strict(&doc, &buf).expect("materialize"); + assert_eq!(v["blob"], json!([0xAA, 0xBB, 0xCC])); + } + + #[test] + fn materialize_aligned_offset_indirect_string() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "fields": [ + { "name": "name", "kind": "string", "encoding": "offset-indirect" } + ] + } + } + }); + let doc = doc_from(&root, "S"); + let mut buf = vec![0u8; 8 + 5]; + buf[0..4].copy_from_slice(&8u32.to_le_bytes()); + buf[4..8].copy_from_slice(&5u32.to_le_bytes()); + buf[8..13].copy_from_slice(b"hello"); + let v = materialize_aligned_strict(&doc, &buf).expect("materialize"); + assert_eq!(v["name"], json!("hello")); + } + + #[test] + fn materialize_aligned_honors_field_level_endian_override() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "crc", "kind": "uint32", "endian": "little" } + ] + } + } + }); + let doc = doc_from(&root, "S"); + let buf = [0x04u8, 0x03, 0x02, 0x01]; + let v = materialize_aligned_strict(&doc, &buf).expect("materialize"); + assert_eq!(v["crc"], json!(0x01020304u32)); + } } \ No newline at end of file diff --git a/src/sequential_reader.rs b/src/sequential_reader.rs index 90a8460..3d58f3f 100644 --- a/src/sequential_reader.rs +++ b/src/sequential_reader.rs @@ -298,6 +298,7 @@ fn read_field_value<'a>( ) -> Result<(FieldValue<'a>, usize), AlkTypeError> { let ty = field.ty(); let resolved = doc.resolve_typeref(ty)?; + let endian = field.effective_endian(endian); match &resolved { BastType::Primitive(AlkTypeKind::Int8) => { @@ -1002,6 +1003,25 @@ mod tests { assert_eq!(reader.endian(), BE); } + #[test] + fn honors_field_level_endian_override() { + let root = json!({ + "$defs": { + "S": { + "kind": "struct", + "endian": "big", + "fields": [ + { "name": "crc", "kind": "uint32", "endian": "little" } + ] + } + } + }); + let buf = [0x04u8, 0x03, 0x02, 0x01]; + let mut reader = reader(&root, "S"); + let (_, value) = reader.read_next(&buf).unwrap().unwrap(); + assert_eq!(value, FieldValue::U32(0x01020304)); + } + #[test] fn reset_rewinds_cursor() { let root = json!({ diff --git a/tests/error_paths.rs b/tests/error_paths.rs index f9bea6b..3ffd614 100644 --- a/tests/error_paths.rs +++ b/tests/error_paths.rs @@ -414,32 +414,27 @@ fn sequential_reader_new_non_struct_returns_schema_error() { } #[test] -fn read_string_indirect_data_region_too_short_returns_access_error() { +fn read_string_indirect_data_too_short_returns_access_error() { let mut index = [0u8; 8]; let _ = data_access::write_u32(&mut index, 0, 100, "idx.off", Endian::Little); let _ = data_access::write_u32(&mut index, 4, 10, "idx.len", Endian::Little); - let data_region = b"too short"; - let err = data_access::read_bytes_indirect(&index, 0, data_region, "blob", Endian::Little) - .unwrap_err(); + let err = data_access::read_bytes_indirect(&index, 0, "blob", Endian::Little).unwrap_err(); assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); } #[test] fn read_bytes_indirect_index_too_short_returns_access_error() { let buffer = [0u8; 4]; - let data_region = b"anything"; - let err = data_access::read_bytes_indirect(&buffer, 0, data_region, "blob", Endian::Little) - .unwrap_err(); + let err = data_access::read_bytes_indirect(&buffer, 0, "blob", Endian::Little).unwrap_err(); assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); } #[test] fn read_string_indirect_invalid_utf8_returns_access_error() { - let data_region: &[u8] = &[0xFF, 0xFE, 0xFD]; - let mut index = [0u8; 8]; - let _ = data_access::write_u32(&mut index, 0, 0, "idx.off", Endian::Little); - let _ = data_access::write_u32(&mut index, 4, 3, "idx.len", Endian::Little); - let err = data_access::read_string_indirect(&index, 0, data_region, "name", Endian::Little) - .unwrap_err(); + let mut buf = vec![0u8; 8 + 3]; + let _ = data_access::write_u32(&mut buf, 0, 8, "idx.off", Endian::Little); + let _ = data_access::write_u32(&mut buf, 4, 3, "idx.len", Endian::Little); + buf[8..11].copy_from_slice(&[0xFF, 0xFE, 0xFD]); + let err = data_access::read_string_indirect(&buf, 0, "name", Endian::Little).unwrap_err(); assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}"); } \ No newline at end of file