fix(client): handle unaligned ImageBytes response buffers (#19)

* fix(client): handle unaligned ImageBytes response buffers

`ASCOMResult<ImageArray>::from_reqwest` reads the 44-byte
`ImageBytesMetadata` header via `bytemuck::try_from_bytes` and the
pixel payload via `bytemuck::try_cast_slice::<u8, T>`. Both APIs
require the source `&[u8]` to be aligned to the target type's
alignment (4 for `i32`, 2 for `i16`/`u16`). The `&[u8]` here is a
slice of the HTTP response body — typically a `bytes::Bytes` chunk
that reqwest assembled from one or more chunked reads — and its
start pointer is not guaranteed to satisfy that alignment. When it
doesn't, both calls return
`PodCastError::TargetAlignmentGreaterAndInputNotAligned` and the
client panics with `ASCOM error UNSPECIFIED:
TargetAlignmentGreaterAndInputNotAligned` — intermittently,
depending on the host allocator's runtime behaviour.

Fix: use the unaligned-friendly equivalents.

- Metadata: `bytemuck::pod_read_unaligned` reads the full struct via
  `ptr::read_unaligned`, no source-alignment requirement.
- Pixel data: `bytemuck::pod_collect_to_vec` allocates a fresh
  `Vec<T>` (which IS aligned) and `memcpy`s the payload in. Adds an
  upfront length-modulo check to keep the existing slop-error
  semantics.

Adds a regression test that constructs an `imagebytes` payload at
each of the four leading-pad offsets (0..4) within a `Vec<u8>`,
guaranteeing at least three of them place the body slice on a
non-4-aligned start. The test fails on the previous code path
(`try_from_bytes` rejects the unaligned metadata immediately) and
passes after the fix.

Closes #18.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: address clippy + Copilot feedback

- cast_raw_data: keep the zero-copy fast path. Try
  `bytemuck::try_cast_slice::<u8, T>` first; only fall back to the
  copying `pod_collect_to_vec` when it returns
  `TargetAlignmentGreaterAndInputNotAligned`. Slop / length-mismatch
  errors propagate verbatim. Common (aligned) case keeps the
  pre-PR memory and copy profile; unaligned case still works.
- Drop redundant `if !data.len().is_multiple_of(elem_size)` length
  check now that the slop error path is reached via try_cast_slice
  directly.
- Metadata fix doc comment: refer to
  `align_of::<ImageBytesMetadata>()` instead of the hard-coded
  `align_of::<i32>() == 4` so it stays correct if the struct's
  layout grows a wider field.
- Test helper doc comment: clarify the alignment guarantee — the
  `Vec<u8>` base allocation is high-aligned by the system
  allocator, so looping leading_pad over 0..4 covers all four mod-4
  rotations and guarantees at least three unaligned cases.
- Test fn signature: split into a `Result<()>`-returning helper
  (`check_one_offset`) using `eyre::ensure!` for validation and a
  panicking `#[test]` shell that surfaces the failing leading_pad
  in the panic message. Avoids clippy::panic_in_result_fn while
  keeping diagnostic context.
- Replace `data.len() % elem_size != 0` with the
  `is_multiple_of` form (clippy::manual_is_multiple_of),
  superseded by the fast-path refactor above.
- Clean up `as i32` casts in the test helper to `i32::from(...)`
  for the enum reprs and `i32::try_from(...)?` for the
  size_of/len conversions (clippy::cast_lossless / cast_possible_truncation).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* perf(client): fuse cast_raw_data slow path into single-pass widen

Replaces the slow path's two-memcpy bytes→Vec<T>→Vec<i32> staging
with a single chunked pass that decodes each `size_of::<T>()`-byte
chunk via a new `widen_from_le_chunk` trait method on
`AsTransmissionElementType` and writes straight into the output
`Vec<i32>`.

Savings:

- One full pixel-buffer memcpy on unaligned input.
- Memory ceiling on the slow path drops from
  `data.len() + sizeof_output` to just `sizeof_output`. For a
  32 MP `i32` image that's ~128 MB peak instead of ~256 MB.
- Drops the `bytemuck::NoUninit` bound on the local function (was
  only required by `pod_collect_to_vec`).

The fast (aligned) path is unchanged from the previous commit —
`try_cast_slice` reinterprets in place, and we iterate-and-widen
into the output. So the common case keeps the pre-PR memory and
copy profile; the slow path is now strictly an improvement.

`from_le_bytes` on `i32` lowers to the same instruction as a normal
aligned `i32` load on little-endian targets, so the per-element
loop in the slow path is essentially identical in cost to a bulk
memcpy + widening loop. For the smaller types (`i16` / `u16` /
`u8`) the upconversion was element-wise anyway, so the staging
buffer was pure overhead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: use as_chunks with a fixed-size &[u8; N] widening trait

Address RReverser's review on the cast_raw_data unaligned fallback: swap
chunks_exact for <[u8]>::as_chunks::<N>() and move the per-element
little-endian widening onto a companion WidenLeChunk<const N> trait whose
method takes a fixed-size &[u8; N]. This ties the byte-chunk width to the
element type at the type level (i16/u16 => 2, i32 => 4, u8 => 1) and drops
the per-pixel bounds checks; as_chunks' remainder replaces the manual
is_multiple_of slop check while preserving the OutputSliceWouldHaveSlop
guarantee. The aligned try_cast_slice fast path is unchanged.

WidenLeChunk is gated behind #[cfg(feature = "client")] since it is only
used by the client-gated cast_raw_data. Also reword a stale test doc
comment that referenced pod_collect_to_vec.

Verified: from_reqwest_handles_unaligned_imagebytes_slice passes; clippy
-D warnings clean on --all-features and on the client,camera /
server,camera feature combos.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* review: fold unaligned widening onto AsTransmissionElementType

Per maintainer feedback, replace the separate `WidenLeChunk<const N>`
trait with a single client-gated `widen_unaligned_le` method on the
existing `AsTransmissionElementType` trait, and drop the const generic
from `cast_raw_data` (call sites are now `cast_raw_data::<i16>` etc.).

Each impl hard-codes `as_chunks::<N>()` to its own byte width, so the
slow path still decodes fixed-size `&[u8; N]` chunks with no per-pixel
bounds checks. The "obvious" form (an associated `const WIDTH` driving
`&[u8; Self::WIDTH]` / `as_chunks::<{ T::WIDTH }>()`) needs the unstable
`generic_const_exprs` feature, so the explicit-width method is the
stable-toolchain realization of the same intent.

No behavioural change: the aligned fast path is byte-for-byte identical,
and the slow path preserves the exact error semantics (a non-empty
`as_chunks` remainder -> `OutputSliceWouldHaveSlop`; only the alignment
failure falls back; all other `PodCastError`s propagate verbatim).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* review: collapse unaligned widening into one default method

Fold the four near-identical `widen_unaligned_le` impls into a single
default method on `AsTransmissionElementType`, reading each pixel with
`bytemuck::pod_read_unaligned` over `chunks_exact(size_of::<Self>())`
instead of per-type `as_chunks::<N>` + `from_le_bytes`. The four impls
shrink to just `const TYPE`.

Drop the redundant `'static` supertrait (already implied by
`AnyBitPattern`) and rename the method to `widen_unaligned`: the read is
host-endian, and little-endianness is guaranteed by the crate's
little-endian-only `compile_error!` guard rather than by this function.

The one-time `chunks_exact` remainder check preserves the
`OutputSliceWouldHaveSlop` guarantee and keeps the per-element read's
length-mismatch panic unreachable, so the output `Vec<i32>` is still
built in a single exact-capacity pass. Add a test covering sign/zero
extension across all four element widths, which the i32-only regression
test did not exercise.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Associate types with fixed-size arrays

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Ingvar Stepanyan <me@rreverser.com>

Author: 3 people

Cargo.toml

@@ -36,6 +36,7 @@ axum = { version = "0.8.8", optional = true }
 bytemuck = { version = "1.24.0", features = [
 	"derive",
 	"extern_crate_std",
+	"must_cast",
 ], optional = true }
 bytes = { version = "1.10.1", optional = true }
 criterion = { version = "0.8.1", optional = true }

src/api/camera/image_array/client.rs

@@ -1,15 +1,15 @@
 use super::{
-    AsTransmissionElementType, ImageArray, ImageArrayRank, ImageBytesMetadata, ImageElementType,
-    TransmissionElementType, COLOUR_AXIS, IMAGE_BYTES_TYPE,
+    AsTransmissionElementType, COLOUR_AXIS, IMAGE_BYTES_TYPE, ImageArray, ImageArrayRank,
+    ImageBytesMetadata, ImageElementType, TransmissionElementType,
 };
 use crate::client::{Response, ResponseTransaction, ResponseWithTransaction};
 use crate::{ASCOMError, ASCOMErrorCode, ASCOMResult};
 use bytemuck::PodCastError;
 use mime::Mime;
 use ndarray::{Array2, Array3};
 use num_enum::TryFromPrimitive;
-use serde::de::{DeserializeOwned, IgnoredAny, MapAccess, Visitor};
 use serde::Deserialize;
+use serde::de::{DeserializeOwned, IgnoredAny, MapAccess, Visitor};
 use serde_ndim::de::MakeNDim;
 use std::fmt::{self, Formatter};
 
@@ -83,11 +83,39 @@ impl<'de> Deserialize<'de> for JsonImageArray {
 }
 
 fn cast_raw_data<T: AsTransmissionElementType>(data: &[u8]) -> Result<Vec<i32>, PodCastError> {
-    Ok(bytemuck::try_cast_slice::<u8, T>(data)?
-        .iter()
-        .copied()
-        .map(T::into)
-        .collect())
+    // Fast path: if `data` is already aligned to `align_of::<T>()`,
+    // `try_cast_slice` reinterprets in place and we iterate-and-widen
+    // into the output `Vec<i32>`. 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::<T>()`-aligned (4 bytes for
+    // `i32`, 2 for `i16` / `u16`). When the fast cast fails with
+    // `TargetAlignmentGreaterAndInputNotAligned`, hand the bytes to
+    // `T::widen_unaligned`, which splits `data` into
+    // `size_of::<T>()`-wide chunks and reads each one with
+    // `bytemuck::pod_read_unaligned` straight into the output
+    // `Vec<i32>` — one pass, no intermediate aligned `Vec<T>`. A
+    // non-empty `chunks_exact` remainder reproduces the
+    // `OutputSliceWouldHaveSlop` guarantee. Other cast errors
+    // propagate verbatim — only the alignment failure falls back.
+    match bytemuck::try_cast_slice::<u8, T>(data) {
+        Ok(aligned) => Ok(aligned.iter().copied().map(T::into).collect()),
+        Err(PodCastError::TargetAlignmentGreaterAndInputNotAligned) => {
+            // Read as unaligned byte blobs of the same size as `T`.
+            Ok(bytemuck::try_cast_slice::<u8, T::Bytes>(data)?
+                .iter()
+                .copied()
+                // Then reinterpret each byte blob as `T`.
+                .map(bytemuck::must_cast)
+                // And convert to the target `i32`.
+                .map(T::into)
+                .collect())
+        }
+        Err(other) => Err(other),
+    }
 }
 
 impl Response for ASCOMResult<ImageArray> {
@@ -108,10 +136,17 @@ impl Response for ASCOMResult<ImageArray> {
                 },
             });
         }
-        let metadata = bytes
+        let metadata_bytes = bytes
             .get(..size_of::<ImageBytesMetadata>())
             .ok_or_else(|| eyre::eyre!("not enough bytes to read image metadata"))?;
-        let metadata = bytemuck::try_from_bytes::<ImageBytesMetadata>(metadata)?;
+        // `bytemuck::try_from_bytes::<ImageBytesMetadata>` requires
+        // the source slice to be aligned to
+        // `align_of::<ImageBytesMetadata>()`. 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 +203,86 @@ impl Response for ASCOMResult<ImageArray> {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Build an `application/imagebytes` payload at the given byte
+    /// offset within a `Vec<u8>`. The `Vec`'s base allocation is
+    /// usually a high-alignment chunk from the system allocator, so
+    /// looping `leading_pad` over `0..align_of::<i32>()` 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<u8>, std::ops::Range<usize>)> {
+        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::<ImageBytesMetadata>())?,
+            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 —
+    /// metadata via `pod_read_unaligned`, and pixels via an aligned
+    /// `try_cast_slice` fast path with a `pod_read_unaligned`
+    /// fallback — 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 = <ASCOMResult<ImageArray> 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<i32> = (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<u8>` 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:?}"));
+        }
+    }
+}

src/api/camera/image_array/mod.rs

@@ -62,24 +62,30 @@ pub(crate) enum ImageElementType {
     I32 = 2,
 }
 
-trait AsTransmissionElementType: 'static + Into<i32> + AnyBitPattern {
+trait AsTransmissionElementType: Into<i32> + AnyBitPattern {
     const TYPE: TransmissionElementType;
+    type Bytes: Pod;
 }
 
 impl AsTransmissionElementType for i16 {
     const TYPE: TransmissionElementType = TransmissionElementType::I16;
+    type Bytes = [u8; size_of::<Self>()];
 }
 
 impl AsTransmissionElementType for i32 {
     const TYPE: TransmissionElementType = TransmissionElementType::I32;
+    type Bytes = [u8; size_of::<Self>()];
 }
 
 impl AsTransmissionElementType for u16 {
     const TYPE: TransmissionElementType = TransmissionElementType::U16;
+    type Bytes = [u8; size_of::<Self>()];
 }
 
 impl AsTransmissionElementType for u8 {
     const TYPE: TransmissionElementType = TransmissionElementType::U8;
+    #[expect(clippy::use_self)]
+    type Bytes = [u8; size_of::<Self>()];
 }
 
 /// Image array.