url validation failing for valid urls

[cowley05]

I ran the downloaded version of the latest jar heatwave version 0.8.0 and started it up using java -jar nevergreen-standalone.jar, I entered the following in the add tray input: http://server:8153/go/cctray.xml

This fails validation and gives the following message: "url" is not a valid url

I have tried clearing cache, local storage etc. this does not work. This seems to be being caused by validate.js

screen shot of the error is attached.

It can be reproduced easily locally too by running the server and using a similar url.

[joejag]

Not good at all. Please use the IP address in the meantime if possible

[jimmythompson]

Taking a look at this on staging it looks like the problem is fixed. However, I tried the following URLs:

• https://
• https:// build.apache.org/cc.xml

These fail to validate in Heatwave, displaying a "not a valid url" message. In staging, however, the tray gets created and then fails to make the request, citing "500 - Expected authority at index 8: https://".

Personally, I prefer the error message in Heatwave (what's an "authority" in a URL, anyway?) Maybe a /bit/ more validation could be put in place? If not, I think it's a fair trade-off if you can now use hostnames in trays.

[GentlemanHal]

I updated it just to check if the input starts with http:// or https:// so that is why the url with the space in the host name is being allowed by the client but then failing on the server. We could make it a bit more complicated but then we are basically back down the regex path and we'll more likely end up block valid urls again.

I guess the question is, what is the client validation for?

I assume our users are very technical (developers) so we are just trying to stop simple mistakes like people typing things like build.apache.org/cc.xml assuming the http:// is implied (like you can in a browser)

If we do make the client validation more complicated we should also make sure the server validation matches, because it's possible for other's to use the API directly and having inconsistent validations could lead to confusing issues.

[joejag]

I'd be in favour of dropping the validation and instead auto-adding a http:// protocol if missing.

If the user gets an error for some invalid URL then they'll correct it. That's better than blocking legit URLs.

[GentlemanHal]

We can't drop the server side validation as we need to stop people using file:// urls. We could append the protocol if it was missing but we really should be encouraging people to use https:// especially on nevergreen.io, so I don't really like the idea of just adding http://

(as an aside I was actually thinking of opening a new issue to give the user a warning if they used the http://)

Maybe we could use a simple regex like ^https?:\/\/\S+$ that would catch a few more accidental typos.

[joejag]

Simple regex sounds good to me

For the aside: I'd only add a warning if there was a username:password being sent.

[GentlemanHal]

Have decided to go with your original suggestion @joejag. I've removed the validation from the client and have improved the responses from the server when you submit an invalid URL.

Also if you add no scheme the client will append http:// otherwise it'll just send whatever the user entered even if it's a mistype like htp://.

This does mean we might be making "obviously" incorrect requests, but given how it gives us a single source of truth and a simpler client I'm happy.

I'm going to close this for now, we can re-open or revisit later if anyone is unhappy.