Skip to content

fix(sdk): DSPX-1561 parse manifests strictly - #436

Open
dmihalcik-virtru wants to merge 1 commit into
mainfrom
DSPX-1561-strict-manifest
Open

dmihalcik-virtru wants to merge 1 commit into
mainfrom
DSPX-1561-strict-manifest

Conversation

@dmihalcik-virtru

@dmihalcik-virtru dmihalcik-virtru commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Proposed Changes

  • The manifest is checked while it streams, before Gson's typed model can keep only one of two values, and whether or not assertions are verified later (ZTDF assertions specification, Section 3):
    • duplicate member names in any object, including escaped spellings, fail with duplicate-member;
    • at each level the SDK models (the manifest root, payload, encryptionInformation, key access objects and their policyBinding, method, integrityInformation, rootSignature, segments, and each assertion's object, statement, and binding), a case variant of a modelled member name, or two members whose names differ only in case, fails with ambiguous-member;
    • inside assertions, lone surrogates, Unicode noncharacters, and numbers outside the IEEE 754 double range fail with invalid-json.
  • One historical spelling, IsStreamable in encryptionInformation.method, is still accepted. java-sdk wrote it up to v0.7.5.
  • The manifest is read as strict JSON and decoded as strict UTF-8; invalid bytes are no longer replaced with U+FFFD. A leading byte order mark is rejected.
  • New SDK.AssertionFailureException (a subclass of SDK.AssertionException) carries the assertion id, when one could be read, and an AssertionFailureReason holding the Section 9 code. Its message includes reason=<code>, with the id and detail escaped and length-capped.

Split out of #432 (draft; the complete change). Cross-SDK counterparts: opentdf/platform#4305, opentdf/web-sdk#1139; cross-SDK test: opentdf/tests#707.

Compatibility: manifests with duplicate or case-variant member names, invalid UTF-8, or a byte order mark no longer load.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

  • mvn -pl sdk,cmdline -am test (new cases in StrictManifestParsingTest, including the 0.7.5 golden TDF with IsStreamable and a long segment list).

Apply the JSON profile of the ZTDF assertions specification (Section 3)
while the manifest streams, before a typed field can keep only one of two
values, and whether or not assertions are verified later:

- duplicate member names in any object, compared after decoding escapes,
  are rejected with duplicate-member;
- at every level the SDK models, from the manifest root through key
  access objects, integrity information and segments down to each
  assertion's statement and binding, a case variant of a modelled member
  name, or two members whose names are case variants of each other, is
  rejected with ambiguous-member. IsStreamable in
  encryptionInformation.method, which java-sdk wrote up to v0.7.5, is
  still accepted;
- inside assertion objects, lone surrogates, Unicode noncharacters and
  numbers outside the IEEE 754 double range are rejected with
  invalid-json.

The manifest is read as strict JSON and decoded as strict UTF-8 rather
than replacing invalid bytes with U+FFFD, and may not start with a byte
order mark.

Add SDK.AssertionFailureException, a subclass of AssertionException that
carries the assertion id (when one could be read) and an
AssertionFailureReason with the Section 9 codes. Its message contains
reason=<code>, with the id and detail escaped and length-limited.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>
@dmihalcik-virtru
dmihalcik-virtru requested review from a team as code owners November 9, 2026 14:50
@coderabbitai

coderabbitai Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 45a2bb9c-d623-49f1-b639-2734d62d4e8a
Commits

Reviewing files that changed from the base of the PR and between 4092636 and 9b19e0d.

Files selected for processing: 8

  • sdk/src/main/java/io/opentdf/platform/sdk/AssertionFailureReason.java
  • sdk/src/main/java/io/opentdf/platform/sdk/AssertionJson.java
  • sdk/src/main/java/io/opentdf/platform/sdk/Manifest.java
  • sdk/src/main/java/io/opentdf/platform/sdk/SDK.java
  • sdk/src/main/java/io/opentdf/platform/sdk/StrictJsonReader.java
  • sdk/src/main/java/io/opentdf/platform/sdk/TDFReader.java
  • sdk/src/test/java/io/opentdf/platform/sdk/StrictManifestParsingTest.java
  • sdk/src/test/java/io/opentdf/platform/sdk/TDFTest.java
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@sonarqubecloud

sonarqubecloud Bot commented Nov 9, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
D Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

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