uri format regexp is validating invalid URI

[vmaurin]

The regexp used by the "uri" format validation doesn't really validate that the string is a proper URI, i.e there is a chance passing a validated string to the java URI class will result into an exception

An example invalid URI that passes the validation is https://gitlab.com/?args=invalid|value

Should the regexp be more complex ? Should the validation be made with the java URI class (then catching the exception to validate) ?

[vmaurin]

The regexp in question is this one

json-schema-validator/src/main/java/com/networknt/schema/JsonMetaSchema.java

Line 50 in 72fe835

COMMON_BUILTIN_FORMATS.add(pattern("uri", "(^[a-zA-Z][a-zA-Z0-9+-.]*:[^\\s]*$)|(^//[^\\s]*$)"));

[stevehu]

@vmaurin The regexp definitely has room to improve. The reason I didn't use the Java URI class to verify the URL is due to performance. Most users are using this library to validate the HTTP request and it needs to be as fast as possible. I would suggest improving the regexp to make it cover more validation scenarios. The other option is to provide both options but default to regexp and with a configuration flag switch to Java URL class. What do you think?

[vmaurin]

I totally agree with the performance issue on relying on try/catch for validation, and also after a quick research, it sounds that URI java class is not perfect regarding validation either. My personal pick would be as you suggested, to enrich a bit the regexp. There are several example online, with this one as the most complete validation http://jmrware.com/articles/2009/uri_regexp/URI_regex.html but it might be also overkill. There are also some middle ground, like this one ^([a-z][a-z0-9+.-]+):(\/\/([^@]+@)?([a-z0-9.\-_~]+)(:\d+)?)?((?:[a-z0-9-._~]|%[a-f0-9]|[!$&'()*+,;=:@])+(?:\/(?:[a-z0-9-._~]|%[a-f0-9]|[!$&'()*+,;=:@])*)*|(?:\/(?:[a-z0-9-._~]|%[a-f0-9]|[!$&'()*+,;=:@])+)*)?(\?(?:[a-z0-9-._~]|%[a-f0-9]|[!$&'()*+,;=:@]|[/?])+)?(\#(?:[a-z0-9-._~]|%[a-f0-9]|[!$&'()*+,;=:@]|[/?])+)?$ that could be implemented, I just don't know what should be the position of json-schema-validator on this regards

[stevehu]

It looks good to me. Would you like to submit a PR to replace the old one? Thanks.

[vmaurin]

I just did but I ran in an other use case. So far, the protocol relative URL were considered valid, that is not the case anymore with the new regex. According json schema, we should follow the URI rfc, then I guess forbid protocol relative URL, but not sure it is something you desire

[stevehu]

What is the protocol-relative URL? Could you please provide an example? Thanks. We want to make sure it is backward compatible if possible. Otherwise, some users will have a broken application after upgrade the library.

[vmaurin]

It is actually a breaking change, see in my PR here https://github.com/networknt/json-schema-validator/pull/409/files#diff-cd18df339efd8831f215e37b5248aeb7dede17bac8013db8830dee30cd59b593L200 . The value asserted "valid" is not actually a valid URI, so there are two options :

• breaking backward compatibility but being more respectful of the json schema specs
• keeping backward compatibility but validating a URI that is not a correct one according the RFC

[vmaurin]

@stevehu I am waiting on your call. Any more restrictive regexp won't be backward compatible by design, it will invalidate value that were considered valid before. Either you want the library to become stricter along time, ensuring good validation but no backward compatibility regarding validation, or you want to keep the library as lax as today, but give options to be more strict if needed. Depending on your decision, I will adapt the PR

[stevehu]

We want to maintain backward compatibility; however, we cannot stop enhancing the library. Please submit a PR with your updated regexp. Thanks.

[vmaurin]

Ok so the PR is open here #409

[kamtschatka]

Because I just ran into the issue, that google font URLs are considered invalid:
Sample: "https://fonts.googleapis.com/css?family=Open+Sans:400,700|Source+Code+Pro:300,600|Titillium+Web:400,600,700"
Doc also says to use "|" and "," in the URLs: https://developers.google.com/fonts/docs/getting_started

The RFC says it is invalid, but seems like google didn't get the memo.
If you navigate to this URL in Chrome/Firefox and then access location.href it is also returned without the | and , URL encoded.

[stevehu]

If you get the address in the above URL, it is already encoded as a valid URL.

https://fonts.googleapis.com/css?family=Open+Sans:400,700%7CSource+Code+Pro:300,600%7CTitillium+Web:400,600,700

[kamtschatka]

no it is not. If you copy/paste it into the browser it shows like this:

If you click on the link above it is URL encoded, because github does that.

[stevehu]

I right-clicked the URL and copied the link address.

[kamtschatka]

as stated: this is github encoding the URL. Use the mouse to mark the URL and copy it.

[stevehu]

I don't know who encoded the URL. What I mean is only the valid URL should be used in JSON schema.

[vmaurin]

The regexp solution for validating a URI is not perfect here, but for sure a URI with | in the query string is not valid according the RFC and then doesn't fit the uri format restriction of json schema too, so it feels normal json schema validator is rejecting these URIs.
Here as @stevehu mentioned, either you pass a valid URI by percent encoding the pipes, or you change your json schema to relax the constraints

[kamtschatka]

yes, i have already created my own format, but what I am saying is: browsers AND the java implementation are not agreeing with the regex (which is RFC compliant from what I can see). As always when it comes to the web, the rules are not always applied.