Skip to content

refactor: value objects for all of ZstdFrameHeader's size fields#103

Merged
dfa1 merged 2 commits into
mainfrom
refactor/frame-header-value-objects
Jul 25, 2026
Merged

refactor: value objects for all of ZstdFrameHeader's size fields#103
dfa1 merged 2 commits into
mainfrom
refactor/frame-header-value-objects

Conversation

@dfa1

@dfa1 dfa1 commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Why

Final piece of the API-stabilization audit, per follow-up feedback: no naked numeric fields left on ZstdFrameHeader — the raw parsed-struct record.

What changed

Component Before After
blockSizeMax() long ZstdByteSize
headerSize() int ZstdByteSize
contentSize() derived method over long frameContentSize real Optional<ZstdByteSize> component (raw frameContentSize removed)
windowSize() long Optional<ZstdByteSize>

Why Optional, not plain ZstdByteSize, for two of them

A hostile header can declare the content size — and, for a single-segment frame, the window size zstd mirrors from it — as an unsigned value at or above 2^63. That wraps negative in a signed long and is not representable as a non-negative ZstdByteSize. Wrapping it directly would turn ZstdFrame.header() parsing into a thrown exception on hostile input (a DoS-shaped regression). Optional<ZstdByteSize> is the honest value-object representation: present when representable, empty otherwise. The existing 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.

Bonus

Resolves the headerSize() asymmetry noted in #102: ZstdFrame.headerSize() and ZstdFrameHeader.headerSize() now both return ZstdByteSize.

Note

contentSize/windowSize are now Optional record components. If your SonarQube profile flags Optional as a component (S3553-adjacent), say so and I'll adjust — but modern Sonar exempts records, and this is the cleanest "always a value object" shape.

Testing

./mvnw clean test green (324); validate + javadoc + checkstyle clean; reactor compiles.

🤖 Generated with Claude Code

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<ZstdByteSize> record component
- windowSize(): long -> Optional<ZstdByteSize>

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 <noreply@anthropic.com>
@dfa1
dfa1 force-pushed the refactor/frame-header-value-objects branch from ea1e0ce to c10ea68 Compare July 25, 2026 16:56
@dfa1 dfa1 changed the title refactor: ZstdByteSize for ZstdFrameHeader's bounded size fields refactor: value objects for all of ZstdFrameHeader's size fields Jul 25, 2026
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 <noreply@anthropic.com>
@dfa1
dfa1 merged commit 7deaa1f into main Jul 25, 2026
1 check passed
@dfa1
dfa1 deleted the refactor/frame-header-value-objects branch July 25, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant