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

url.format does not postfix :// when parsed url is not fully qualified but a protocol is specified #4101

Closed
Jonahss opened this issue Dec 1, 2015 · 9 comments
Labels
url Issues and PRs related to the legacy built-in url module.

Comments

@Jonahss
Copy link

Jonahss commented Dec 1, 2015

Carried over from archived repo issue: nodejs/node-v0.x-archive#6100

In: https://nodejs.org/api/url.html#url_url_format_urlobj

It states that: "The protocols http, https, ftp, gopher, file will be postfixed with :// (colon-slash-slash)".

I'm unsure whether urlObjects are meant to be modified so they can be formatted again (although this seems reasonable).

It appears that if the parsed URL was not fully qualified then assigning a protocol doesn't postfix the :// (colon-slash-slash) as expected.

Code below demonstrates the problem:

var url = require("url");
var assert = require("assert");

var fqdnUrl = "http://www.google.com/";
var fqdnObject = url.parse(fqdnUrl);

assert.equal(fqdnObject.protocol, "http:");
assert.equal(url.format(fqdnObject), fqdnUrl);

var partialUrl = "www.google.com/";
var partialObject = url.parse(partialUrl);
assert(!partialObject.protocol);
partialObject.protocol = fqdnObject.protocol; // "http:" (but "http" should also work)
assert.equal(url.format(partialObject), fqdnUrl); // <-- This assertion fails. 
@mscdex mscdex added the url Issues and PRs related to the legacy built-in url module. label Dec 1, 2015
@claudiorodriguez
Copy link
Contributor

See #3361 and #4097

@claudiorodriguez
Copy link
Contributor

Seeing as how modifying the url module would be more drastic and could have unintended effects in userland, I'd say going with the doc fix is the safer approach, sent a PR.

@Jonahss
Copy link
Author

Jonahss commented Dec 2, 2015

Fixing the docs def fixes confusion.

Altering the url module would definitely effect many users, but the current behavior isn't very useful. I was hoping to use the url module to convert any partial/full url to a full url, eg:

google.com -> http://google.com
www.google.com -> http://www.google.com
http://www.google.com -> http://www.google.com

Should I publish a module to perform this common operation?

@claudiorodriguez
Copy link
Contributor

For your particular use case, isn't it sufficient to set "slashes" to true on the url object, then format?

@Jonahss
Copy link
Author

Jonahss commented Dec 4, 2015

I didn't try with slashes, thanks for pointing it out.

Unfortunately, when I set slashes to true, the formatted url came out with three slashes because when the simple "www.google.com" url is parsed, there is no hostname. (instead it's put into pathname/path).

let url = require('url')
let partial = url.parse('google.com')
console.log(url.format(partial))  // google.com

partial.protocol = 'http'
console.log(url.format(partial))  // http:google.com

partial.slashes = true
console.log(url.format(partial)) // http:///google.com

So I guess my function will need to do this:

partial = url.parse('google.com')
partial.protocol = 'http'
partial.slashes = true
partial.hostname = partial.pathname
partial.pathname = null

Of course, now if my original text had a path, it would never be parsed out.

I guess moral of the story is prepend "http://" to any input url that doesn't have a protocol and parse from there.

@claudiorodriguez
Copy link
Contributor

Yeah, the url module needs a breaking change to fix that stuff. After my PR for doc change lands, I'll get around submitting another one for this kind of stuff.

@Jonahss
Copy link
Author

Jonahss commented Dec 7, 2015

👍

@NamespaceValentine
Copy link

+1 is there a current workaround for this or an eta on completion?

@bnoordhuis
Copy link
Member

I'll close this out. url.format() is frozen for practical purposes, bugs and all, and superseded by the WHATWG URL implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
url Issues and PRs related to the legacy built-in url module.
Projects
None yet
Development

No branches or pull requests

5 participants