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

Fails to find and validate schemas that are defined as a $ref by URN #423

Closed
sondrele opened this issue Feb 18, 2017 · 9 comments · Fixed by #544
Closed

Fails to find and validate schemas that are defined as a $ref by URN #423

sondrele opened this issue Feb 18, 2017 · 9 comments · Fixed by #544

Comments

@sondrele
Copy link
Contributor

The code is available on runkit:

https://runkit.com/sondrele/58a88642b5e12e0013c5bf15

What version of Ajv are you using? Does the issue happen if you use the latest version?
4.11.2 (also tested on 5.0.2-beta.0)

Ajv options object (see https://github.com/epoberezkin/ajv#options):

var Ajv = require('ajv@4.11.2');
ajv = new Ajv({
    v5: true,
    allErrors: true,
    errorDataPath: 'property',
    verbose: true,
    missingRefs: "fail",
    unknownFormats: 'ignore',
    multipleOfPrecision: 12,
    extendRefs: true,
    passContext: true,
    i18n: false
});

JSON Schema (please make it as small as possible to reproduce the issue):

var schema = {
    "type": "object",
    "properties": {
        "ip1": {
            "id": "urn:some:ip:prop",
            "type": "string",
            "format": "ipv4"
        },
        "ip2": {
            "$ref": "urn:some:ip:prop"
        }
    },
    "required": [ 
        "ip1", 
        "ip2"
    ]
};

Data (please make it as small as posssible to reproduce the issue):

var data = {
    "ip1": "0.0.0.0",
    "ip2": "0.0.0.0"
};

Your code (please use options, schema and data as variables):

console.log(ajv.validate(schema, data));

Validation result, data AFTER validation, error messages:

false

What results did you expect?
I had expected the data to be successfully validated, instead the validation ends with the error message "can't resolve reference urn:some:ip:prop from id #".

It seams like there might be a problem with the id being encoded as URN's, because the validation will succeed if we change the "id": "urn:some:ip:prop" to e.g. "id": "urn:some/ip:prop" (notice that I've only replaced the second : with a /).

When I'm looking at the object that keeps references to the internal schemas, I can see that the key for the ip1 schema gets translated to urn:some/ip:prop, so that when ajv tries to validate ip2 it fails to find the schema by it's original id.
It looks like the id's of the schemas might be treated like URL's instead of URN's, but I can't say this for sure.

@epoberezkin
Copy link
Member

Interesting. Never tried using URNs as refs

@sondrele
Copy link
Contributor Author

Yeah. I generate my schemas from java classes, so the class names gets serialized to a URN that identifies the schemas.

My current workaround is to translate all id's and $ref's before validating the schemas with ajv, but this is obviously not optimal.
If support for URN's is not on top of your priority list (so that it will take a while before it's implemented), I would happy to look into the issue instead.

@epoberezkin
Copy link
Member

epoberezkin commented Feb 19, 2017

I generate my schemas from java classes, so the class names gets serialized to a URN

@sondrele out of curiosity, what do you use to generate the schemas? We have some legacy Java code where it may be useful...

If support for URN's is not on top of your priority list (so that it will take a while before it's implemented), I would happy to look into the issue instead.

It wasn't in this list at all, to be honest :) Yes, I would definitely appreciate it. I think it is most likely caused by using node url package to parse and resolve URIs (and whatever browserify replaces it with in the browser). The replacement you use (if url package is indeed the reason) should not be much bigger, ideally even smaller :)

@sondrele
Copy link
Contributor Author

sondrele commented Feb 19, 2017

I was actually looking at it a little earlier today, and it is indeed the node-url package that's causing this.

My quick fix was to rename the url function on the object exported from the resolve module to uri, along with the following implementation:

// in the file lib/compile/resolve.js

// resolve.url = resolveUrl;
resolve.uri = resolveUri;

// ...

function resolveUri(baseId, id) {
  return /^urn:/i.test(id) ? id : resolveUrl(baseId, id);
}

//  the rest is like before...

function resolveUrl(baseId, id) {
  id = normalizeId(id);
  return url.resolve(baseId, id);
}

Looking at the URN syntax standard, it seems like it might be enough to check for whether the id starts with urn: to determine if it is a URN or a URL (depending on how thorough we want to be).

If you prefer to solve this with a library instead, it seams like the URI.js library is about the same size as node-url (a little smaller actually) and it looks like the API is fairly similar as well. I haven't gotten around to testing it out yet, but I will probably have some time to do it later this week if you think this is the better option :)

Ah, and I almost forgot.
We're currently extending the FasterXML jsonschema generator to support what we need for the draft v4/v5, it's not the prettiest, but it's working for the moment :)

@epoberezkin
Copy link
Member

epoberezkin commented Feb 19, 2017

It's not sufficient I think. If Ajv were to support URNs it has to support URN resolution, URNs with hash fragments, etc., etc.

Essentially all the tests that exist for URLs in refs have to copied and made into the tests that support URNs, both the tests that are in Ajv and in JSON-Schema-Test-Suite (these tests can be added there directly).

It's a bigger effort than making a simple case work, but I'd rather have no URN support than partial URN support.

@epoberezkin
Copy link
Member

epoberezkin commented Feb 19, 2017

draft v4/v5

If by v5 you mean a special mode in Ajv, it's not standardised and removed from the next version - see 5.0.1-beta.0 (although all the features and v5 schemas will be supported with options).

If you mean recently published draft-05, then the consensus is that it was rushed out, has many inaccuracies, and as it was supposed to just "clarify the language" it should be ignored and instead draft-06 that is going to be published soon should be used. Ajv 5.0.0 will support it, the lastest beta supports almost everything from draft-06 apart from renaming "id" keyword to "$id".

@epoberezkin
Copy link
Member

It's not sufficient I think.

I meant a simple fix. Replacing url with another library is the right approach, that would be very good if you could do it. Thanks.

@handrews
Copy link
Contributor

handrews commented May 5, 2017

It's not sufficient I think. If Ajv were to support URNs it has to support URN resolution

It seems to me that there are two possible levels of support for any particular URI scheme (including non-URL schemes like urn: and tag:):

  1. Usage as opaque identifiers (required by the spec)
  2. Support for automatic retrieval of the resource through scheme-specific resolution

I'm quite possibly missing a subtlety, but 1 seems supportable independent of scheme and should be the first step anyway- presumably you don't fetch HTTP URLs over the network if you have already loaded the resource the URL identifies.

@epoberezkin I gather your concern is to be able to clearly define the limitation, and it seems to me that if you document that 1 is supported but 2 is not, that is clear. 1 is completely independent of scheme (so it's not really "URN support" anyway), while 2 is scheme-specific. For 2, it's completely reasonable (and I believe compliant with the spec) to document that they are not automatically retrieved.

This would (to me) be the expected behavior for any scheme that is unknown to an implementation, whether URL or URN (which includes more than just urn:).

Fragment resolution is identical for all schemes, so as long as you can parse the fragment off that part should also work regardless of scheme or URL vs URN. If you have the resource, you can apply the fragment no matter how it was identified.

@brandontom
Copy link

This package change has caused AJV to fail for me.

I receive the following:
SyntaxError: Unexpected token 'const'
at webpack:///node_modules/ajv/node_modules/uri-js/dist/esnext/uri.js:39:0 <- karma-test-shim.js:41043

The error goes away if i don't call new Ajv() OR use Ajv 6.2.x instead of 6.4.x

for now i'm just going to stick with Ajv 6.2.x :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants