username/password/port should be removed when "file" is added as scheme

[watilde]

According to this section in the spec, the file scheme can't have username/password/port. I think they also should be removed automatically when file: is added as the scheme. Currently, they are left in the href even they cannot be overwritten.

e.g.

var url = new URL('http://user:[email protected]:21/aaa/zzz?l=24#test');
url.protocol = 'file:';
// url.href:
// + expected: file://foo.bar.com/aaa/zzz?l=24#test
// + actual: "file://user:pass%40foo.bar.com:21/aaa/zzz?l=24#test"

[annevk]

We should probably not allow setting to file under those conditions. That's what we do for similar situations elsewhere (e.g., setting host to the empty string when port is non-empty).

[watilde]

The basis why this behavior should avoid is that it allows having an invalid URL in the href:

var url = new URL('http://user:[email protected]:21/aaa/zzz?l=24#test');
url.protocol = 'file:';

var url2 = new URL(url.href);
// => Uncaught TypeError: Failed to construct 'URL': Invalid URL
//      at <anonymous>:1:1
//  (anonymous) @ VM326:1

As another workaround, it can escape the invalid character strings, but it's more predictable and natural to delete automatically imho because the properties such as username/password/port are independent of each other.

[annevk]

I still think making the setter fail (return early) is the most consistent.

[watilde]

I just noticed that URL allows adding an invalid URL into href:

var url = new URL('http://example.com');
url.href = 'file://user:pass%40foo.bar.com:21/aaa/zzz?l=24#test';

Then this issue is not an exception anymore. Fair enough :)

Thanks, I'm going to close this as resolved.

[annevk]

I don't think that's true per the URL Standard?

[annevk]

In particular, x = new URL("https://test/"); x.href = "https://test:test/" is supposed to throw.

[watilde]

The essential point of this issue report is that whether the latter part of the below is correct or not: Errors should be thrown when an invalid URL string is passed to the constructor, and also errors should be thrown too when an invalid property is set to the instance of the URL.

If x = new URL("https://test/"); x.href = "https://test:test/" is supposed to throw, then we could consider about the case to change its protocol that makes href invalid imo.

[annevk]

I'm not sure I follow what you're saying. Setting href throws for invalid input, but other properties typically do nothing (no-ops). That's not super consistent, but that was already how those properties behaved elsewhere in the platform and given all the minor inconsistencies across browsers not throwing ends up breaking less code as well.

[annevk]

I posted a PR, would appreciate a review @watilde. I'll also write some tests.

[watilde]

Wow that's great. I will check it now 😃