Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow // when base exists #553

Closed
watilde opened this issue Oct 10, 2020 · 5 comments
Closed

Allow // when base exists #553

watilde opened this issue Oct 10, 2020 · 5 comments

Comments

@watilde
Copy link

watilde commented Oct 10, 2020

As new URL('http://localhost:3000//') is allowed, new URL('//', 'http://localhost:3000') should work to have the same beheivour.

Steps to reproduce with whatwg-url

new URL('http://localhost:3000//').pathname
/*
=> '//'
*/

new URL('//', 'http://localhost:3000').pathname
/*
/home/daijiro/Developments/url/whatwg-url/dist/URL-impl.js:21
      throw new TypeError(`Invalid URL: ${url}`);
      ^

TypeError: Invalid URL: //
    at new URLImpl (/home/daijiro/Developments/url/whatwg-url/dist/URL-impl.js:21:13)
    at Object.exports.setup (/home/daijiro/Developments/url/whatwg-url/dist/URL.js:54:12)
    at new URL (/home/daijiro/Developments/url/whatwg-url/dist/URL.js:107:22)
    at Object.<anonymous> (/home/daijiro/Developments/url/whatwg-url/test.js:3:1)
    at Module._compile (internal/modules/cjs/loader.js:956:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)
    at Module.load (internal/modules/cjs/loader.js:812:32)
    at Function.Module._load (internal/modules/cjs/loader.js:724:14)
    at Function.Module.runMain (internal/modules/cjs/loader.js:1025:10)
    at internal/main/run_main_module.js:17:11
*/
@domenic
Copy link
Member

domenic commented Oct 10, 2020

I suspect this doesn't work because // is an incomplete, invalid form of a scheme-relative URL. To use a complete scheme-relative URL you need to do //foo or similar. Then it will work:

(new URL('//foo', 'http://localhost:3000')).href

// => "http://foo/"

@watilde
Copy link
Author

watilde commented Oct 11, 2020

Right, that's the current spec's behavior. What the original test case tried to handling // as is path-relative-URL instead. That is not valid at the moment, but it tries to reproduce http://localhost:3000//. The only way we can do that is to make the first item a clear relative path with a dot like the below:

new (URL('/.//', 'http://localhost:3000).href

// => "http://localhost:3000//"

I'm wondering if it's worth letting scheme-relative-special-URL string ignore the provided item only when it's an empty host "//" and leverage path-relative-URL to handle// which is what the original issue reporter expected. Otherwise, there is a case that we need to always give the first argument '/./' as a prefix to clearly make it a relative path.

@annevk
Copy link
Member

annevk commented Oct 12, 2020

Wouldn't it be extremely confusing that // would have very different behavior from //a?

Looking at nodejs/node#35458 I think what you want is something like this

let finalURL = null;
try {
  finalURL = new URL(req.url);
} catch (e) {
  if (req.url.startsWith("/") {
    finalURL = new URL(`http://${req.headers.host}${req.url}`); // can you trust .host though?
  }
}

but it's worth clarifying with the HTTP specification what the exact behavior should be there (e.g., this does not account for *).

@watilde
Copy link
Author

watilde commented Oct 19, 2020

This is a good point and I realized that we might not know if users want to handle the input as host or path in this case since // and //a are also valid value as path*.

Wouldn't it be extremely confusing that // would have very different behavior from //a?

*// and //a should be a valid value as path since new URL('https://example.com//') and new URL('https://example.com//a') work and their pathname are // and //a respectively in the order given.

@annevk
Copy link
Member

annevk commented Oct 20, 2021

I forgot above that setting pathname would also work.

https://datatracker.ietf.org/doc/html/draft-ietf-httpbis-semantics-19#section-4.1 is pretty clear that this is a special production that is different from the one that does not allow // and therefore it stands to reason it cannot be parsed normally. I would expect that any "relative URL" library would also need out-of-band information to support this.

Closing this therefore, but feel free to leave comments in case I missed something.

@annevk annevk closed this as completed Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants