fix(jcs): count open containers for the nesting bound, so an empty container leaf is refused - #1294
Open
kuangmi-bit wants to merge 1 commit into
Open
kuangmi-bit wants to merge 1 commit into
kuangmi-bit wants to merge 1 commit into
Conversation
An empty container was accepted one level past MAX_DEPTH because the counter charges a level per value on the path (0-based) instead of per open container (outermost at 1), so the effective bound depended on whether the innermost value was a container or a scalar. `_clean_empty` runs before canonicalization and carried the same counter. Adds the jcs_depth_v1 corpus as tests: preimage digests first, published bytes and SHA-256 for the accepts, a catchable refusal for the rejects, and the empty-container boundary pair through both the canonicalizer and the pre-pass.
🧪 Code Coverage (vs
|
This branch has not been deployed
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.
What
_jcs.canonicalizebounds nesting atMAX_DEPTH = 128, but the counter tracks values on the path (0-based) rather than open containers (outermost at 1). An empty container is therefore accepted one level past the bound, while the same shape wrapped around a scalar is refused — the effective limit depends on what the innermost value is.This counts open containers instead, and applies the same rule to
signing._clean_empty, which runs before canonicalization and would otherwise be the exhaustion path.Why
jcs_depth_v1(a2aproject/A2A#2246) pins eight inputs and their verdicts; it is added here astests/utils/jcs_depth_vectors.json, retained as published with attribution. On the current code the empty-container case is mis-accepted:jcs-depth-001…-004jcs-depth-101,-102CanonicalizationErrorCanonicalizationErrorjcs-depth-103(empty-container leaf)CanonicalizationErrorjcs-depth-104RecursionError, at the decodeRecursionError, at the decodejcs-depth-104never reaches the walk: the JSON decoder refuses that input first, in both trees. The test accepts either refusal, as the vector asks, rather than claiming the canonicalizer produced it.The intended rule is already written down in this repo — the docstring of
test_nesting_at_the_limit_is_acceptedsays "the deepest container is at depth n + 1", i.e. containers with the outermost at 1 — so this makes the implementation match its own stated intent, and that boundary test keeps passing unchanged.Tests
tests/utils/test_jcs.pygains the corpus-driven tests: every preimage digest checked before any expectation is used; accepts compared byte-for-byte against the published bytes and SHA-256; rejects asserted to raise; plus a direct empty-container boundary pair and the same pair through_clean_empty.test_jcs.pyandtest_signing.pyboth green.ruff checkandruff format --checkclean on all four files.Scope
Independent of a2aproject/A2A#2122 and of #1287, which are about canonicalization scope, not the nesting bound. Nothing at or below the bound changes: the 24 cards in a2aproject/a2a-tck#246 canonicalize byte-identically before and after.
Reported at a2aproject/A2A#2255. Corpus authored by Sankalp Gilda (Apache-2.0); the JSON keeps the upstream pointer and states that every expected byte string came from the reference implementation it names.