fix(json): register provider and fix JSON/YAML parity gaps - #195
Merged
sspaink merged 1 commit intoAug 11, 2026
Merged
Conversation
sspaink
force-pushed
the
fix/register-json-provider
branch
from
August 11, 2026 14:05
3b454b7 to
3aa42fd
Compare
The BuiltinProvider entry for opa-builtins-json was commented out in its
META-INF/services file, so ServiceLoader never discovered it and none of
the twelve json/yaml builtins were reachable for consumers.
Registering it exposed 16 failing compliance cases:
- json.marshal_with_options ignored "pretty" and "prefix" and used
Jackson's layout. Go composes prefix + MarshalIndent(v, prefix,
indent): the prefix leads the document and repeats on every line,
the separator is ": ", an empty container stays on one line, and an
explicit "pretty": false disables indent and prefix. An unknown
option key is a type error, which was not reported at all.
- json.is_valid raised a type error for a non-string operand where Go
returns false.
- json.match_schema and json.verify_schema accepted a schema with an
unknown "type". The validator silently never matches it, whereas Go
rejects the schema, so the schema is now checked up front. Their
error objects also used networknt's field names and wording rather
than the desc/error/field/type shape Go returns.
- json.patch could not address Rego sets. Sets serialize as arrays, so
their locations are recorded before serializing: a member is then
matched by value rather than index, an absent member appends, and
the result is rebuilt as a set. Since the path segment is the member,
an "add" whose value differs from the segment is undefined.
yaml.unmarshal now reports Go's terse "yaml: line N: <problem>" instead
of SnakeYAML's multi-line snippet. The residual wording difference is
recorded in ComplianceTest's alternate-message map, as mapping every
SnakeYAML diagnostic onto yaml.v2's phrasing is not tractable.
Raises the evaluator's test heap. The jsonpatch/json_patch_tests fixture
runs the whole upstream JSON Patch spec suite inside a single policy, and
once json.patch resolves it needs more than Gradle's default 512m. This
is a test-only limit, but it does say json.patch is memory-hungry on
large inputs and is worth profiling separately.
A path segment of digits that overflow an int is no longer read as an
array index: it reached Integer.parseInt and threw NumberFormatException
out of the builtin, where it should leave the call undefined. Reported by
CodeQL on the pull request; the pre-existing resolver caught this, the
new set-aware one did not. Also replaces the deprecated JsonNode.fields()
with properties() and rejects an odd argument count in the test helper.
Adds JsonPatchSetTest for the set behaviour and the index overflow. Four
of its five cases fail against the unfixed code.
Removes the twelve json/yaml entries from known-missing-builtins.txt
(64 -> 52), leaving only builtins that are genuinely unimplemented.
Signed-off-by: Sebastian Spaink <sebastianspaink@gmail.com>
sspaink
force-pushed
the
fix/register-json-provider
branch
from
August 11, 2026 14:15
3aa42fd to
1445292
Compare
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
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.
Follow-up to #192, #193 and #194, and the last of the five unregistered providers. The
BuiltinProviderentry foropa-builtins-jsonwas commented out in itsMETA-INF/servicesfile, so none of the twelve json/yaml builtins were reachable for consumers.Registering it exposed 16 failing compliance cases:
json.marshal_with_optionsignoredprettyandprefixand used Jackson's layout. Go composesprefix + MarshalIndent(v, prefix, indent): the prefix leads the document and repeats on every line, the separator is": ", an empty container stays on one line, and an explicit"pretty": falsedisables indent and prefix. An unknown option key is a type error, which was not reported at all.json.match_schemaandjson.verify_schemaaccepted a schema with an unknown"type". The validator silently never matches it, whereas Go rejects the schema, so the schema is now checked up front. Their error objects also used networknt's field names and wording rather than thedesc/error/field/typeshape Go returns.json.patchcould not address Rego sets. Sets serialize as arrays, so their locations are recorded before serializing: a member is matched by value rather than index, an absent member appends, and the result is rebuilt as a set. Since the path segment is the member, anaddwhose value differs from the segment is undefined.json.is_validraised a type error for a non-string operand where Go returnsfalse.yaml.unmarshalnow reports Go's terseyaml: line N: <problem>instead of SnakeYAML's multi-line snippet. The residual wording difference (expected ',' or ']'vs yaml.v2'sdid not find expected ',' or ']') is recorded inComplianceTest's alternate-message map, as mapping every SnakeYAML diagnostic onto yaml.v2's phrasing is not tractable.This also raises the evaluator's test heap. The
jsonpatch/json_patch_testsfixture runs the whole upstream JSON Patch spec suite inside a single policy, and oncejson.patchresolves it needs more than Gradle's default 512m (1g is enough; 2g leaves headroom). That is a test-only limit, but it does sayjson.patchis memory-hungry on large inputs and is worth profiling separately.Adds
JsonPatchSetTestfor the set behaviour, three of whose four cases fail against the unfixed code.Removes the twelve json/yaml entries from
known-missing-builtins.txt(64 → 52). With all five providers registered, every remaining entry is a builtin that is genuinely unimplemented rather than merely unreachable.Note: #193 and #194 are still open and also edit
known-missing-builtins.txt, so whichever of the three merges later will need a rebase.