feat(sdk): accept io.Reader in CreateTDF and drop the 64 GB payload cap - #3945
Open
dmihalcik-virtru wants to merge 2 commits into
Open
feat(sdk): accept io.Reader in CreateTDF and drop the 64 GB payload cap#3945dmihalcik-virtru wants to merge 2 commits into
dmihalcik-virtru wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
comp:sdk
This was referenced Sep 1, 2026
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
…he input Two problems in CreateTDFContext's encrypt loop. io.Reader.Read is permitted to return fewer bytes than the caller asked for without erroring, and the loop treated that as fatal: "io.ReadSeeker.Read size mismatch". A *bytes.Reader or *os.File on a local disk rarely returns short, which is why this has held up, but any wrapping ReadSeeker -- a decompressor, a network-backed store, an instrumented reader -- can trigger it and there is nothing wrong with the input when it does. io.ReadFull retries until the segment is full, so the manual size check goes away with it. The new Test_TDFCreateShortReads fails on main with exactly that error message. The read buffer was also sized on defaultSegmentSize alone, which is 2 MiB, so encrypting a twelve-byte payload allocated 2 MiB to hold it. Size it to min(segmentSize, inputSize) instead; the max(inputSize, 1) keeps the empty-payload case, which still emits one empty segment, from asking for a zero-length buffer. Peeled out of the DSPX-2604 stack. Signed-off-by: David Mihalcik <dmihalcik@virtru.com>
CreateTDF and CreateTDFContext took an io.ReadSeeker, so a caller with a pipe,
a socket, or any other one-pass source had to spool the whole payload to disk
or memory first. That is the block DSPX-2604 exists to remove: the Everfox
re-wrap pipeline hands us a stream it cannot rewind. Both now take an
io.Reader and consume it from its current position through EOF.
Seekability was only ever used to measure the input. The length still matters,
but it is now resolved rather than required:
- WithInputSize(n) declares it outright, for a reader that cannot report it;
- failing that, a reader that happens to implement io.Seeker is probed, and
the cursor restored, so every existing caller keeps today's behavior byte
for byte;
- failing both, the payload is unmeasurable and is read until it ends.
The one thing an unmeasurable payload gives up is the compact ZIP32 layout.
The ZIP64 decision is baked into the payload's local file header, which is
emitted ahead of the first segment, so it cannot be revisited once the archive
has started; a payload that might exceed a 32-bit offset has to be written as
ZIP64 from the outset. WithInputSize exists to buy that back — declaring the
length of a piped payload keeps it in ZIP32 when it fits.
The read loop no longer computes a segment count up front. It reads a buffer
at a time until EOF, which is what makes an unknown length workable, and
happens to be the same code path for a short final segment. An empty payload
still produces one empty segment. The segment count is still passed to the
archive writer when it is known, because that is what keeps a large declared
count from being clamped to a one-segment capacity hint.
Two behavior changes worth calling out:
- The 64 GB cap (maxFileSizeSupported/errFileTooLarge) is gone. It could
only ever be enforced on a measurable payload, so keeping it would have
meant `encrypt bigfile` failing where `encrypt < bigfile` succeeded. Both
were unexported; nothing outside the package referenced them.
- A declared size is exact, not an upper bound. A reader that reaches EOF
early now fails the call with errInputShorterThanDeclared instead of
returning a TDF that is silently short of the payload the caller asked to
encrypt. Reading still stops at the declared size if the reader has more.
Testing: Test_CreateTDF_StreamingInput covers the three measurement modes
across empty, sub-segment, exact-multiple, and partial-final-segment payloads,
asserting the ZIP64 choice, the segment count, and a full round trip through
LoadTDF. Test_CreateTDF_InputSizeBounds covers the negative, over-long, short,
and mid-stream-start cases. Both guards were mutation-checked: removing the
io.LimitReader fails "declared size bounds the read", and dropping the
unknown-size ZIP64 rule fails every unmeasurable case.
Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
dmihalcik-virtru
force-pushed
the
dspx-2604-07-readfull
branch
from
September 3, 2026 14:13
8d6b5c4 to
ad7385f
Compare
dmihalcik-virtru
force-pushed
the
dspx-2604-16-createtdf-reader
branch
from
September 3, 2026 14:13
dd43c7e to
002849a
Compare
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
|
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed Changes
CreateTDF and CreateTDFContext took an io.ReadSeeker, so a caller with a pipe,
a socket, or any other one-pass source had to spool the whole payload to disk
or memory first. That is the block DSPX-2604 exists to remove: the Everfox
re-wrap pipeline hands us a stream it cannot rewind. Both now take an
io.Reader and consume it from its current position through EOF.
Seekability was only ever used to measure the input. The length still matters,
but it is now resolved rather than required:
the cursor restored, so every existing caller keeps today's behavior byte
for byte;
The one thing an unmeasurable payload gives up is the compact ZIP32 layout.
The ZIP64 decision is baked into the payload's local file header, which is
emitted ahead of the first segment, so it cannot be revisited once the archive
has started; a payload that might exceed a 32-bit offset has to be written as
ZIP64 from the outset. WithInputSize exists to buy that back — declaring the
length of a piped payload keeps it in ZIP32 when it fits.
The read loop no longer computes a segment count up front. It reads a buffer
at a time until EOF, which is what makes an unknown length workable, and
happens to be the same code path for a short final segment. An empty payload
still produces one empty segment. The segment count is still passed to the
archive writer when it is known, because that is what keeps a large declared
count from being clamped to a one-segment capacity hint.
Two behavior changes worth calling out:
The 64 GB cap (maxFileSizeSupported/errFileTooLarge) is gone. It could
only ever be enforced on a measurable payload, so keeping it would have
meant
encrypt bigfilefailing whereencrypt < bigfilesucceeded. Bothwere unexported; nothing outside the package referenced them.
A declared size is exact, not an upper bound. A reader that reaches EOF
early now fails the call with errInputShorterThanDeclared instead of
returning a TDF that is silently short of the payload the caller asked to
encrypt. Reading still stops at the declared size if the reader has more.
Testing: Test_CreateTDF_StreamingInput covers the three measurement modes
across empty, sub-segment, exact-multiple, and partial-final-segment payloads,
asserting the ZIP64 choice, the segment count, and a full round trip through
LoadTDF. Test_CreateTDF_InputSizeBounds covers the negative, over-long, short,
and mid-stream-start cases. Both guards were mutation-checked: removing the
io.LimitReader fails "declared size bounds the read", and dropping the
unknown-size ZIP64 rule fails every unmeasurable case.
Checklist
Testing Instructions
Test_CreateTDF_StreamingInputandTest_CreateTDF_InputSizeBoundsare thenew coverage. Both guards were mutation-checked: removing the
io.LimitReaderfails "declared size bounds the read", and dropping the unknown-size ZIP64 rule
fails every unmeasurable case.
The full DSPX-2604 stack — 20 PRs
mainmainmainmainmainmainmaindspx-2604-base-11= #3932 + #3934 + #3935dspx-2604-base-17= #3944 + #3945dspx-2604-base-19= #3947 + #3939Reviewable in parallel right now, since they sit directly on
mainand depend onnothing else: 01, 02, 04, 05, 06, 07, 08.
Why three PRs have a
dspx-2604-base-*base. A GitHub PR takes one base branch,but 11, 17 and 19 each build on more than one parent. The
base-*branches are emptymerge commits that exist only to join those parents so the PR diff shows exactly its
own change and nothing else. They contain no code, have no PR of their own, and go
away once their parents land — retarget the child onto
mainat that point.Wants a cross-SDK xtest run before merge: 15, 17 (and therefore 20). They touch
the KAS wire format.
Red checks you may see are network flakes, not this stack. Four distinct ones hit
this batch and all clear on re-run:
golangci-lint config verifytiming out onhttps://golangci-lint.run/.../golangci.v2.8.jsonschema.json(fails the wholego (<module>)job and fail-fast cancels its siblings), the bats installer getting a 403,Docker Hub timing out on
keycloak/keycloak:26.4, andbufreporting "the serverhosted at that remote is unavailable" while the Java SDK generates sources. The
govulncheckstep also emits##[error]annotations against the go1.25.11 stdlib, butit is
continue-on-error: trueand never fails a job — 01 bumps the toolchain andclears those annotations.