feat(java): reject repeated JSON properties on read by pavel-ptashyts · Pull Request #4166 · apache/fory · GitHub
Skip to content

feat(java): reject repeated JSON properties on read - #4166

Open
pavel-ptashyts wants to merge 1 commit into
apache:mainfrom
pavel-ptashyts:feat/json-duplicate-properties
Open

pavel-ptashyts wants to merge 1 commit into
apache:mainfrom
pavel-ptashyts:feat/json-duplicate-properties

Conversation

@pavel-ptashyts

@pavel-ptashyts pavel-ptashyts commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Why?

When a JSON object repeats one of a bean's known properties, fory-json keeps the last value and there is no way to reject the document instead. Parsers disagree on duplicate names (first wins, last wins, or fail), so a service that validates a document from a third party and passes it on can read a different value than the next hop. Jackson only offers the parser-wide STRICT_DUPLICATE_DETECTION, which applies to every object and map and costs every read. Details and the motivating case (rejecting repeated top-level fields of inbound OpenRTB requests) are in #4165.

What does this PR do?

  • API. ForyJsonBuilder.failOnDuplicateProperties(boolean), default false (unchanged behaviour: the last occurrence wins), and a class annotation @JsonDuplicateProperties(fail = true | false) that overrides the builder for one class. Like @JsonInclude, the annotation is not inherited and an exact-target Mixin can supply it; it is registered in the runtime and annotation-processor Mixin lists. It applies where the class is read as a JSON object.
  • Semantics. A known property (read-enabled field, setter, creator parameter, or a name flattened in by @JsonUnwrapped) that appears twice in one object fails the read with ForyJsonException: Duplicate JSON property <name> on <class>. An explicit null counts as an occurrence, also under onNullRead(SKIP). Unknown and ignored names and Any entries are not tracked. Nested objects follow their own class setting.
  • Interpreter (ObjectCodec). Strict ordinary beans use their own per-representation loop (read*StrictFixedObject): one extra check inside the lenient bean loop measured about +8% on interpreted reads, so the lenient loop is left exactly as it was. Creator, Any and unwrapped loops check a bit set that is allocated only for strict types; their lenient timings stayed within noise.
  • Generated readers (JsonReaderCodegen). The decision is made at generation time, so lenient types generate exactly the same source as before. Strict bean readers keep long words seen0..seenK: the fast path (which never repeats a member) only sets bits, group helpers receive the words as parameters and return their own group-relative bits as a long (-1 when the object is finished), and slow methods take the words as parameters and check every resolved known index. Ordered creator prefixes have no look-ahead, so the creator slow method seeds its bits from the prefix length. Unwrapped and Any-creator readers keep the words as locals of their single loop. Names for runtime-index failures come from a cold generated switch.
  • Generated class sharing. The effective setting is part of the generated codec key for reader roles, so strict and lenient instances in one JVM never share a reader class.
  • Docs. docs/json/object-mapping.md (builder option) and docs/json/annotations.md (annotation).
  • Tests. New JsonDuplicatePropertiesTest (both codegen modes, Latin1/UTF-16/UTF-8 String/bytes/stream inputs): beans, explicit nulls, class override in both directions, nested objects, ordered and unordered creators, Any beans and Any creators, unwrapped with creators and prefixes, unwrapped with Any, a subtype with Any, a generic bean, a Mixin, a 70-property bean whose group straddles the 64-bit word boundary, a slow fallback from inside a group helper, long (non-direct) names, and two instances with different settings in one JVM. JsonGeneratedCodecTest.duplicateTrackingSource checks the generated Latin1, UTF-16 and UTF-8 reader sources: no tracking for a lenient class, tracking for a strict one.

Related issues

AI Contribution Checklist

  • Substantial AI assistance was used in this PR: yes
  • If yes, I included a completed AI Contribution Checklist in this PR description and the required AI Usage Disclosure.
  • If yes, my PR description includes the required ai_review summary and screenshot evidence or equivalent persisted links of the final clean AI review results from both fresh reviewers described in AI_POLICY.md, the Fory-guided reviewer and the independent general reviewer, on the current PR diff or current HEAD after the latest code changes.

Completed checklist from AI_POLICY.md section 9:

  • Substantial AI assistance was used in this PR: yes
  • I included the standardized AI Usage Disclosure block below.
  • I can explain and defend all important changes without AI help.
  • I reviewed AI-assisted code changes line by line before submission.
  • I completed line-by-line self-review first and fixed issues before requesting AI review.
  • I ran two fresh AI review agents on the current PR diff or current HEAD after the latest code changes: one Fory-guided reviewer using AGENTS.md and .agents/ci-and-pr.md, and one independent general reviewer in a separate clean-context review session that was not pointed to .agents/ci-and-pr.md or any copied Fory-specific review checklist.
  • I addressed all AI review comments and repeated the review loop until both ai reviewers reported no further actionable comments.
  • I attached equivalent persisted links of the final clean AI review results from both fresh reviewers on the current PR head in this PR body.
  • I ran adequate human verification and recorded evidence (checks run locally, pass/fail summary, and confirmation I reviewed results).
  • I added/updated tests and specs where required.
  • I validated protocol/performance impacts with evidence when applicable.
  • I verified licensing and provenance compliance.
AI Usage Disclosure
- substantial_ai_assistance: yes
- scope: design drafting, code drafting, tests, benchmark harness, docs, PR text
- affected_files_or_subsystems: java/fory-json (builder/config, ObjectCodec interpreter, JsonReaderCodegen, generated codec key, Mixin registration), fory-annotation-processor Mixin list, docs/json
- ai_review: line-by-line self-review of the AI-drafted change is pending by the contributor (see unchecked items above). Two-reviewer loop on fresh sessions for every round, three rounds. Fixed from review: UTF-16 test input no longer starts with an unknown member, so the ordered fast path is covered there too; the error names the owning class; unwrapped names are resolved only on failure; added tests for a slow fallback from inside a group helper, unwrapped creators and prefixes, unwrapped with Any, a subtype with Any, creator nulls under SKIP, a generic bean, an unordered (17-member) creator, non-direct names, and a generated-source check for lenient and strict readers; the annotation scope is documented. Found by the author's own measurements between rounds: the first design (no fast-path look-ahead for strict readers) cost +63% on documents missing an early member and was replaced by carrying the bits through the fast path; one check inside the lenient interpreted bean loop cost +8% and was moved to a separate strict loop. Kept by decision: deferred creator members (language-module object models only) share the creator loop check without a Java fixture; a Native Image build-time/runtime setting mismatch falls back to the interpreter like failOnMissingRequiredProperties. Final round on the current head: both reviewers report no further actionable comments.
- ai_review_artifacts: Fory-guided reviewer: https://github.com/apache/fory/pull/4166#issuecomment-6111855733 ; independent general reviewer: https://github.com/apache/fory/pull/4166#issuecomment-6111855963 (both on head commit 6fe221e0, final round, no further actionable comments)
- human_verification: see "Verification" below
- performance_verification: see "Benchmark" below
- provenance_license_confirmation: all code written for this PR against the Apache Fory sources; no third-party code introduced

Verification

JDK 25.0.2, Windows 11 x86_64, from java/:

  • mvn -pl fory-json -am install -DskipTests: success
  • mvn -pl fory-json,fory-annotation-processor test: fory-json 1324 tests, fory-annotation-processor 102 tests, 0 failures, 0 errors, 0 skipped
  • mvn -pl fory-json,fory-annotation-processor spotless:check checkstyle:check: clean
  • Mutation checks, each made JsonDuplicatePropertiesTest fail as intended: no interpreter marks; no generated marks; no fast-path marks; no second-word carry of group bits; group bits not passed from a helper to the slow method; setting removed from the generated codec key.
  • Generated-source identity: with FORY_CODE_DIR, the 30 generated sources for a bean, an Any bean, a creator, an unwrapped model and a 70-property bean (all writer and reader roles) are byte-identical between apache/main (b602392) and this branch.

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change? ForyJsonBuilder.failOnDuplicateProperties(boolean) and @JsonDuplicateProperties; the default behaviour is unchanged.
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

Default (lenient) path: generated readers are byte-identical to apache/main (see above). Interpreted lenient reads (withCodegen(false), 24 int fields, UTF-8 bytes, ad-hoc timing loop, median of 6 alternating JVMs per jar): declaration-ordered document 343 ns on main vs 340 ns on this branch; document with the first member last 384 vs 365 ns (that case is bimodal across JVMs on both jars). Interpreted 10-member creator and Any bean: within noise.

Strict readers, generated code (same 24-field bean, strict vs lenient class in one JVM): declaration-ordered document within noise; document with the first member moved to the end about +5..10%.

Add ForyJsonBuilder.failOnDuplicateProperties(boolean) and the class
annotation @JsonDuplicateProperties(fail = ...). When enabled for a type,
a known property (field, setter, creator parameter or unwrapped name)
that appears twice in one JSON object fails the read with a
ForyJsonException naming the property. The default is unchanged: the
last occurrence wins, and lenient types generate the same reader code as
before.

Strict generated readers carry one bit per member through the fast path,
its group helpers and the slow method; the interpreter keeps a per-object
bit set. The effective setting is part of the generated codec key.

Closes apache#4165

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pavel-ptashyts

Copy link
Copy Markdown
Contributor Author

@pavel-ptashyts

Copy link
Copy Markdown
Contributor Author

This branch has not been deployed

No deployments
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.

[Java][JSON] No way to reject a repeated JSON property on read

1 participant