URI.js icon indicating copy to clipboard operation
URI.js copied to clipboard

bug: cannot get domain of URI missing protocol

Open jklmli opened this issue 10 years ago • 18 comments

> URI('portquiz.net:8').domain()
< ""

jklmli avatar Oct 26 '15 22:10 jklmli

host() and port()also don't work.

jklmli avatar Oct 26 '15 22:10 jklmli

> URI.parse('portquiz.net:8')
< Object {protocol: "portquiz.net", urn: true, path: "8"}

Looks due to https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L494 What protocols include .+-? https://github.com/medialize/URI.js/blob/gh-pages/src/URI.js#L199

jklmli avatar Oct 26 '15 22:10 jklmli

yes, that's known behavior - not a bug. see #232 or #187 for details.

rodneyrehm avatar Oct 27 '15 06:10 rodneyrehm

This seems like an unintuitive result for a very common scenario. Maybe have URI() and URI.parse() ignore the literal spec, and introduce a new function URI.strictParse() which follows it? Having every user write a preprocessing method is a bit wasteful.

jklmli avatar Nov 05 '15 22:11 jklmli

This seems like an unintuitive result for a very common scenario.

I don't disagree, judging by the number of people who got stumped by this so far.

Maybe have URI() and URI.parse() ignore the literal spec, and introduce a new function URI.strictParse() which follows it? Having every user write a preprocessing method is a bit wasteful.

Since the developer likely knows what data they're dealing with, they could go with URI.parseAsHttp(input) instead of URI(input) to state their expectation of working with an HTTP/S URL. We're certainly not changing the way URI works right now, but we can easily add this feature in a non-breaking way. Should we start by bikeshedding the API? Do you want to implement this and send a PR after discussions?

Here are a few quick thoughts

  1. URI.parseAsHttp(input) with leads to a number of new functions if we were to add support for more protocols
  2. because of the existing signature of URI.parse(inputString, outputMap), we can't easily add an options object as another argument. I'm not sure I like URI.parse({uri: inputString, option1: true}, outputMap) to get around that
  3. since URI.parse is an internal method and the proposed new function an external method (i.e. it is not ever used internally, only called by the developer to initiate working with URI), we could go with URI.parseWithDefault(inputString, optionsMap). This way optionsMap could contain defaults for all the components, not only the protocol. This looks dangerously close to URI(inputString, relativeBase), though.

rodneyrehm avatar Nov 06 '15 08:11 rodneyrehm

Maybe something like this:

> URI.parse('portquiz.net:8').withDefaults({protocol: 'http'})
< Object {protocol: "http", domain: "portquiz.net", port: "8"}

where withDefaults returns a new URI object?

jklmli avatar Nov 06 '15 08:11 jklmli

URI.parse('portquiz.net:8').withDefaults({protocol: 'http'})

that's not going to work (easily) because at the point withDefaults() is evaluated, the "wrong interpretation" of your input string has already happened.

rodneyrehm avatar Nov 06 '15 08:11 rodneyrehm

Sure, but we should be able to toString the URI object to get back the original string, and re-parse from there with the defaults.

jklmli avatar Nov 06 '15 08:11 jklmli

Sure, but we should be able to toString the URI object to get back the original string, and re-parse from there with the defaults.

why knowingly parse the string twice? unless you expect .withDefaults() to not engage very often (which is an assumption I'm not ready to share).

rodneyrehm avatar Nov 06 '15 08:11 rodneyrehm

Doesn't seem too expensive to do so, and makes the API clean.

Alternatively, you could flip the order to only parse once:

URI.withDefaults({protocol: 'http'}).parse('portquiz.net:8')

which is also pretty clean.

jklmli avatar Nov 06 '15 08:11 jklmli

Mind re-opening this issue to get more pairs of eyes reviewing any proposals?

jklmli avatar Nov 06 '15 08:11 jklmli

Why do you consider URI.withDefaults({protocol: 'http'}).parse('portquiz.net:8') better than URI.parseWithDefaults('portquiz.net:8', {protocol: 'http'})?

rodneyrehm avatar Nov 06 '15 08:11 rodneyrehm

I'm partial to the builder pattern for flexibility/future-proofing and readability. A lot of good discussion here which I'd do a disservice to if I were to try to summarize.

jklmli avatar Nov 06 '15 08:11 jklmli

In that case I'd prefer to go the functional programming route:

URI.withDefaults({ protocol: 'http' })('portquiz.net:8');

var httpUri = URI.withDefaults({ protocol: 'http' });
[ 'portquiz.net:8' ].map(httpUri);

This would allow the API user to do the work only once.

Now the next question (less question, more obstacle) is if the defaults transcend the URI.parse() method, so that the following behaves the same way:

var uri = URI.withDefaults({ protocol: 'http' })('portquiz.net:8');
uri.href('portquiz.net:9')

I guess URI.withDefaults() would work like a factory, bootstrapping the URI "class" so that any instance of that customized URI behaves consistently. We've dealt with this in an ugly and global way in the past, as URI.escapeQuerySpace can attest.

rodneyrehm avatar Nov 06 '15 10:11 rodneyrehm

Sure, that sounds really reasonable.

jklmli avatar Nov 25 '15 02:11 jklmli

I'm running into this as well. I want to parse a list of URLs that may or may not contain protocols, it may even just be a path. i.e.: ['http://hostname:port/path?query', 'hostname:port', '/path']

Based on: https://medialize.github.io/URI.js/about-uris.html It would think we could even have a flag that will ensure we always parse a URL instead of a URN? Or simply URI.parseURL(urlString) so I could do new URI(URI.parseURL(urlString))

mvanderlee avatar May 02 '18 17:05 mvanderlee

I'd love to see this implemented.

DoDSoftware avatar Dec 10 '18 09:12 DoDSoftware

I've implemented a possible solution using @MichielVanderlee 's suggestion.

https://github.com/medialize/URI.js/pull/374

This allows me to now do let uri = new URI(URI.toUri('somesite.com', {scheme: 'http'})); after which, uri.domain() will return the expected somesite.com

We'll see if it's worthy of being pulled in, but for now, it's working for my needs.

DoDSoftware avatar Dec 10 '18 12:12 DoDSoftware