url.resolve auth issue

[stevenvachon]

require("url").resolve(
  "http://user:pass@fakeurl2.com:80/path/resource.html?query#hash",
  "http://fakeurl.com/path/resource.html?query#hash"
);

produces http://user:pass@fakeurl.com/path/resource.html?query#hash

Node.js has been doing this for as long as I've been using it. How can it be correct to add auth information from one absolute url to another absolute url?

[Fishrock123]

@stevenvachon what do you expect the output to be? The latter is supposed to be a relative URL.

[meandmycode]

Well, the documentation describes resolve as "Take a base URL, and a href URL, and resolve them as a browser would for an anchor tag. ".

So I would say that if it received a URL that had a protocol then it should just return that URL and not try to merge it in any way.

[Fishrock123]

cc @domenic?

[domenic]

Yeah seems very bad, confirmed. Does not match browsers (easy to test with new URL(input, base)) and I haven't confirmed with the spec but seems pretty unlikely to match it either.

[rlidwka]

I wonder what should happen in this case?

> url.resolve('mailto:user@example.org', 'example.com')
'mailto:user@example.com'

"user@" technically is auth info here.

[domenic]

I don't think so. @annevk can correct us but I am pretty sure that is a case where the "scheme data" comes into play, so the breakdown is scheme = mailto, scheme data = user@example.com, all others = null.

[stevenvachon]

is "scheme data" part of url.parse()? It uses "protocol" in the public export, anyway.

[domenic]

It's returned by pathname in browsers at least. https://url.spec.whatwg.org/#dom-urlutils-pathname

[annevk]

mailto doesn't support relative URLs.

[sholladay]

I would expect the example from @rlidwka ...

url.resolve('mailto:user@example.org', 'example.com')

... to return the first argument as-is, since this is a special case where the URL is already resolved to the maximum extent possible.

Resolving as the API describes is only applicable to certain schemes.

[stevenvachon]

mailto: (like javascript:) is a URI, not a URL. I would go as far as saying that the url lib should only deal with http/https/ftp. Meh, yeah, it's a URL without a password.

[domenic]

URI is a deprecated term anyway. No (web-compatible) software ever recognized the difference between URI, IRI, URL, URN, so they all were merged into URL, as documented in https://url.spec.whatwg.org.

[stevenvachon]

Cool. data: is now a URL with a "local scheme".

[PixnBits]

To clarify, are we expecting

require("url").resolve(
  "http://user:pass@fakeurl2.com:80/path/resource.html?query#hash",
  "http://fakeurl.com/path/resource.html?query#hash"
);

to produce
http://fakeurl.com/path/resource.html?query#hash? (If so, then I can submit PixnBits@499976c as a PR)

also, any thoughts on @sholladay's comment on mailto & domains? (Though maybe we should pull that out into a separate issue?)

[robcolburn]

@domenic,
So, just to clarify for @PixnBits. It's expected that this line in node.

require("url").resolve(baseUrl, hrefUrl);

Will behave exactly like this line in the browser?

new URL(hrefUrl, baseUrl).href;

Refs:

• https://nodejs.org/api/url.html#url_url_resolve_from_to
• https://developer.mozilla.org/en-US/docs/Web/API/URL/URL

[jasnell]

FYI.. as mentioned in #3893 we have two PRs that address this problem. One has been stalled for a while tho. I've got it on my todo to try to compare the two either later today or tomorrow. I'd definitely like to get this fixed soon tho.

[domenic]

@robcolburn yes. Although in general there are lots of failing test cases in that regard, the overall movement should be in that direction for any changes to the url module's behavior.

[robcolburn]

@domenic: sure. IMO: if we change url.resolve's behavior it's a breaking change -- and, cue "ripping off the bandage analogy". So, IMO if we can come up with our 100's of tests, fire them against a browser, and compare url's result we might be in good shape. Who knows maybe we can snag test-cases from firefox/webkit…

[domenic]

There are a bunch of test cases, and previous PRs have tried to do that :) https://github.com/w3c/web-platform-tests/blob/master/url/urltestdata.txt It's a big job though.

[robcolburn]

Ah, fun ;)

[stevenvachon]

Here's one that I wrote for broken-link-checker: https://github.com/stevenvachon/broken-link-checker/blob/master/test/json/linkObj.json

I'll be finishing urlobj in 2016.. maybe that can replace url, or something.