Conversation
… context array
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.
These tests fail on the head of #481 (444b304) by design, so the CI of this PR is expected to be red until the findings of the second review on #481 are fixed. The eleven tests of the first review are part of #481 now, this branch replaces them with the tests of the second review.
The on-demand term definition on the compact IRI prefix path of
iri_expansionstill passes an empty base URL (only the step 3 path was fixed). Test "a prefix term defined on demand is parsed with the base url of the document" fails withCHECK( == http://ex/ctx.jsonld )(the scoped context is never requested), "invalid scoped context (loading document failed)", and in the second subcase "protected term redefinition".After a remote context with
"@propagate": false,previous_contextis set to the context from before the whole@contextarray instead of the context built by the earlier entries. Test "@propagate false in a remote context falls back to the earlier entries of the context array" fails withCHECK( 1 == 2 ), the quad<http://ex/o> <http://ex/a> "v"is missing.The
@importbranch keeps the URL as a view into the thread_local buffer ofIRIFactoryand uses it as cache key afterrequest_urlreturned. Test "two node objects that import the same context request it only once" fails withCHECK( 2 == 1 ); under ASan it reportsheap-use-after-free ... RemoteContextCache::resolve JsonLdContextParser.cpp:22.The
@importbranch callsset_resolution_basefor absolute URLs too, so an absolute@importinside a scoped context with an empty base URL fails. Test "an absolute @import url is loaded without a base url" fails withCHECK( == http://ex/imp.jsonld )and "invalid scoped context (loading remote context failed)".The nested
parse_contextstarted byhandle_remotevalidates the scoped contexts of the earlier array entries against the half-built context and clears theirneeds_context_check. Test "a scoped context before a remote context is validated against the complete context" fails with "invalid scoped context (invalid type mapping (not IRI or keyword))" and no quads.handle_nullreturns a context withoutprevious_context, so a type scoped context ofnullnever reverts for nested node objects. Test "a null scoped context that does not propagate keeps the previous context" fails withCHECK( 2 == 3 )(the quad<http://ex/o> <http://ex/p> "v"is missing) and, in the array subcase, with a quad that must not appear. This one exists on develop as well, the PR only moves the code.The
is_relativeguard inhandle_remoteremoved the validation of absolute context URLs. Test "an absolute remote context url that is no valid iri is rejected" fails withCHECK( http://ex ample/ctx.jsonld == ), the invalid URL reachesrequest_urland a quad is produced.A remote context body that is not valid JSON throws
simdjson_errorout ofparse_local_context, which is only caught at the top of the parser, so the remaining node objects of the document are lost. Test "a remote context that is no valid json only fails its own node object" fails withCHECK( EMPTY: no JSON found == invalid remote context )and the third node object missing, in three subcases (empty body, truncated body, empty body behind@import).Pass condition: for each test, a small local fix of the library (not part of this PR) was applied, the test passed (
test cases: 1 | 1 passed), and the fix was reverted. The@importcache key test was also run under ASan with the fix. With the library of the PR head, the full tests_JSON_LD_Parser run shows exactly these 8 tests failing and all 35 other tests passing.(prepared with claude)