Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

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

Closed
andrewwakeling opened this issue Aug 22, 2013 · 8 comments

Comments

@andrewwakeling
Copy link

In:

http://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. 
@ssafejava
Copy link

For reference, here is a runnable asserting this behavior.

It may be that this isn't actually a bug. RFC 3986#3 requires that URIs have a scheme, except in the case of relative URIs, defined in RFC 3986#4.2. A relative URI is required to have a leading '//'. Node's url.parse, in this case, identifies "www.google.com" as a path, not a host.

Without a defined host, L386 of url.js will not append slashes to the protocol.

After a quick reading of the spec it appears this is the correct behavior.

In the user's case, he could make this work by prepending '//' to his incoming URIs, or by setting partialObject.slashes = true.

@andrewwakeling
Copy link
Author

RFC 2616#3.2.2 indicates that it is not possible to use the "http" scheme without using the double slashes. ("https" scheme follows the same rules. RFC 2818#2.4 ). There are some grounds to always postfix :// with some schemes.

Not fussed if this isn't performed automatically, but it would be nice if the "slashes" property could be documented in url.parse and/or url.format.

(Also, any chance we can rename this class to URI instead of URL? I know that they are often casually used interchangeably, but URL is technically a subset of URI).

@karenism
Copy link

Even with
partialObject.slashes = true
url.format(partialObject) returns 'http:///www.google.com'

There are 3 slashes, not two, with v0.10.26

@louischatriot
Copy link

Hey guys,

Any news on this? Just bumped on this bug, behavior is definitely wrong and not in line with the doc ...

@trevnorris
Copy link

We're moving to make url operate more closely to the whatwg url spec. So either the docs should be updated to follow current behavior, if the behavior follows the spec, or we should get a patch landed to the url module.

@jasnell
Copy link
Member

jasnell commented Jun 3, 2015

See: nodejs/node#49

@Jonahss
Copy link

Jonahss commented Dec 1, 2015

I find that this error is still present in Node v5.0.0, and the issue mentioned above seems unrelated. Should a matching issue be created?

@MylesBorins
Copy link

oh hey @Jonahss

I think you totally should. Provided a minimal test case failing would help speed it up

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants