Skip to content

IRI fixes #333

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Nov 19, 2019
+35 −46
Merged

IRI fixes #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4725fe2
Use rval rather than activeCtx when processing `@base` and `@vocab`.
gkellogg Nov 13, 2019
130069d
Check for compactIri yielding an absoluteIri where the scheme matches…
gkellogg Nov 13, 2019
d83d7ef
Better checking of absolute IRIs.
gkellogg Nov 13, 2019
3cfae0b
Terms that beging with a ':' are not considered absolute or compact I…
gkellogg Nov 13, 2019
223d7d7
`@vocab` can be a compact IRI.
gkellogg Nov 13, 2019
a812777
Don't use terms with expanded definitions as prefixes in 1.0 mode.
gkellogg Nov 14, 2019
3d18f9d
Apply suggestions fro @dlongleym code review
gkellogg Nov 14, 2019
804f0a7
Fix check for term having _prefix.
gkellogg Nov 14, 2019
c5ab700
Apply suggestions from @davidlehn code review
gkellogg Nov 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
### Changed
- Default processing mode changed to json-ld-1.1. Allows a 1.1 context to be
used after non-1.1 contexts.
- `@vocab` can be relative or a Compact IRI in 1.1, resolved against either a previous `@vocab`,
`@base` or document base.
- Better checking of absolute IRIs.
- Terms that begin with a ':' are not considered absolute or compact IRIs.
- Don't use terms with `"@prefix": false` or expanded term definitions to construct compact IRIs.

### Removed
- **BREAKING**: Remove callback API support. This includes removing support
Expand Down
11 changes: 11 additions & 0 deletions lib/compact.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,17 @@ api.compactIri = ({
return choice;
}

// If iri could be confused with a compact IRI using a term in this context,
// signal an error
for(const [term, td] of activeCtx.mappings) {
if(td && td._prefix && iri.startsWith(term + ':')) {
throw new JsonLdError(
`Absolute IRI "${iri}" confused with prefix "${term}".`,
'jsonld.SyntaxError',
{code: 'IRI confused with prefix', context: activeCtx});
}
}

// compact IRI relative to base
if(!relativeTo.vocab) {
return _removeBase(activeCtx['@base'], iri);
Expand Down
32 changes: 17 additions & 15 deletions lib/context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ api.process = async ({

// if not set explicitly, set processingMode to "json-ld-1.1"
rval.processingMode =
rval.processingMode || activeCtx.processingMode || 'json-ld-1.1';
rval.processingMode || activeCtx.processingMode;

// handle @base
if('@base' in ctx) {
Expand All @@ -205,7 +205,7 @@ api.process = async ({
} else if(_isAbsoluteIri(base)) {
base = parseUrl(base);
} else if(_isRelativeIri(base)) {
base = parseUrl(prependBase(activeCtx['@base'].href, base));
base = parseUrl(prependBase(rval['@base'].href, base));
} else {
throw new JsonLdError(
'Invalid JSON-LD syntax; the value of "@base" in a ' +
Expand All @@ -227,13 +227,14 @@ api.process = async ({
'Invalid JSON-LD syntax; the value of "@vocab" in a ' +
'@context must be a string or null.',
'jsonld.SyntaxError', {code: 'invalid vocab mapping', context: ctx});
} else if(!_isAbsoluteIri(value)) {
} else if(!_isAbsoluteIri(value) && api.processingMode(rval, 1.0)) {
throw new JsonLdError(
'Invalid JSON-LD syntax; the value of "@vocab" in a ' +
'@context must be an absolute IRI.',
'jsonld.SyntaxError', {code: 'invalid vocab mapping', context: ctx});
} else {
rval['@vocab'] = value;
rval['@vocab'] = _expandIri(rval, value, {vocab: true, base: true},
undefined, undefined, options);
}
defined.set('@vocab', true);
}
Expand Down Expand Up @@ -389,7 +390,7 @@ api.createTermDefinition = (
// always compute whether term has a colon as an optimization for
// _compactIri
const colon = term.indexOf(':');
mapping._termHasColon = (colon !== -1);
mapping._termHasColon = (colon > 0);

if('@reverse' in value) {
if('@id' in value) {
Expand Down Expand Up @@ -444,9 +445,9 @@ api.createTermDefinition = (
}
mapping['@id'] = id;
// indicate if this term may be used as a compact IRI prefix
mapping._prefix = (!mapping._termHasColon &&
id.match(/[:\/\?#\[\]@]$/) &&
(simpleTerm || api.processingMode(activeCtx, 1.0)));
mapping._prefix = (simpleTerm &&
!mapping._termHasColon &&
id.match(/[:\/\?#\[\]@]$/));
}
}

Expand Down Expand Up @@ -753,7 +754,7 @@ function _expandIri(activeCtx, value, relativeTo, localCtx, defined, options) {

// split value into prefix:suffix
const colon = value.indexOf(':');
if(colon !== -1) {
if(colon > 0) {
const prefix = value.substr(0, colon);
const suffix = value.substr(colon + 1);

Expand All @@ -769,13 +770,15 @@ function _expandIri(activeCtx, value, relativeTo, localCtx, defined, options) {
}

// use mapping if prefix is defined
if(activeCtx.mappings.has(prefix)) {
const mapping = activeCtx.mappings.get(prefix);
const mapping = activeCtx.mappings.get(prefix);
if(mapping && mapping._prefix) {
return mapping['@id'] + suffix;
}

// already absolute IRI
return value;
if(_isAbsoluteIri(value)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this extra check needed now? It still appears to be within the positive check for a colon in value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a colon isn’t sufficient to make it an absolute (or compact) IRI. This final test ensure that only an absolute IRI will be returned, otherwise, if falls through.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

return value;
}
}

// prepend vocab
Expand Down Expand Up @@ -1086,11 +1089,10 @@ api.getAllContexts = async (input, options) => {
*/
api.processingMode = (activeCtx, version) => {
if(version.toString() >= '1.1') {
return activeCtx.processingMode &&
return !activeCtx.processingMode ||
activeCtx.processingMode >= 'json-ld-' + version.toString();
} else {
return !activeCtx.processingMode ||
activeCtx.processingMode === 'json-ld-1.0';
return activeCtx.processingMode === 'json-ld-1.0';
}
};

Expand Down
2 changes: 1 addition & 1 deletion lib/url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ api.prependBase = (base, iri) => {
return iri;
}
// already an absolute IRI
if(iri.indexOf(':') !== -1) {
if(api.isAbsolute(iri)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidlehn -- it would be good to understand the performance impact on things like this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have benchmarking code. I'm not sure what the tests would be.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While a regex comparison is more compute intensive than an indexOf, they should be pretty similar. In any case, it's incorrect to just look for a ':' as a test for if an IRI is absolute.

return iri;
}

Expand Down
31 changes: 1 addition & 30 deletions tests/test-common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,9 @@ const TEST_TYPES = {
specVersion: ['json-ld-1.0'],
// FIXME
idRegex: [
// terms
/compact-manifest.jsonld#tp001$/,
// rel iri
/compact-manifest.jsonld#t0095$/,
// type set
/compact-manifest.jsonld#t0104$/,
/compact-manifest.jsonld#t0105$/,
// rel vocab
/compact-manifest.jsonld#t0107$/,
// @type: @none
/compact-manifest.jsonld#ttn01$/,
/compact-manifest.jsonld#ttn02$/,
Expand All @@ -52,8 +46,6 @@ const TEST_TYPES = {
/compact-manifest.jsonld#tpi04$/,
/compact-manifest.jsonld#tpi05$/,
/compact-manifest.jsonld#tpi06$/,
// IRI confusion
/compact-manifest.jsonld#te002$/,
// @propogate
/compact-manifest.jsonld#tc026$/,
/compact-manifest.jsonld#tc027$/,
Expand Down Expand Up @@ -117,17 +109,6 @@ const TEST_TYPES = {
/expand-manifest.jsonld#t0102$/,
// multiple graphs
/expand-manifest.jsonld#t0103$/,
// rel iri
/expand-manifest.jsonld#t0092$/,
// iris
/expand-manifest.jsonld#t0109$/,
// rel vocab
/expand-manifest.jsonld#t0110$/,
/expand-manifest.jsonld#t0111$/,
/expand-manifest.jsonld#t0112$/,
// terms beginning with ':'
/expand-manifest.jsonld#t0117$/,
/expand-manifest.jsonld#t0118$/,
// terms having form of keyword
/expand-manifest.jsonld#t0119$/,
/expand-manifest.jsonld#t0120$/,
Expand Down Expand Up @@ -180,8 +161,6 @@ const TEST_TYPES = {
/expand-manifest.jsonld#te049$/,
// invalid keyword alias
/expand-manifest.jsonld#te051$/,
// IRI prefixes
/expand-manifest.jsonld#tpr29$/,
// protected null IRI mapping
/expand-manifest.jsonld#tpr28$/,
// remote
Expand Down Expand Up @@ -214,9 +193,6 @@ const TEST_TYPES = {
/expand-manifest.jsonld#tso11$/,
// colliding keywords
/expand-manifest.jsonld#t0114$/,
// vocab iri/term
/expand-manifest.jsonld#te046$/,
/expand-manifest.jsonld#te047$/,
// included
/expand-manifest.jsonld#tin01$/,
/expand-manifest.jsonld#tin02$/,
Expand Down Expand Up @@ -416,9 +392,6 @@ const TEST_TYPES = {
idRegex: [
// blank node properties
/toRdf-manifest.jsonld#t0118$/,
// terms beginning with ':'
/toRdf-manifest.jsonld#te117$/,
/toRdf-manifest.jsonld#te118$/,
// terms having form of keyword
/toRdf-manifest.jsonld#te119$/,
/toRdf-manifest.jsonld#te120$/,
Expand Down Expand Up @@ -460,10 +433,8 @@ const TEST_TYPES = {
/toRdf-manifest.jsonld#t0132$/,
// @vocab mapping
/toRdf-manifest.jsonld#te075$/,
// rel IRI
/toRdf-manifest.jsonld#te092$/,
/toRdf-manifest.jsonld#te109$/,
/toRdf-manifest.jsonld#te110$/,
// Invalid Statement
/toRdf-manifest.jsonld#te111$/,
/toRdf-manifest.jsonld#te112$/,
// index maps
Expand Down