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