EVCacheValueSerde: drop bodyLength from the wire format

The bodyLength field was meant to serve two purposes — additive
forward-compat (skip extension bytes past the known fields) and
corruption detection (reject trailing bytes). Those goals conflict:
extension bytes and under-framed length prefixes are observationally
identical to the reader, so the design could only catch one and ended up
catching the cosmetic one — trailing bytes the serializer cannot produce
by construction — while letting length-prefix under-framing slip
through unchecked.

Removing bodyLength entirely. End-of-envelope is now implicit at
bytes.length. Newer writers append optional fields after createTime;
older readers stop after the known fields and leave any remaining
bytes unread. Required fields still need the version-byte rollout
documented in the class javadoc.

Tests: the two forward-compat tests (v0 pinned-payload trip-wire and
future-extension-bytes) refrozen against the simpler format and both
pass; the bodyLength-specific corruption tests are removed
(testDecodeBinaryWithBogusBodyLengthReturnsNull,
testDecodeBinaryWithNegativeBodyLengthReturnsNull,
testDecodeBinaryWithTrailingBytesBeyondBodyReturnsNull). The remaining
bounds-check tests are simplified to drop the no-longer-present bodyLength
prefix from their crafted bytes.

Bit-corruption detection for value bytes / length-prefix tampering is
deferred to a follow-up — it will not be bodyLength.

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

Author: Joe Lee

evcache-core/src/main/java/com/netflix/evcache/pool/EVCacheValueSerde.java

@@ -17,21 +17,20 @@
  *
  * <pre>
  * [byte 0: magic 0x0C][byte 1: reserved/version 0x00]
- * [int bodyLength]
  * [int keyLen][key UTF-8 bytes]
  * [int valLen][value bytes]
  * [int flags][long ttl][long createTime]
- * [... extension fields ...]
+ * [... optional extension fields appended by newer writers ...]
  * </pre>
  *
  * <ul>
  *   <li><b>Magic {@code 0x0C}</b> disambiguates from Java {@code ObjectOutputStream} (starts
  *       {@code 0xAC 0xED}); callers route via {@link #isBinaryFormat(byte[])}.</li>
  *   <li><b>Reserved/version byte</b> is currently {@code 0x00}, read-and-ignored. Bump only
  *       for breaking changes (see Upgrades).</li>
- *   <li><b>bodyLength</b> = byte count of everything that follows it. Enables additive forward
- *       compatibility — older readers skip any unread bytes inside the declared body, returning
- *       the EVCacheValue normally.</li>
+ *   <li><b>End of envelope</b> is implicit at {@code bytes.length}. There is no declared body
+ *       length on the wire; bytes past the last known field are treated as extension data for
+ *       additive forward-compat (see Upgrades).</li>
  *   <li><b>Byte order:</b> big-endian / network, set explicitly on both sides.</li>
  *   <li><b>Error contract:</b> any corrupt/truncated input returns {@code null} after a WARN
  *       log identifying the failing field and a (truncated) hex dump of the bytes. Matches
@@ -40,12 +39,13 @@
  *
  * <h2>Upgrades</h2>
  *
- * <p><b>Additive optional (non-breaking).</b> Append a new field at the end of the body; the
- * writer grows {@code bodyLength} to include it. Newer readers MUST gate each added field
- * with {@code buffer.position() < bodyEnd} and supply a default when absent — a new reader
- * will encounter items written by an old writer (in cache until TTL expires) that don't
- * contain the field. Only works when a graceful default exists. A new <i>required</i> field
- * has no acceptable default and is therefore Breaking, not additive.
+ * <p><b>Additive optional (non-breaking).</b> Append a new field at the end of the envelope,
+ * after {@code createTime}. Older readers stop after the known fields and never look at the
+ * extension bytes. Newer readers MUST gate each added field with {@code buffer.hasRemaining()}
+ * and supply a default when absent — they will encounter items written by old writers
+ * (in cache until TTL expires) that don't contain the field. Only works when a graceful
+ * default exists. A new <i>required</i> field has no acceptable default and is therefore
+ * Breaking, not additive.
  *
  * <p><b>Breaking</b> (field reorder, type widen, semantic change, new required field):
  * rollout MUST be <i>reader-before-writer</i> — items written by an early writer would be
@@ -89,19 +89,17 @@ public static byte[] serialize(EVCacheValue v) {
         final byte[] keyBytes = v.getKey().getBytes(StandardCharsets.UTF_8);
         final byte[] valueBytes = v.getValue();
 
-        final int bodyLength =
-                Integer.BYTES + keyBytes.length      // keyLen + key
-              + Integer.BYTES + valueBytes.length    // valLen + value
-              + Integer.BYTES                        // flags
-              + Long.BYTES                           // ttl
-              + Long.BYTES;                          // createTime
-
-        final int bufferSize = Byte.BYTES + Byte.BYTES + Integer.BYTES + bodyLength;
+        final int bufferSize =
+                Byte.BYTES + Byte.BYTES                // magic + reserved/version
+              + Integer.BYTES + keyBytes.length        // keyLen + key
+              + Integer.BYTES + valueBytes.length      // valLen + value
+              + Integer.BYTES                          // flags
+              + Long.BYTES                             // ttl
+              + Long.BYTES;                            // createTime
         final ByteBuffer buffer = ByteBuffer.allocate(bufferSize).order(ByteOrder.BIG_ENDIAN);
 
         buffer.put(BINARY_SERDE_MAGIC_CONSTANT_BYTE);
         buffer.put(RESERVED_VERSION_BYTE);
-        buffer.putInt(bodyLength);
 
         buffer.putInt(keyBytes.length);
         buffer.put(keyBytes);
@@ -115,10 +113,10 @@ public static byte[] serialize(EVCacheValue v) {
     }
 
     /**
-     * Decode the binary envelope. Length prefixes are bounds-checked before allocation; corrupt
-     * or truncated payloads return {@code null} after a WARN log identifying the failing field.
-     * Extension bytes appended by newer writers (inside {@code bodyLength}) are silently skipped;
-     * bytes beyond the declared body are treated as corruption.
+     * Decode the binary envelope. Length prefixes are bounds-checked before allocation. A
+     * truncated or malformed payload returns {@code null} after a WARN log identifying the
+     * failing field. Bytes remaining past the known fields are not read — they're reserved for
+     * additive extension fields appended by newer writers (see Upgrades).
      */
     public static EVCacheValue deserialize(byte[] bytes) {
         String field = "magic";
@@ -133,18 +131,6 @@ public static EVCacheValue deserialize(byte[] bytes) {
             field = "reserved";
             buffer.get();
 
-            field = "bodyLength";
-            final int bodyLength = buffer.getInt();
-            if (bodyLength < 0 || bodyLength > buffer.remaining()) {
-                logCorruption(bytes, "Invalid bodyLength: " + bodyLength + ", remaining=" + buffer.remaining());
-                return null;
-            }
-            // Cap reads to the declared body. Field reads that would overrun are caught as
-            // BufferUnderflowException; extension bytes are skipped explicitly below.
-            final int originalLimit = buffer.limit();
-            final int bodyEnd = buffer.position() + bodyLength;
-            buffer.limit(bodyEnd);
-
             field = "keyLength";
             final int keyLength = buffer.getInt();
             if (keyLength < 0 || keyLength > buffer.remaining()) {
@@ -173,15 +159,8 @@ public static EVCacheValue deserialize(byte[] bytes) {
             field = "createTime";
             final long createTime = buffer.getLong();
 
-            // Skip any forward-compat extension bytes inside the declared body.
-            buffer.position(bodyEnd);
-
-            // Bytes beyond the declared body weren't claimed by the writer — corruption.
-            buffer.limit(originalLimit);
-            if (buffer.remaining() != 0) {
-                logCorruption(bytes, "trailing " + buffer.remaining() + " bytes beyond declared bodyLength");
-                return null;
-            }
+            // Any remaining bytes are forward-compat extension fields a newer writer appended;
+            // an older reader (this one) leaves them unread.
 
             return new EVCacheValue(key, valueBytes, flags, ttl, createTime);
         } catch (BufferUnderflowException e) {

evcache-core/src/test/java/com/netflix/evcache/pool/EVCacheValueSerdeTest.java

@@ -220,73 +220,32 @@ public void testDecodeTruncatedBinaryReturnsNull() {
     }
 
     @Test
-    public void testDecodeBinaryWithBogusBodyLengthReturnsNull() {
-        // Magic + reserved + wildly oversized bodyLength. Bounds check rejects.
+    public void testDecodeBinaryWithBogusKeyLengthReturnsNull() {
+        // Magic + reserved + wildly oversized keyLength. Bounds check rejects.
         byte[] bytes = new byte[2 + Integer.BYTES];
         bytes[0] = EVCacheValueSerde.BINARY_SERDE_MAGIC_CONSTANT_BYTE;
         bytes[1] = 0x00;
-        bytes[2] = (byte) 0x7F;
-        bytes[3] = (byte) 0xFF;
-        bytes[4] = (byte) 0xFF;
-        bytes[5] = (byte) 0xFF;
-        CachedData cd = new CachedData(SERIALIZED, bytes, CachedData.MAX_SIZE);
-        assertThat(defaultTranscoder().decode(cd)).isNull();
-    }
-
-    @Test
-    public void testDecodeBinaryWithNegativeBodyLengthReturnsNull() {
-        // Magic + reserved + negative bodyLength prefix.
-        byte[] bytes = new byte[2 + Integer.BYTES];
-        bytes[0] = EVCacheValueSerde.BINARY_SERDE_MAGIC_CONSTANT_BYTE;
-        bytes[1] = 0x00;
-        bytes[2] = (byte) 0xFF;
-        bytes[3] = (byte) 0xFF;
-        bytes[4] = (byte) 0xFF;
-        bytes[5] = (byte) 0xFF;
-        CachedData cd = new CachedData(SERIALIZED, bytes, CachedData.MAX_SIZE);
-        assertThat(defaultTranscoder().decode(cd)).isNull();
-    }
-
-    @Test
-    public void testDecodeBinaryWithBogusKeyLengthInsideValidBodyReturnsNull() {
-        // Valid bodyLength claiming 4 bytes of body, but those 4 bytes encode a bogus keyLength.
-        byte[] bytes = new byte[2 + Integer.BYTES + Integer.BYTES];
-        bytes[0] = EVCacheValueSerde.BINARY_SERDE_MAGIC_CONSTANT_BYTE;
-        bytes[1] = 0x00;
         ByteBuffer bb = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN);
-        bb.putInt(2, Integer.BYTES); // bodyLength = 4 (just the keyLength field)
-        bb.putInt(6, 0x7FFFFFFF);    // bogus keyLength
+        bb.putInt(2, 0x7FFFFFFF);
         CachedData cd = new CachedData(SERIALIZED, bytes, CachedData.MAX_SIZE);
         assertThat(defaultTranscoder().decode(cd)).isNull();
     }
 
     @Test
-    public void testDecodeBinaryWithNegativeKeyLengthInsideValidBodyReturnsNull() {
-        byte[] bytes = new byte[2 + Integer.BYTES + Integer.BYTES];
+    public void testDecodeBinaryWithNegativeKeyLengthReturnsNull() {
+        byte[] bytes = new byte[2 + Integer.BYTES];
         bytes[0] = EVCacheValueSerde.BINARY_SERDE_MAGIC_CONSTANT_BYTE;
         bytes[1] = 0x00;
         ByteBuffer bb = ByteBuffer.wrap(bytes).order(ByteOrder.BIG_ENDIAN);
-        bb.putInt(2, Integer.BYTES); // bodyLength = 4
-        bb.putInt(6, -1);            // negative keyLength
+        bb.putInt(2, -1);
         CachedData cd = new CachedData(SERIALIZED, bytes, CachedData.MAX_SIZE);
         assertThat(defaultTranscoder().decode(cd)).isNull();
     }
 
-    @Test
-    public void testDecodeBinaryWithTrailingBytesBeyondBodyReturnsNull() {
-        // A well-formed envelope with extra bytes appended beyond the declared bodyLength is
-        // rejected — the writer didn't intend those bytes, so they indicate corruption (e.g. a
-        // length prefix that decoded short, leaving the missing payload at the tail).
-        byte[] full = binaryTranscoder().encode(typical()).getData();
-        byte[] withTrailing = Arrays.copyOf(full, full.length + 3);
-        CachedData cd = new CachedData(SERIALIZED, withTrailing, CachedData.MAX_SIZE);
-        assertThat(defaultTranscoder().decode(cd)).isNull();
-    }
-
     // ---- 8. Forward compatibility trip-wire: pinned v0 payload must always decode ----
     //
     // If this test starts failing after a change to EVCacheValueSerde.deserialize(), someone
-    // likely added a required field without the `buffer.position() < bodyEnd` guard. See the
+    // likely added a required field without the `buffer.hasRemaining()` guard. See the
     // "Additive optional" section of EVCacheValueSerde's Javadoc — a future reader must be
     // able to decode the v0 payload below (which an old writer would have produced) for as
     // long as items written by old writers can still be in any cache.
@@ -297,7 +256,6 @@ public void testV0PayloadDecodesAsOptionalAdditiveFieldTripWire() {
         byte[] v0Bytes = {
             (byte) 0x0C,                                              // magic
             (byte) 0x00,                                              // reserved/version
-            0x00, 0x00, 0x00, 0x1E,                                   // bodyLength = 30
             0x00, 0x00, 0x00, 0x01,                                   // keyLength = 1
             (byte) 'k',
             0x00, 0x00, 0x00, 0x01,                                   // valueLength = 1
@@ -312,30 +270,25 @@ public void testV0PayloadDecodesAsOptionalAdditiveFieldTripWire() {
         EVCacheValue expected = new EVCacheValue("k", new byte[] {0x76}, 1, 60L, 42L);
         assertThat(out)
                 .as("Pinned v0 payload must decode cleanly. If it doesn't, a required field was "
-                        + "likely added to deserialize() without the buffer.position() < bodyEnd "
-                        + "guard. See EVCacheValueSerde Javadoc 'Additive optional'.")
+                        + "likely added to deserialize() without the buffer.hasRemaining() guard. "
+                        + "See EVCacheValueSerde Javadoc 'Additive optional'.")
                 .isEqualTo(expected);
     }
 
-    // ---- 9. Forward compatibility: newer-writer extension bytes inside bodyLength ----
+    // ---- 9. Forward compatibility: newer-writer extension bytes past createTime ----
     //
-    // A writer that adds new fields after createTime grows bodyLength to include them. An
-    // older reader (this one) should skip those extra bytes and return the EVCacheValue it
-    // does know how to decode — NOT a corruption event.
+    // A writer that adds new optional fields appends them after createTime. An older reader
+    // (this one) reads its known fields, leaves the extension bytes unread, and returns the
+    // EVCacheValue it does know how to decode — NOT a corruption event.
 
     @Test
     public void testDecodeBinaryWithFutureExtensionFieldsIsForwardCompat() {
         EVCacheValue v = typical();
         byte[] validBytes = binaryTranscoder().encode(v).getData();
 
-        // Layout: [magic][reserved][bodyLength int][body...]. Append 3 extension bytes after
-        // the body and bump the bodyLength field to claim them — what a future writer would do.
-        final int extensionSize = 3;
-        byte[] withExtension = new byte[validBytes.length + extensionSize];
-        System.arraycopy(validBytes, 0, withExtension, 0, validBytes.length);
-        ByteBuffer bb = ByteBuffer.wrap(withExtension).order(ByteOrder.BIG_ENDIAN);
-        int currentBodyLength = bb.getInt(2);
-        bb.putInt(2, currentBodyLength + extensionSize);
+        // Append 3 extension bytes past the end of the v0 envelope — what a future writer
+        // would do. End of envelope is implicit at bytes.length, no header to update.
+        byte[] withExtension = Arrays.copyOf(validBytes, validBytes.length + 3);
 
         CachedData cd = new CachedData(SERIALIZED, withExtension, CachedData.MAX_SIZE);
         Object out = defaultTranscoder().decode(cd);