diff --git a/src/api/camera/image_array/client.rs b/src/api/camera/image_array/client.rs index aa095f0..88f9d4e 100644 --- a/src/api/camera/image_array/client.rs +++ b/src/api/camera/image_array/client.rs @@ -83,11 +83,37 @@ impl<'de> Deserialize<'de> for JsonImageArray { } fn cast_raw_data(data: &[u8]) -> Result, PodCastError> { - Ok(bytemuck::try_cast_slice::(data)? - .iter() - .copied() - .map(T::into) - .collect()) + // Fast path: if `data` is already aligned to `align_of::()`, + // `try_cast_slice` reinterprets in place and we iterate-and-widen + // into the output `Vec`. This is the common case — most host + // allocators hand out 16-aligned buffers so the body slice that + // reqwest hands us satisfies a 4-byte alignment requirement. + // + // Slow path: the HTTP response body is a `bytes::Bytes` slice + // from reqwest's chunked read, and its start pointer is not + // *guaranteed* to be `align_of::()`-aligned (4 bytes for + // `i32`, 2 for `i16` / `u16`). When the fast cast fails with + // `TargetAlignmentGreaterAndInputNotAligned`, decode each chunk + // via `T::widen_from_le_chunk` straight into the output + // `Vec` — one pass, no intermediate aligned `Vec`. + // Memory ceiling on the slow path is `sizeof_output` rather + // than `data.len() + sizeof_output`. Length-mismatch / slop + // errors propagate verbatim — only the alignment failure gets + // the unaligned fallback. + match bytemuck::try_cast_slice::(data) { + Ok(aligned) => Ok(aligned.iter().copied().map(T::into).collect()), + Err(PodCastError::TargetAlignmentGreaterAndInputNotAligned) => { + let elem_size = size_of::(); + if !data.len().is_multiple_of(elem_size) { + return Err(PodCastError::OutputSliceWouldHaveSlop); + } + Ok(data + .chunks_exact(elem_size) + .map(T::widen_from_le_chunk) + .collect()) + } + Err(other) => Err(other), + } } impl Response for ASCOMResult { @@ -108,10 +134,17 @@ impl Response for ASCOMResult { }, }); } - let metadata = bytes + let metadata_bytes = bytes .get(..size_of::()) .ok_or_else(|| eyre::eyre!("not enough bytes to read image metadata"))?; - let metadata = bytemuck::try_from_bytes::(metadata)?; + // `bytemuck::try_from_bytes::` requires + // the source slice to be aligned to + // `align_of::()`. The body buffer's start + // pointer is not guaranteed to satisfy that (see + // `cast_raw_data` for the parallel issue); + // `pod_read_unaligned` reads via `ptr::read_unaligned` and + // works for any pointer. + let metadata: ImageBytesMetadata = bytemuck::pod_read_unaligned(metadata_bytes); eyre::ensure!( metadata.metadata_version == 1_i32, "unsupported metadata version {}", @@ -168,3 +201,82 @@ impl Response for ASCOMResult { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Build an `application/imagebytes` payload at the given byte + /// offset within a `Vec`. The `Vec`'s base allocation is + /// usually a high-alignment chunk from the system allocator, so + /// looping `leading_pad` over `0..align_of::()` is sufficient + /// to cover all four `mod 4` cases — at least three of those + /// rotations land the body slice on a non-4-aligned start, which + /// is what reproduces the upstream-pre-fix + /// `TargetAlignmentGreaterAndInputNotAligned` path. + fn imagebytes_payload_at_offset( + leading_pad: usize, + pixels: &[i32], + ) -> eyre::Result<(Vec, std::ops::Range)> { + let metadata = ImageBytesMetadata { + metadata_version: 1_i32, + error_number: 0_i32, + client_transaction_id: None, + server_transaction_id: None, + data_start: i32::try_from(size_of::())?, + image_element_type: i32::from(TransmissionElementType::I32), + transmission_element_type: i32::from(TransmissionElementType::I32), + rank: i32::from(ImageArrayRank::Rank2), + dimension_1: i32::try_from(pixels.len())?, + dimension_2: 1_i32, + dimension_3: 0_i32, + }; + let mut buf = vec![0u8; leading_pad]; + let payload_start = buf.len(); + buf.extend_from_slice(bytemuck::bytes_of(&metadata)); + buf.extend_from_slice(bytemuck::cast_slice(pixels)); + let payload_end = buf.len(); + Ok((buf, payload_start..payload_end)) + } + + /// Regression test for issue #18: parsing an `imagebytes` body + /// whose slice start is not 4-byte aligned must succeed. + /// Pre-fix this path failed with + /// `bytemuck::PodCastError::TargetAlignmentGreaterAndInputNotAligned` + /// because both the metadata `try_from_bytes` and the pixel + /// `try_cast_slice` require source alignment. After the fix — + /// using `pod_read_unaligned` and `pod_collect_to_vec` — the + /// parse is alignment-independent. + fn check_one_offset(leading_pad: usize, pixels: &[i32]) -> eyre::Result<()> { + let (buf, range) = imagebytes_payload_at_offset(leading_pad, pixels)?; + let slice = &buf[range]; + let mime: Mime = IMAGE_BYTES_TYPE.parse()?; + let parsed = as Response>::from_reqwest(mime, slice)?; + let array = parsed + .response + .map_err(|e| eyre::eyre!("ascom error: {e:?}"))?; + eyre::ensure!( + array.shape() == [pixels.len(), 1_usize, 1_usize], + "shape mismatch: {:?}", + array.shape() + ); + for (i, &p) in pixels.iter().enumerate() { + let actual = array[[i, 0_usize, 0_usize]]; + eyre::ensure!(actual == p, "pixel {i} mismatch: got {actual}, expected {p}"); + } + Ok(()) + } + + #[test] + fn from_reqwest_handles_unaligned_imagebytes_slice() { + let pixels: Vec = (0_i32..16_i32).collect(); + // Try every offset 0..4 — at least three of these rotations + // land the body slice on a non-4-aligned start (regardless of + // the `Vec` base alignment), which is the case that fails + // pre-fix. + for leading_pad in 0_usize..4_usize { + check_one_offset(leading_pad, &pixels) + .unwrap_or_else(|e| panic!("leading_pad={leading_pad}: {e:?}")); + } + } +} diff --git a/src/api/camera/image_array/mod.rs b/src/api/camera/image_array/mod.rs index 6ecd71a..7003e15 100644 --- a/src/api/camera/image_array/mod.rs +++ b/src/api/camera/image_array/mod.rs @@ -64,22 +64,53 @@ pub(crate) enum ImageElementType { trait AsTransmissionElementType: 'static + Into + AnyBitPattern { const TYPE: TransmissionElementType; + + /// Read a single little-endian `Self` from a + /// `size_of::()`-byte chunk and widen to `i32`. Used by + /// the unaligned-input slow path of `cast_raw_data` (see + /// `image_array/client.rs`) so the conversion can go from + /// response bytes straight into `Vec` in one pass without + /// staging through an intermediate aligned `Vec`. + /// Callers must feed exactly `size_of::()` bytes (e.g. via + /// `data.chunks_exact(size_of::())`). + fn widen_from_le_chunk(chunk: &[u8]) -> i32; } impl AsTransmissionElementType for i16 { const TYPE: TransmissionElementType = TransmissionElementType::I16; + + fn widen_from_le_chunk(chunk: &[u8]) -> i32 { + i32::from(Self::from_le_bytes([chunk[0_usize], chunk[1_usize]])) + } } impl AsTransmissionElementType for i32 { const TYPE: TransmissionElementType = TransmissionElementType::I32; + + fn widen_from_le_chunk(chunk: &[u8]) -> i32 { + Self::from_le_bytes([ + chunk[0_usize], + chunk[1_usize], + chunk[2_usize], + chunk[3_usize], + ]) + } } impl AsTransmissionElementType for u16 { const TYPE: TransmissionElementType = TransmissionElementType::U16; + + fn widen_from_le_chunk(chunk: &[u8]) -> i32 { + i32::from(Self::from_le_bytes([chunk[0_usize], chunk[1_usize]])) + } } impl AsTransmissionElementType for u8 { const TYPE: TransmissionElementType = TransmissionElementType::U8; + + fn widen_from_le_chunk(chunk: &[u8]) -> i32 { + i32::from(chunk[0_usize]) + } } /// Image array.