fix(messages): derive the declared payload length when encoding a message

ARCHITECTURE.md

@@ -576,6 +576,18 @@ one TCP connection at a time; entity status and vehicle identification requests
 over TCP are silently dropped; the handler passed to `Server::new` is not
 validated.
 
+`Message::decode` still accepts a header whose `payload_length` disagrees with
+what the payload actually occupies -- it hands the payload exactly that many
+bytes and does not require them all to be consumed. `Message::encode` no longer
+propagates such a length (it derives the field), so the frame it emits is always
+self-consistent, but the lenient decode remains. ISO 13400-2 has an entity
+answer an invalid payload length with NACK `0x04`, and
+`MessageError::PayloadLengthTooShort` exists, unused, for exactly this. Making
+the decode strict is a redesign rather than a fix: `Payload::decode` would have
+to report unconsumed bytes, and the identification requests deliberately
+discard their EID/VIN body, so a `0x0002` request carrying its 6 EID bytes
+would start being rejected outright rather than declined.
+
 `RoutingActivationRequest::encode` omits the optional vehicle-manufacturer
 field when it is `None`, writing 7 bytes instead of 11. That is what the
 optionality means, and every golden vector agrees — but no vector exercises a

CHANGELOG.md

@@ -11,10 +11,13 @@ This file was reconstructed from the commit and pull-request history when the
 crate was prepared for publication, so entries before that point describe what
 changed rather than what was announced at the time.
 
-## [Unreleased]
+## [0.6.0] — 2026-09-10
 
 ### Added
 
+- `MessageError::PayloadTooLarge`, for a payload that cannot be described by
+  the `u32` `payload_length` field. `MessageError` is `#[non_exhaustive]`, so
+  this is not a breaking change.
 - `LICENSE-MIT` and `LICENSE-APACHE`. The manifest had declared
   `MIT OR Apache-2.0` without carrying either text.
 - `CHANGELOG.md`, `CONTRIBUTING.md`, `SECURITY.md`.
@@ -25,6 +28,19 @@ changed rather than what was announced at the time.
 
 ### Fixed
 
+- **Breaking:** `Message::encode` derives the header's `payload_length` from
+  the payload instead of writing the field verbatim. A decoded message keeps
+  whatever length the wire declared, and `Payload::decode` need not consume
+  all of it, so re-encoding a frame that arrived with a mismatched length
+  emitted a frame no decoder would accept. Anything that decodes and re-emits
+  -- a proxy, a replay tool, a logging fake -- was turning a
+  malformed-but-accepted frame into a corrupt one. Nothing stops compiling;
+  what changes is the bytes emitted, which is why this is a minor bump and
+  not a patch. For such a frame,
+  `decode(encode(m)).header.payload_length` is now the payload's real size
+  rather than the length it arrived with.
+- `MessageError::PayloadLengthTooShort`'s message said "does match" where it
+  meant "does not match".
 - `ClientConnectionInfo::logical_address` carries the address the tester
   activated routing with, instead of always being `0x0000`. A handler can now
   tell which tester is asking, and the default `alive_check` answers with the
@@ -117,9 +133,10 @@ changed rather than what was announced at the time.
 Initial implementation: DoIP message types, framing, and an async client and
 server over tokio.
 
-<!-- Only v0.1.0 and v0.5.1 were ever tagged, so the intermediate versions
-     have no comparison range to link. -->
+<!-- Only v0.1.0, v0.5.1 and v0.5.2 were ever tagged, so the intermediate
+     versions have no comparison range to link. -->
 
-[Unreleased]: https://github.com/luminartech/simple_doip/compare/v0.5.1...HEAD
+[0.6.0]: https://github.com/luminartech/simple_doip/compare/v0.5.2...v0.6.0
+[0.5.2]: https://github.com/luminartech/simple_doip/compare/v0.5.1...v0.5.2
 [0.5.1]: https://github.com/luminartech/simple_doip/compare/v0.1.0...v0.5.1
 [0.1.0]: https://github.com/luminartech/simple_doip/releases/tag/v0.1.0

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "simple_doip"
-version = "0.5.2"
+version = "0.6.0"
 edition = "2024"
 rust-version = "1.88"
 description = "An ISO 13400-2 (DoIP) implementation with a no_std, zero-copy protocol core and optional async client and server"

fuzz/fuzz_targets/fuzz_roundtrip.rs

@@ -1,7 +1,7 @@
 #![no_main]
 
 use libfuzzer_sys::fuzz_target;
-use simple_doip::messages::{Decode, Encode, Header, Message};
+use simple_doip::messages::{Decode, Encode, Message};
 
 // The asymmetry hunt: if a frame decodes, re-encoding it and decoding that
 // again must yield the same message.
@@ -19,16 +19,6 @@ fuzz_target!(|data: &[u8]| {
     let Ok(size) = message.encoded_size() else {
         return;
     };
-
-    // Skip frames whose header declares a length the payload does not actually
-    // occupy. `decode` accepts those, keeps the declared length verbatim, and
-    // `encode` then writes it beside a payload of the real size -- emitting a
-    // frame no decoder will accept. That is luminartech/simple_doip#15, not an
-    // asymmetry in the field order this target is hunting for, and the check
-    // comes out when #15 is fixed.
-    if message.header.payload_length as usize != size - Header::SIZE {
-        return;
-    }
     let mut encoded = vec![0u8; size];
     {
         let mut writer: &mut [u8] = &mut encoded;
@@ -40,5 +30,30 @@ fuzz_target!(|data: &[u8]| {
     let (reparsed, rest) =
         Message::decode(&encoded).expect("re-decoding an encoded message must not fail");
     assert!(rest.is_empty(), "re-encoding left trailing bytes");
-    assert_eq!(message, reparsed, "round trip changed the message");
+
+    // The payload, not the whole message: `encode` derives the header's
+    // declared length from the payload, so a frame that arrived claiming a
+    // length its payload did not occupy comes back with the real one. That
+    // normalization is the fix for #15 -- asserting full `Message` equality
+    // here would assert the bug back into existence.
+    assert_eq!(
+        message.payload, reparsed.payload,
+        "round trip changed the payload"
+    );
+    assert_eq!(
+        message.header.payload_type, reparsed.header.payload_type,
+        "round trip changed the payload type"
+    );
+
+    // Encoding is idempotent: having normalized once, a second pass must
+    // produce the very same bytes. This is the property that would catch a
+    // field order asymmetry, now that the length no longer masks it.
+    let mut again = vec![0u8; reparsed.encoded_size().expect("size of a decoded message")];
+    {
+        let mut writer: &mut [u8] = &mut again;
+        reparsed
+            .encode(&mut writer)
+            .expect("re-encode must not fail");
+    }
+    assert_eq!(encoded, again, "encoding is not idempotent");
 });

src/messages/message_error.rs

@@ -43,14 +43,26 @@ pub enum MessageError {
     ///
     /// Currently never produced by this crate.
     #[error(
-        "Payload length in header does match expected payload type length: {value:?}, expected: {expected:?}"
+        "Payload length in header does not match expected payload type length: {value:?}, expected: {expected:?}"
     )]
     PayloadLengthTooShort {
         /// The payload length actually declared in the header.
         value: usize,
         /// The minimum payload length required for the header's `payload_type`.
         expected: u32,
     },
+    /// The payload encodes to more bytes than the header's `payload_length`
+    /// field can describe. `DoIP` declares that length as a `u32`, so such a
+    /// message has no valid wire form. Recoverable: nothing was written.
+    ///
+    /// Unreachable for a frame that came off the wire, whose length was itself
+    /// a `u32`; it takes a hand-built [`Payload`](crate::messages::Payload)
+    /// borrowing more than `u32::MAX` bytes.
+    #[error("Payload of {size} bytes exceeds the u32 payload_length field")]
+    PayloadTooLarge {
+        /// The payload's encoded size.
+        size: usize,
+    },
     /// A structurally valid, supported [`PayloadType`] was received in a context
     /// where it is not a legal response (e.g. a request-only type arriving as a
     /// response). Recoverable: the frame itself decoded fine.

src/messages/mod.rs

@@ -300,12 +300,34 @@ impl Encode for Message<'_> {
         Ok(Header::SIZE + self.payload.encoded_size()?)
     }
 
-    /// Serialize this message (header + payload) into `writer`
+    /// Serialize this message (header + payload) into `writer`.
+    ///
+    /// The header's `payload_length` is **derived from the payload** rather
+    /// than taken from `self.header`. A decoded message keeps whatever length
+    /// the wire declared, and [`Payload::decode`] is not required to consume
+    /// all of it -- so writing that field verbatim can emit a frame whose
+    /// declared length no decoder, including this one, will accept. Deriving
+    /// it means an encoded frame is always self-consistent.
+    ///
+    /// The visible consequence: for a frame that arrived with a declared
+    /// length the payload did not occupy, `decode(encode(m)).header
+    /// .payload_length` is the payload's real size, not the length `m`
+    /// arrived with.
     ///
     /// # Errors
-    /// Returns a [`MessageError`] if the header or payload cannot be serialized
+    /// Returns a [`MessageError`] if the header or payload cannot be
+    /// serialized, or [`MessageError::PayloadTooLarge`] if the payload does
+    /// not fit the `u32` length field.
     fn encode(&self, writer: &mut impl embedded_io::Write) -> Result<usize, MessageError> {
-        let written = self.header.encode(writer)?;
+        let payload_length = self.payload.encoded_size()?;
+        let header = Header::new(
+            self.header.protocol_version,
+            self.header.payload_type,
+            u32::try_from(payload_length).map_err(|_| MessageError::PayloadTooLarge {
+                size: payload_length,
+            })?,
+        );
+        let written = header.encode(writer)?;
         Ok(written + self.payload.encode(writer)?)
     }
 }

tests/encode_consistency.rs

@@ -0,0 +1,77 @@
+//! `Message::encode` must never emit a frame `Message::decode` rejects.
+//!
+//! Regression tests for the asymmetry the `fuzz_roundtrip` target found:
+//! `decode` takes exactly `header.payload_length` bytes and lets
+//! `Payload::decode` consume fewer without complaint, so a decoded `Message`
+//! could carry a declared length its payload did not occupy. `encode` wrote
+//! that stale field verbatim, producing a frame that failed to decode with
+//! `Incomplete`.
+//!
+//! Both inputs below are frames a peer can actually send. Neither is
+//! well-formed, but both are accepted, and being accepted is what put them on
+//! the re-encode path.
+
+use simple_doip::messages::{Decode, Encode, Message, Payload, PayloadType};
+
+/// Encode `message` into a fresh buffer sized by `encoded_size`.
+fn encode(message: &Message<'_>) -> Vec<u8> {
+    let mut buf = vec![0u8; message.encoded_size().expect("encoded_size failed")];
+    {
+        let mut writer: &mut [u8] = &mut buf;
+        message.encode(&mut writer).expect("encode failed");
+    }
+    buf
+}
+
+/// A NACK body is one byte. This frame's header claims five.
+#[test]
+fn a_fixed_size_payload_with_an_overlong_declared_length_still_round_trips() {
+    let framed: [u8; 13] = [
+        0x02, 0xFD, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x03, 0x00, 0x00, 0x00, 0x00,
+    ];
+    let (message, _rest) = Message::decode(&framed).expect("the frame is accepted as-is");
+    assert_eq!(
+        message.header.payload_length, 5,
+        "decode still reports what the wire declared"
+    );
+
+    let encoded = encode(&message);
+    let (reparsed, rest) = Message::decode(&encoded).expect("re-decode must succeed");
+    assert!(rest.is_empty());
+    assert_eq!(
+        reparsed.header.payload_length, 1,
+        "the encoded frame declares the length its payload actually occupies"
+    );
+    assert_eq!(message.payload, reparsed.payload);
+}
+
+/// `VehicleIdentificationRequest` is a unit variant -- an empty body. This
+/// frame's header claims one byte, which `Payload::decode` discards.
+#[test]
+fn a_unit_payload_with_a_nonzero_declared_length_still_round_trips() {
+    let framed: [u8; 9] = [0x00, 0xFF, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00];
+    let (message, _rest) = Message::decode(&framed).expect("the frame is accepted as-is");
+    assert_eq!(message.payload, Payload::VehicleIdentificationRequest);
+    assert_eq!(message.header.payload_length, 1);
+
+    let encoded = encode(&message);
+    assert_eq!(encoded.len(), 8, "a unit payload encodes to a bare header");
+    let (reparsed, rest) = Message::decode(&encoded).expect("re-decode must succeed");
+    assert!(rest.is_empty());
+    assert_eq!(reparsed.header.payload_length, 0);
+    assert_eq!(
+        reparsed.header.payload_type,
+        PayloadType::VehicleIdentificationRequest
+    );
+}
+
+/// A well-formed frame is untouched: the derived length equals the declared
+/// one, so the bytes are identical. This is what keeps the golden vectors
+/// valid.
+#[test]
+fn a_well_formed_frame_encodes_to_the_bytes_it_came_from() {
+    let framed: [u8; 9] = [0x02, 0xFD, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03];
+    let (message, rest) = Message::decode(&framed).expect("decode");
+    assert!(rest.is_empty());
+    assert_eq!(encode(&message), framed);
+}