unshiftio / url-parse

Small footprint URL parser that works seamlessly across Node.js and browser environments.
http://unshift.io
MIT License
1.03k stars 104 forks source link

origin defaults to "null" #38

Open gt53 opened 8 years ago

gt53 commented 8 years ago

I noticed that origin defaults to "null" unlike any of the other default values that the parser returns and was wondering if that is intentional. Here's what I get when passing an empty string to the parser:

{
  protocol: '',
  slashes: false,
  hash: '',
  query: '',
  pathname: '',
  auth: '',
  host: '',
  port: '',
  hostname: '',
  password: '',
  username: '',
  origin: 'null',
  href: ''
}

Besides the inconsistency, "null" evaluates truthily, which seems potentially problematic to url-parse users.

If an empty string is ultimately desired for origin when no value is present, I can submit a PR for that.

lpinca commented 8 years ago

As bad as it is I think it makes sense, see https://url.spec.whatwg.org/#origin. We should return an opaque origin whose value is the 'null' string.

3rd-Eden commented 8 years ago

@lpinca I'm starting to doubt here, it never says it's a string so it might have been just the null it self..

lpinca commented 8 years ago

@3rd-Eden run new URL('file:///foo') in Firefox console and you'll see that it is a string.

lpinca commented 8 years ago

From https://url.spec.whatwg.org/#urlutils-members

The origin attribute’s getter must return the Unicode serialization of context object’s url’s origin.

and https://html.spec.whatwg.org/multipage/browsers.html#unicode-serialisation-of-an-origin

  1. If origin is an opaque origin, then return "null".

So I think we are doing it correctly.

lpinca commented 8 years ago

There is actually a case where browsers return an empty string for origin and that is when the URL is invalid:

> var a = document.createElement('a');
undefined
> a.href
""
> a.origin
""

so maybe we can use the empty string as well when we pass an empty string to the parser? Another wrong behavior I noticed is the following:

> parse('sip:alice@atlanta.com')
URL {
  protocol: 'sip:',
  slashes: false,
  hash: '',
  query: '',
  pathname: '',
  auth: 'alice',
  host: 'atlanta.com',
  port: '',
  hostname: 'atlanta.com',
  password: '',
  username: 'alice',
  origin: 'sip://atlanta.com',
  href: 'sip:alice@atlanta.com' }

in this case I think origin should be 'null' as ['blob', 'ftp', 'gopher', 'http', 'https', 'ws', 'wss', 'file'].indexOf('sip') === -1.

3rd-Eden commented 8 years ago

Its that technically considered an URL?

On Sep 2, 2016, at 7:51 PM, Luigi Pinca notifications@github.com wrote:

There is actually a case where browsers return an empty string for origin and that is when the URL is invalid:

var a = document.createElement('a'); undefined a.href "" a.origin "" href = 'foo' "foo" a.origin "" so maybe we can use the empty string as well when we pass an empty string to the parser? Another wrong behavior I noticed is the following:

parse('sip:alice@atlanta.com') URL { protocol: 'sip:', slashes: false, hash: '', query: '', pathname: '', auth: 'alice', host: 'atlanta.com', port: '', hostname: 'atlanta.com', password: '', username: 'alice', origin: 'sip://atlanta.com', href: 'sip:alice@atlanta.com' } in this case I think origin should be 'null' as ['blob', 'ftp', 'gopher', 'http', 'https', 'ws', 'wss', 'file'].indexOf('sip') === -1.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

lpinca commented 8 years ago

@3rd-Eden what are you referring to specifically? The <a> element?