From c10ea68711665d0201f56140971a605c623c7160 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sat, 25 Jul 2026 13:39:40 +0200 Subject: [PATCH 1/2] refactor: value objects for all of ZstdFrameHeader's size fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the last naked numeric fields from ZstdFrameHeader: - blockSizeMax(): masked long -> ZstdByteSize (always non-negative) - headerSize(): int -> ZstdByteSize (now matches ZstdFrame.headerSize()) - the raw `long frameContentSize` component is gone; contentSize() is now a real Optional record component - windowSize(): long -> Optional contentSize and windowSize are Optional because a hostile header can declare either as an unsigned value at or above 2^63 (zstd copies the content-size field verbatim, and a single-segment frame sets windowSize equal to it), which is not representable as a non-negative size. Mapping those to empty keeps ZstdFrame.header() parsing total instead of throwing on hostile input — the sign-bit test now asserts both contentSize() and windowSize() go empty. The shared ZstdByteSize.fromFrameHeaderContentSize helper is generalized to fromUnsignedFrameHeaderField since it now wraps both fields. Also resolves the headerSize() asymmetry from the previous PR: ZstdFrame.headerSize() and ZstdFrameHeader.headerSize() now both return ZstdByteSize. Co-Authored-By: Claude Opus 4.8 --- .github/smoke/src/test/java/SmokeTest.java | 2 +- CHANGELOG.md | 8 ++++ .../io/github/dfa1/zstd/ZstdByteSize.java | 26 ++++++------ .../java/io/github/dfa1/zstd/ZstdFrame.java | 14 +++---- .../io/github/dfa1/zstd/ZstdFrameHeader.java | 41 ++++++++++--------- .../io/github/dfa1/zstd/ZstdFrameTest.java | 10 +++-- 6 files changed, 57 insertions(+), 44 deletions(-) diff --git a/.github/smoke/src/test/java/SmokeTest.java b/.github/smoke/src/test/java/SmokeTest.java index a297c0e..5f190da 100644 --- a/.github/smoke/src/test/java/SmokeTest.java +++ b/.github/smoke/src/test/java/SmokeTest.java @@ -184,7 +184,7 @@ void frameIntrospection() { check(header.contentSize().isPresent() && header.contentSize().get().value() == original.length, "header(byte[]).contentSize() mismatch"); check(header.frameType() == ZstdFrameType.STANDARD, "header(byte[]).frameType() expected STANDARD"); - check(header.headerSize() == headerSize, "header(byte[]).headerSize() disagreed with headerSize(byte[])"); + check(header.headerSize().value() == headerSize, "header(byte[]).headerSize() disagreed with headerSize(byte[])"); check(!header.hasChecksum(), "header(byte[]).hasChecksum() expected false (checksum not enabled)"); } diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d1fada..ce7b605 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,14 @@ git tags, which trigger publication to Maven Central. `String`, and `Zstd.versionNumber()` is removed. Use `Zstd.version().toString()` for the `x.y.z` string and `Zstd.version().number()` for the packed number. +- **Breaking:** `ZstdFrameHeader` no longer exposes any naked numeric field. + `blockSizeMax()` and `headerSize()` now return `ZstdByteSize` (were `long`/ + `int`; call `.value()` for the raw number). The raw `long frameContentSize` + component is gone — `contentSize()` is now a real `Optional` + record component — and `windowSize()` likewise returns `Optional`. + Both are `Optional` because a hostile header can declare either as an + unrepresentable value at or above `2^63`, which maps to empty rather than + throwing while the header is parsed. ## [0.11] - 2026-07-24 diff --git a/zstd/src/main/java/io/github/dfa1/zstd/ZstdByteSize.java b/zstd/src/main/java/io/github/dfa1/zstd/ZstdByteSize.java index 27e9561..55fd55b 100644 --- a/zstd/src/main/java/io/github/dfa1/zstd/ZstdByteSize.java +++ b/zstd/src/main/java/io/github/dfa1/zstd/ZstdByteSize.java @@ -84,22 +84,24 @@ static ZstdByteSize fromFrameContentSize(long raw) { return new ZstdByteSize(raw); } - /// Wraps a `ZSTD_getFrameHeader`-parsed content size, returning empty instead - /// of throwing when the frame does not record a usable one — unlike - /// [#fromFrameContentSize(long)], a [ZstdFrameHeader] was already validated - /// by the time this runs, so nothing here is an error. + /// Wraps a `ZSTD_getFrameHeader`-parsed size field — the content size or the + /// window size — returning empty instead of throwing when the field holds no + /// representable size, unlike [#fromFrameContentSize(long)]: a + /// [ZstdFrameHeader] was already validated by the time this runs, so nothing + /// here is an error. /// /// Any negative reading maps to empty, not just the "not stored" sentinel: - /// `ZSTD_getFrameHeader` copies the 8-byte Frame_Content_Size field verbatim - /// without validating its magnitude, so a hostile header can declare an - /// unsigned value at or above `2^63` that wraps negative in a signed `long` — - /// no such size is representable in any buffer this library can produce, so + /// `ZSTD_getFrameHeader` copies these 8-byte fields verbatim without + /// validating their magnitude, so a hostile header can declare an unsigned + /// value at or above `2^63` that wraps negative in a signed `long` (and a + /// single-segment frame sets `windowSize` equal to the content size) — no + /// such size is representable in any buffer this library can produce, so /// "absent" is the honest answer rather than an exception. /// - /// @param raw a content size, the "not stored" sentinel, or an unrepresentable - /// declared value read as a negative `long` - /// @return the wrapped size, or empty if the frame does not store a usable one - static Optional fromFrameHeaderContentSize(long raw) { + /// @param raw a size field reading, the "not stored" sentinel, or an + /// unrepresentable declared value read as a negative `long` + /// @return the wrapped size, or empty if the field holds no representable size + static Optional fromUnsignedFrameHeaderField(long raw) { return raw < 0 ? Optional.empty() : Optional.of(new ZstdByteSize(raw)); } } diff --git a/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrame.java b/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrame.java index 1f2a1df..dc840a7 100644 --- a/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrame.java +++ b/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrame.java @@ -259,13 +259,13 @@ private static ZstdFrameHeader header(MemorySegment data, long size) { throw new ZstdException("incomplete frame header: need " + remaining + " more bytes"); } return new ZstdFrameHeader( - zfh.get(JAVA_LONG, 0), // frameContentSize - zfh.get(JAVA_LONG, 8), // windowSize - zfh.get(JAVA_INT, 16) & 0xFFFFFFFFL, // blockSizeMax - ZstdFrameType.of(zfh.get(JAVA_INT, 20)), // frameType - zfh.get(JAVA_INT, 24), // headerSize - ZstdDictionaryId.of(zfh.get(JAVA_INT, 28)), // dictID - zfh.get(JAVA_INT, 32) != 0); // checksumFlag + ZstdByteSize.fromUnsignedFrameHeaderField(zfh.get(JAVA_LONG, 0)), // contentSize + ZstdByteSize.fromUnsignedFrameHeaderField(zfh.get(JAVA_LONG, 8)), // windowSize + new ZstdByteSize(zfh.get(JAVA_INT, 16) & 0xFFFFFFFFL), // blockSizeMax + ZstdFrameType.of(zfh.get(JAVA_INT, 20)), // frameType + new ZstdByteSize(zfh.get(JAVA_INT, 24)), // headerSize + ZstdDictionaryId.of(zfh.get(JAVA_INT, 28)), // dictID + zfh.get(JAVA_INT, 32) != 0); // checksumFlag } } diff --git a/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java b/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java index 6db678b..fe3b320 100644 --- a/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java +++ b/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java @@ -4,28 +4,29 @@ /// Parsed contents of a zstd frame header, from `ZSTD_getFrameHeader`. /// -/// @param frameContentSize the stored decompressed size, or a sentinel if unknown; -/// prefer [#contentSize()] -/// @param windowSize the back-reference window size needed to decode the frame -/// @param blockSizeMax the maximum block size used in the frame -/// @param frameType whether this is a standard or skippable frame -/// @param headerSize the size of the frame header in bytes -/// @param dictId the dictionary id, or [ZstdDictionaryId#NONE] if none (for a -/// skippable frame, [ZstdDictionaryId#raw()] is the magic variant 0..15) -/// @param hasChecksum whether a 4-byte content checksum follows the frame +/// `contentSize` and `windowSize` are [Optional] rather than a plain +/// [ZstdByteSize]: a hostile header can declare either as an unsigned value at or +/// above `2^63` (zstd copies the content-size field verbatim, and for a +/// single-segment frame sets `windowSize` equal to it), which is not +/// representable as a non-negative size — so it maps to empty rather than +/// throwing while the header is parsed. +/// +/// @param contentSize the stored decompressed size, or empty if the frame does +/// not record a representable one +/// @param windowSize the back-reference window size needed to decode the frame, +/// or empty if the frame declares an unrepresentable one +/// @param blockSizeMax the maximum block size used in the frame +/// @param frameType whether this is a standard or skippable frame +/// @param headerSize the size of the frame header in bytes +/// @param dictId the dictionary id, or [ZstdDictionaryId#NONE] if none (for a +/// skippable frame, [ZstdDictionaryId#raw()] is the magic variant 0..15) +/// @param hasChecksum whether a 4-byte content checksum follows the frame public record ZstdFrameHeader( - long frameContentSize, - long windowSize, - long blockSizeMax, + Optional contentSize, + Optional windowSize, + ZstdByteSize blockSizeMax, ZstdFrameType frameType, - int headerSize, + ZstdByteSize headerSize, ZstdDictionaryId dictId, boolean hasChecksum) { - - /// The decompressed size, if the frame records it. - /// - /// @return the content size, or empty if the frame does not store it - public Optional contentSize() { - return ZstdByteSize.fromFrameHeaderContentSize(frameContentSize); - } } diff --git a/zstd/src/test/java/io/github/dfa1/zstd/ZstdFrameTest.java b/zstd/src/test/java/io/github/dfa1/zstd/ZstdFrameTest.java index fac77cf..7db6298 100644 --- a/zstd/src/test/java/io/github/dfa1/zstd/ZstdFrameTest.java +++ b/zstd/src/test/java/io/github/dfa1/zstd/ZstdFrameTest.java @@ -172,7 +172,7 @@ void matchesTheParsedHeaderSize() { byte[] frame = Zstd.compress(PAYLOAD); // Then the light probe agrees with the fully parsed header - assertThat(ZstdFrame.headerSize(frame)).isEqualTo(new ZstdByteSize(ZstdFrame.header(frame).headerSize())); + assertThat(ZstdFrame.headerSize(frame)).isEqualTo(ZstdFrame.header(frame).headerSize()); } @Test @@ -278,7 +278,7 @@ void reportsContentSizeAndNoChecksumByDefault() { assertThat(header.contentSize()).hasValue(new ZstdByteSize(PAYLOAD.length)); assertThat(header.hasChecksum()).isFalse(); assertThat(header.dictId()).isEqualTo(ZstdDictionaryId.NONE); - assertThat(header.windowSize()).isPositive(); + assertThat(header.windowSize()).hasValueSatisfying(w -> assertThat(w.value()).isPositive()); } @Test @@ -297,7 +297,7 @@ void reportsASaneBlockSizeMax() { // Then blockSizeMax is the masked 32-bit field — a real block size, never // the all-ones value a sign-extension or wrong mask would produce - assertThat(header.blockSizeMax()).isPositive().isLessThanOrEqualTo(128 * 1024L); + assertThat(header.blockSizeMax().value()).isPositive().isLessThanOrEqualTo(128 * 1024L); } @Test @@ -323,8 +323,10 @@ void contentSizeIsEmptyForAContentSizeWithTheSignBitSet() { ZstdFrameHeader header = ZstdFrame.header(frame); // Then contentSize() reports absent instead of leaking an - // IllegalArgumentException out of ZstdByteSize's non-negative guard + // IllegalArgumentException out of ZstdByteSize's non-negative guard — + // and windowSize(), which a single-segment frame mirrors from it, too assertThat(header.contentSize()).isEmpty(); + assertThat(header.windowSize()).isEmpty(); } // A minimal single-segment zstd frame header (magic + descriptor + 8-byte From 7deaa1fc6f86dc7f8f041b61e5cd6d72b4682228 Mon Sep 17 00:00:00 2001 From: Davide Angelocola Date: Sat, 25 Jul 2026 20:26:36 +0200 Subject: [PATCH 2/2] docs: explain the hasChecksum flag on ZstdFrameHeader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records why only the flag — not the checksum value — appears here: the content-checksum bit is header data (Frame_Header_Descriptor), but the 4-byte XXH64 checksum is a frame trailer over the decompressed content, so it is not part of the parsed header. Notes the flag's practical use: gating on integrity before decompressing. Co-Authored-By: Claude Opus 4.8 --- .../src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java b/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java index fe3b320..788a834 100644 --- a/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java +++ b/zstd/src/main/java/io/github/dfa1/zstd/ZstdFrameHeader.java @@ -20,7 +20,12 @@ /// @param headerSize the size of the frame header in bytes /// @param dictId the dictionary id, or [ZstdDictionaryId#NONE] if none (for a /// skippable frame, [ZstdDictionaryId#raw()] is the magic variant 0..15) -/// @param hasChecksum whether a 4-byte content checksum follows the frame +/// @param hasChecksum whether the frame descriptor sets the content-checksum +/// flag — a 4-byte XXH64 checksum of the decompressed content +/// then trails the frame, which the decoder verifies on +/// decompress. Only this flag is header data; the checksum +/// value itself lives in the frame trailer and is not exposed +/// here. Gate on it to require integrity before decompressing. public record ZstdFrameHeader( Optional contentSize, Optional windowSize,