fix(sdk): DSPX-4589 zip64 + manifest conformance (combined view of #397 + #398)

[dmihalcik-virtru]

Jira: https://virtru.atlassian.net/browse/DSPX-4589

Top of a 2-PR stack. Do not review or merge this one — review #397 and #398.

This PR keeps its base at main, so its diff is the combined work: it exists to show what the stack adds up to and to hold the shared context. Its head branch is the same commit as #398's.

	PR	base	contents
1	#397	main	per-segment size defaults — the finding with field impact
2	#398	#397	zip64 EOCD sentinels, truncated archive detection, UTF-8 entry names
—	this PR	main	the combined diff of 1 + 2

Merge order: #397, then #398. This PR closes itself out once both have landed.

---

Four zip64 / manifest conformance findings from the container audit. Findings 1-3 were written against pre-#393 main; each was re-verified against current main (98c839e2) before being changed, and the honest status of each is below. Only finding 4 has field impact, which is why it was pulled to the bottom of the stack as #397.

Finding 1 — end of central directory sentinel thresholds (fixed) — #398

ZipWriter.finish() decided whether the archive needed a zip64 end of central directory record with:

(numEntries & ~0xFF) != 0 || (startOfCentralDirectory & ~0xFFFF) != 0 || (sizeOfCentralDirectory & ~0xFFFF) != 0

None of those masks match the real field widths. The entry count field is 2 bytes, not 1, and the central directory offset and size fields are 4 bytes each, not 2. The practical effect was that any archive with 256 or more entries was needlessly promoted to zip64, and the offset/size checks fired three orders of magnitude too early.

Now:

var isZip64 = hasZip64Entry
        || numEntries > MAX_NON_ZIP64_ENTRY_COUNT          // 0xFFFE; 0xFFFF is the sentinel itself
        || needsZip64(startOfCentralDirectory, sizeOfCentralDirectory);

The offset and size go through needsZip64, so they honor the same deliberate 2 GiB (Integer.MAX_VALUE) ceiling as the per-entry fields — released readers widen these unsigned wire fields with signed reads, so we never write a value that would come back negative. Using needsZip64 also keeps the ZipWriter(out, maxNonZip64Value) test seam that #393 added usable for these two fields.

Not touched by #393 — verified with jj diff -r 98c839e2.

Finding 2 — truncated archive detection (fixed, but the branch was latent) — #398

ZipReader.readEndOfCentralDirectory() scanned backwards for the EOCD signature and treated a null from readInteger() (a short read) as if it were a signature match, so a truncated archive could fall out of the loop and parse whatever bytes followed as an EOCD record. It now only breaks on a genuine match and throws InvalidZipException if the scan runs off the front, and separately rejects an archive too small to hold the zip64 locator its EOCD claims to have.

Stated plainly: with the SeekableInMemoryByteChannel the SDK actually uses, the scan position is always <= size - 22, so readInteger() always had four bytes available and the null branch was unreachable. This is a correctness/robustness fix (a FileChannel could in principle short-read), not a bug anyone was hitting. The new tests still pass on unpatched code for the plain-truncation cases; they are there to lock the behavior in.

Finding 3 — UTF-8 filename length (already correct on the wire; dead code removed) — #398

The ticket says the central directory filename length was computed from String.length(). That assignment did exist:

cdFileHeader.filenameLength = (short) fileInfo.filename.length();

but CDFileHeader.write() never read the field — it wrote (short) filename.length from the already-encoded byte array. The bytes on the wire were already correct. No archive was ever mis-written.

What changed: the misleading dead filenameLength field is removed from both LocalFileHeader and CDFileHeader, the name is encoded to UTF-8 once instead of twice, and a new encodeFilename helper rejects a name whose encoded length exceeds 0xFFFF with an SDKException rather than silently truncating it into the 2-byte field.

Finding 4 — per-segment sizes not defaulted from the manifest defaults (fixed; this is the one with field impact) — #397

integrityInformation.segments[].segmentSize and .encryptedSegmentSize are optional in the TDF spec; when absent, the reader is supposed to fall back to segmentSizeDefault / encryptedSegmentSizeDefault. Gson left the absent primitives at 0, and java-sdk read the payload with a zero-length segment. Every web SDK TDF larger than one default segment (1 MiB) failed to decrypt in java-sdk, surfacing as a confusing integrity error rather than as a manifest problem.

A primitive long cannot distinguish an absent JSON key from a literal 0, so the fix consults the parse tree: a Gson TypeAdapterFactory registered for IntegrityInformation walks the parsed segments array alongside the deserialized list and fills in the defaults only where the key is absent or JSON null. Boxing Segment.segmentSize to Long would have been the other option, but it breaks the public API (== in Segment.equals, an int -> Long assignment in TDF, existing assertEquals(Long, int) in tests) for no added behavior, so the post-deserialization fixup was chosen instead. Explicit 0 in the JSON is preserved as 0.

TDF.Reader.readPayload additionally rejects a segment with a non-positive encryptedSegmentSize up front — an encrypted segment always carries at least an IV and a tag — so a manifest that supplies neither a per-segment size nor a usable default now says so instead of failing downstream with an unrelated complaint about the payload being too small to GMAC.

Tests

11 new tests, all in sdk/src/test/java/io/opentdf/platform/sdk/:

• ZipWriterTest (fix(sdk): DSPX-4589 zip64 EOCD sentinels, truncated archive detection, and UTF-8 entry names #398) — one test per EOCD sentinel driver: entry count (0xFFFE non-zip64 vs 0xFFFF zip64, round-tripped through ZipReader), central directory offset, and central directory size, each isolated so only the EOCD is zip64 and no entry is; plus filenameLengthIsMeasuredInUtf8Bytes ("🔒両.txt", String.length() 7 vs 11 UTF-8 bytes, asserted at both header offsets) and rejectsAnEntryNameTooLongToDescribe.
• ZipReaderTest (fix(sdk): DSPX-4589 zip64 EOCD sentinels, truncated archive detection, and UTF-8 entry names #398) — testTruncatedArchiveIsRejected (four truncation shapes) and testArchiveTooShortForTheZip64LocatorIsRejected, both asserting InvalidZipException.
• ManifestTest (fix(sdk): DSPX-4589 default absent per-segment sizes to the manifest defaults #397) — testAbsentSegmentSizesFallBackToTheManifestDefaults (absent / partially overridden / fully overridden, plus a toJson round trip) and testExplicitZeroSegmentSizeIsNotTreatedAsAbsent.
• TDFTest (fix(sdk): DSPX-4589 default absent per-segment sizes to the manifest defaults #397) — testReadingATDFThatOmitsDefaultedSegmentSizes encrypts ~2 MiB + 4242 bytes at a 1 MiB segment size, strips every per-segment size equal to the default from the manifest, and asserts a byte-exact decrypt; testZeroLengthSegmentIsRejectedWithAClearError.

The sentinel and default-fallback tests were confirmed to be genuine regression tests by reverting the EOCD mask fix and the registerTypeAdapterFactory line and watching them fail.

CI-equivalent commands, on JDK 21:

mvn --batch-mode verify -Dmaven.antrun.skip -P 'coverage,non-fips,!fips'
  -> 242 tests, 0 failures, 0 errors, 8 skipped (231 before this change)
     #397 alone: 235 tests, 0 failures, 0 errors, 8 skipped

mvn --batch-mode install -pl sdk-fips-bc -am -Dmaven.antrun.skip -Dmaven.test.skip
mvn --batch-mode test enforcer:enforce -P 'fips,!non-fips' -Dmaven.antrun.skip
  -> BUILD SUCCESS

End-to-end validation of finding 4

Run on the opentdf/tests DSPX-4592-02-chunky branch, which adds test_tdfs.py::test_chunky_roundtrip — a 5 MiB round trip, versus the 128 bytes the suite has used for four years, which is what it takes for a writer to emit a segment whose size equals the manifest default. All runs pass force-supports=chunky, which makes tdfs.skip_chunky_skew return early so the cell reports a real pass or fail instead of skipping on the unreleased version gate.

	java-ref	run	js -> java chunky cell
fix (#397)	DSPX-4589-01-segment-size-defaults	34353420418 ✅	PASSED
control	main (the stack's base)	34355312405 ❌	FAILED
full stack (#398 / this branch)	DSPX-4589-02-zip-format	34357326964 ✅	PASSED

Exactly one cell flips between the fix run and the control. Every chunky pair, side by side:

encrypt -> decrypt	control (java@main)	fix (java@DSPX-4589-01-...)
js -> java	FAILED	PASSED
go -> java	PASSED	PASSED
java -> java	PASSED	PASSED
java -> go	PASSED	PASSED
java -> js	PASSED	PASSED

(The four non-java pairs report SKIPPED in both runs — focus-sdk=java deselects them, not the feature gate.)

js -> java is precisely finding 4: a web-SDK writer omits the per-segment sizes, and the java reader cannot default them back. The control fails with the confusing downstream symptom described above:

java.lang.IllegalArgumentException: tried to calculate GMAC on too small a payload. payload is 0bytes while GMAC is 16 bytes
	at io.opentdf.platform.sdk.TDF.calculateSignature(TDF.java:481)
	at io.opentdf.platform.sdk.TDF$Reader.readPayload(TDF.java:447)

Job totals: control js job 1 failed, 23 passed, 50 skipped; fix java job 82 passed, 22 skipped, no failures and no chunky skips.

All confirmed by grepping the run logs for the cell's own PASSED/FAILED/SKIPPED line rather than trusting the job's colour — a green job with a skipped cell is the vacuous pass the test exists to prevent.

Findings 1-3 (#398) have no xtest cell of their own; they are covered by unit tests plus the full-stack run above showing no regression.

⚠️ Follow-up required in opentdf/tests — merging will NOT flip it

force-supports is a pre-release override for the runs above only. xtest/sdk/java/cli.sh still answers chunky unsupported: see DSPX-4589 and hard-codes exit 1, so by default test_chunky_roundtrip keeps skipping for java. That shim lives in the opentdf/tests repo, not in this one, so nothing in this stack changes it.

When this fix releases, someone must version-gate the chunky) case in xtest/sdk/java/cli.sh so java reports support at or above the releasing version. Deliberately not done here: this stack touches only opentdf/java-sdk, and DSPX-4592 owns the tests repo.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

X-Test Results

✅ java@pull-396-v0.26.0
✅ java@v0.18.0-main
✅ java@pull-396-main
✅ java@v0.18.0-v0.26.0
✅ js@main-v0.26.0
✅ js@main-main
✅ go@main-main
✅ go@main-v0.26.0

[github-actions]

X-Test Results

✅ java@v0.18.0-main
✅ java@pull-396-v0.26.0
✅ java@pull-396-main
✅ java@v0.18.0-v0.26.0
✅ js@main-v0.26.0
✅ go@main-v0.26.0
✅ go@main-main
✅ js@main-main

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
86.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

X-Test Results

✅ java@v0.18.0-main
✅ java@pull-396-main
✅ java@pull-396-v0.26.0
✅ java@v0.18.0-v0.26.0
✅ go@main-v0.26.0
✅ js@main-v0.26.0
✅ js@main-main
✅ go@main-main

[dmihalcik-virtru]

Closing as a duplicate of #398.

This PR existed to show the combined diff of the 2-PR stack (#397 + #398). #397 has merged, so what is left of the stack is exactly #398 — and with both PRs now based on main and pointing at the same head commit, the two are literally the same pull request. No reason to keep both open.

Review continues in #398. Not deleting DSPX-4589-zip64-conformance for now; it is the same commit as DSPX-4589-02-zip-format, so nothing is lost with it.