Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify URI resolution relative to base without trailing slash #430

Merged
merged 2 commits into from Mar 25, 2020

Conversation

@fsteeg
Copy link
Member

fsteeg commented Mar 24, 2020

This came up when fixing jsonld-java/jsonld-java#279 in jsonld-java/jsonld-java#280, where we noticed the original issue described on the Apache Jena list was not covered by the spec tests.

Relative resolution in RFC 3986 [1][2] (see 8) in spec [3]):

  • rel relative to example is example/rel
  • rel relative to example/base is also example/rel

Already tested elsewhere, for clarity here, with trailing slash:

  • rel relative to example/base/ is example/base/rel

[1] https://tools.ietf.org/html/rfc3986#section-5.2
[2] https://blog.cdivilly.com/2019/02/28/uri-trailing-slashes
[3] https://w3c.github.io/json-ld-api/#algorithm-4

Relative resolution in RFC 3986 [1][2] (see `8)` in spec [3]):

- `rel` relative to `example` is `example/rel`
- `rel` relative to `example/base` is also `example/rel`

Already tested elsewhere, for clarity here, with trailing slash:

- `rel` relative to `example/base/` is `example/base/rel`

[1] https://tools.ietf.org/html/rfc3986#section-5.2
[2] https://blog.cdivilly.com/2019/02/28/uri-trailing-slashes
[3] https://w3c.github.io/json-ld-api/#algorithm-4
@gkellogg

This comment has been minimized.

Copy link
Member

gkellogg commented Mar 24, 2020

0130 seems to be similar to toRdf/0128, but there isn't an expand version of this AFAICT.

Ruby passes them all okay, but pyld fails 0129, so clearly testing a corner case. Looks like the playground (jsonld.js) isn't passing 0129 either.

We'll need toRdf versions of these as well, and both expand-manifest.html (and toRdf-manifest.html) should be updated using the rake task, but if you have problems with this, I can update after the fact.

@gkellogg gkellogg requested review from davidlehn and rubensworks Mar 24, 2020
@gkellogg

This comment has been minimized.

Copy link
Member

gkellogg commented Mar 24, 2020

Also, note that toRdf versions should be named "e129" and "e130" to match the naming scheme for other expansion tests brought over.

See #430
@fsteeg

This comment has been minimized.

Copy link
Member Author

fsteeg commented Mar 25, 2020

Thanks @gkellogg, I've added toRdf tests and the rake output. Hope I got it right.

@gkellogg

This comment has been minimized.

Copy link
Member

gkellogg commented Mar 25, 2020

Thanks for the extra effort, @fsteeg!

@gkellogg gkellogg merged commit 12f0fa4 into w3c:master Mar 25, 2020
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
ipr PR deemed acceptable as non-substantive by @fsteeg.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.