fix(ngInput): change URL_REGEXP to better match RFC3987

[andrewaustin]

The URL_REGEXP in use to perform validation in ngInput is too restrictive and fails to
follow RFC3987. In particular, it only accepts ftp, http, and https scheme components and
rejects perfectly valid schemes such as "file", "mailto", "chrome-extension",
etc. The regex also requires the scheme to be followed by two "/" but the RFC says
0 to n are acceptable. This change fixes both of these issues to better align to
the standard.

Closes #11341

[andrewaustin]

Does anything else need to be done for this?

[thatmarvin]

👍 I need to validate rtsp:// urls, so this would be useful for me too.

[dgieselaar]

Anything holding this back?

[Narretz]

Is the regey taken from somewhere or is it a custom one? I'd like to follow an established regex if possible. What does Chrome's regex look like these days?

[andrewaustin]

It's custom, if you can tell me where to find Google's I'll update it to match.

[gkalpak]

Why \w+ (in /^([0-9]+|\w+... instead of [A-Za-z]+ ?
It seems like \w+ will (incorrectly) match characters like _.

If fact, I would expect more something like: /^([A-Za-z][A-Za-z0-9.+-]+):...

[gkalpak]

Or are non-latin letters also allowed ?

[gkalpak]

According to KURL's implementation (used by WebKit (used by Chromium)), only A-Z, a-z and 0-9 are allowed: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/platform/weborigin/KURL.cpp&q=kurl.&sq=package:chromium&type=cs&l=116

[gkalpak]

A little research:

• (As expected), Chrome (<= Chromium <= WebKit) does not use a RegExp, they parse the URL instead.
• They use KURL for that.

I don't think it's worth implementing a URL parser into core.
Let's work on that RegExp :)

[gkalpak]

Based on a quick walk-through the code used in Chromium (and specifically url_parse.cc) and also considering the fact that we don't want to parse/break-up the url (only validate it), I would propose the following RegExp:

/^[A-Za-z][A-Za-z\d.+-]*:\/*(?:\w+(?::\w+)?@)?[^\s/]+(?::\d+)?(?:\/[\w#!:.?+=&%@\-/]*)?$/

This is not strictly following neither the RFC nor KURL's implementation, but I think it works fine for our needs (considering the previous RegExp wasn't following RFC/Chrome either).

I made some minor improvements to the original one (removed duplicate duplicate characters, removed unnecessary escapes, stopped tracking groups etc) and fixed a bug:

By trying to match the host with \S+ it would match everything including the host, port, path, query and fragment (since + matches eagerly). The intention was clearly to match the host only, so I replaced it with [^\s/]+ (which more closely matches KURL's implementation).

Besides the cases fixed by @andrewaustin's version, there are a couple more cases that are recognized incorrectly by his RegExp (which are also fixed with the above):

• 0scheme://example.com: Should be invalid (because scheme shouldn't start with a digit or .+-).
• http://example.com:9999/~~```: Should be invalid (because of ~`s ans ```s in the path).

[andrewaustin]

@gkalpak I updated the PR with your suggested REGEX. I also added two more tests for the two urls you pointed out that should be invalid.

[gkalpak]

It LGTM (obviously :P).
Considering how the previous RegExp didn't work perfectly either, I would be fine with this getting merged, if it looks good to some other team-member as well :)