[go: up one dir, main page]
Skip to content

Commit

Permalink
Disallow bracketed hostnames.
Browse files Browse the repository at this point in the history 
Fixes #235
  • Loading branch information
@RubenVerborgh
RubenVerborgh committed Dec 30, 2023
1 parent 05629af commit 7a6567e
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 33 deletions.
64 changes: 42 additions & 22 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ var assert = require("assert");
var debug = require("./debug");

// Whether to use the native URL object or the legacy url module
var hasNativeURL = typeof URL !== "undefined";
var useNativeURL = false;
try {
assert(new URL());
}
catch (error) {
useNativeURL = error.code === "ERR_INVALID_URL";

This comment has been minimized.

Sign in to view

Copy link
@Electroid

Electroid Aug 29, 2024

@RubenVerborgh Hi there, just resurfacing this diff. useNativeURL appears to always be false.

❯ node --print 'new URL()'
node:internal/url:803
      throw new ERR_MISSING_ARGS('url');
      ^

TypeError [ERR_MISSING_ARGS]: The "url" argument must be specified
    at new URL (node:internal/url:803:13)
    at [eval]:1:1
    at node:internal/main/eval_string:51:3 {
  code: 'ERR_MISSING_ARGS'
}

Node.js v22.2.0

Since ERR_MISSING_ARGS != ERR_INVALID_URL, that means useNativeURL is always false in Node.js. We discovered this bug while testing Node.js compatiblity with Bun.

Did you mean to use new URL("") instead? That appears to show the correct error code.

❯ node --print 'new URL("")'
node:internal/url:814
    const href = bindingUrl.parse(input, base, raiseException);
                            ^

TypeError: Invalid URL
    at new URL (node:internal/url:814:29)
    at [eval]:1:1
    at node:internal/main/eval_string:51:3 {
  code: 'ERR_INVALID_URL',
  input: ''
}

Node.js v22.2.0

This comment has been minimized.

Sign in to view

Copy link
@RubenVerborgh

RubenVerborgh Sep 14, 2024

Author Collaborator

Since ERR_MISSING_ARGS != ERR_INVALID_URL, that means useNativeURL is always false in Node.js. We discovered this bug while testing Node.js compatiblity with Bun.

Good catch; fixed in 458ca8e
}

// URL fields to preserve in copy operations
var preservedUrlFields = [
Expand Down Expand Up @@ -493,27 +499,16 @@ function wrap(protocols) {

// Executes a request, following redirects
function request(input, options, callback) {
// Parse parameters
if (isString(input)) {
var parsed;
try {
parsed = spreadUrlObject(new URL(input));
}
catch (err) {
/* istanbul ignore next */
parsed = url.parse(input);
}
if (!isString(parsed.protocol)) {
throw new InvalidUrlError({ input });
}
input = parsed;
}
else if (hasNativeURL && (input instanceof URL)) {
// Parse parameters, ensuring that input is an object
if (isURL(input)) {
input = spreadUrlObject(input);
}
else if (isString(input)) {
input = spreadUrlObject(parseUrl(input));
}
else {
callback = options;
options = input;
options = validateUrl(input);
input = { protocol: protocol };
}
if (isFunction(options)) {
Expand Down Expand Up @@ -554,14 +549,35 @@ function wrap(protocols) {

function noop() { /* empty */ }

function parseUrl(string) {
/* istanbul ignore next */
return hasNativeURL ? new URL(string) : url.parse(string);
function parseUrl(input) {
var parsed;
/* istanbul ignore else */
if (useNativeURL) {
parsed = new URL(input);
}
else {
// Ensure the URL is valid and absolute
parsed = validateUrl(url.parse(input));
if (!isString(parsed.protocol)) {
throw new InvalidUrlError({ input });
}
}
return parsed;
}

function resolveUrl(relative, base) {
/* istanbul ignore next */
return hasNativeURL ? new URL(relative, base) : parseUrl(url.resolve(base, relative));
return useNativeURL ? new URL(relative, base) : parseUrl(url.resolve(base, relative));
}

function validateUrl(input) {
if (/^\[/.test(input.hostname) && !/^\[[:0-9a-f]+\]$/i.test(input.hostname)) {
throw new InvalidUrlError({ input: input.href || input });
}
if (/^\[/.test(input.host) && !/^\[[:0-9a-f]+\](:\d+)?$/i.test(input.host)) {
throw new InvalidUrlError({ input: input.href || input });
}
return input;
}

function spreadUrlObject(urlObject, target) {
Expand Down Expand Up @@ -646,6 +662,10 @@ function isBuffer(value) {
return typeof value === "object" && ("length" in value);
}

function isURL(value) {
return URL && value instanceof URL;
}

// Exports
module.exports = wrap({ http: http, https: https });
module.exports.wrap = wrap;
120 changes: 109 additions & 11 deletions test/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,67 @@ describe("follow-redirects", function () {
});
});

it("http.get to bracketed IPv4 address", function () {
var error = null;
try {
http.get("http://[127.0.0.1]:3600/a");
}
catch (err) {
error = err;
}
assert(error instanceof Error);
assert(error instanceof TypeError);
assert.equal(error.code, "ERR_INVALID_URL");
assert.equal(error.input, "http://[127.0.0.1]:3600/a");
});

it("http.get to bracketed IPv4 address specified as host", function () {
var error = null;
try {
http.get({
host: "[127.0.0.1]:3600",
path: "/a",
});
}
catch (err) {
error = err;
}
assert(error instanceof Error);
assert(error instanceof TypeError);
assert.equal(error.code, "ERR_INVALID_URL");
});

it("http.get to bracketed IPv4 address specified as hostname", function () {
var error = null;
try {
http.get({
hostname: "[127.0.0.1]",
port: 3600,
path: "/a",
});
}
catch (err) {
error = err;
}
assert(error instanceof Error);
assert(error instanceof TypeError);
assert.equal(error.code, "ERR_INVALID_URL");
});

it("http.get to bracketed hostname", function () {
var error = null;
try {
http.get("http://[localhost]:3600/a");
}
catch (err) {
error = err;
}
assert(error instanceof Error);
assert(error instanceof TypeError);
assert.equal(error.code, "ERR_INVALID_URL");
assert.equal(error.input, "http://[localhost]:3600/a");
});

it("http.get redirecting to IPv4 address", function () {
app.get("/a", redirectsTo("http://127.0.0.1:3600/b"));
app.get("/b", sendsJson({ a: "b" }));
Expand Down Expand Up @@ -241,6 +302,46 @@ describe("follow-redirects", function () {
});
});

it("http.get redirecting to bracketed IPv4 address", function () {
app.get("/a", redirectsTo("http://[127.0.0.1]:3600/b"));
app.get("/b", sendsJson({ a: "b" }));

return server.start(app)
.then(asPromise(function (resolve, reject) {
http.get("http://localhost:3600/a", concatJson(reject)).on("error", resolve);
}))
.then(function (error) {
assert(error instanceof Error);
assert.equal(error.code, "ERR_FR_REDIRECTION_FAILURE");

var cause = error.cause;
assert(cause instanceof Error);
assert(cause instanceof TypeError);
assert.equal(cause.code, "ERR_INVALID_URL");
assert.equal(cause.input, "http://[127.0.0.1]:3600/b");
});
});

it("http.get redirecting to bracketed hostname", function () {
app.get("/a", redirectsTo("http://[localhost]:3600/b"));
app.get("/b", sendsJson({ a: "b" }));

return server.start(app)
.then(asPromise(function (resolve, reject) {
http.get("http://localhost:3600/a", concatJson(reject)).on("error", resolve);
}))
.then(function (error) {
assert(error instanceof Error);
assert.equal(error.code, "ERR_FR_REDIRECTION_FAILURE");

var cause = error.cause;
assert(cause instanceof Error);
assert(cause instanceof TypeError);
assert.equal(cause.code, "ERR_INVALID_URL");
assert.equal(cause.input, "http://[localhost]:3600/b");
});
});

it("http.get with response event", function () {
app.get("/a", redirectsTo("/b"));
app.get("/b", redirectsTo("/c"));
Expand All @@ -266,8 +367,8 @@ describe("follow-redirects", function () {
try {
http.get("/relative");
}
catch (e) {
error = e;
catch (err) {
error = err;
}
assert(error instanceof Error);
assert(error instanceof TypeError);
Expand Down Expand Up @@ -963,9 +1064,9 @@ describe("follow-redirects", function () {
.then(asPromise(function (resolve, reject) {
http.get("http://localhost:3600/a")
.on("response", function () { return reject(new Error("unexpected response")); })
.on("error", reject);
.on("error", resolve);
}))
.catch(function (error) {
.then(function (error) {
assert(error instanceof Error);
assert.equal(error.message, "Redirected request failed: Unsupported protocol about:");

Expand Down Expand Up @@ -1266,8 +1367,8 @@ describe("follow-redirects", function () {
try {
req.write(12345678);
}
catch (e) {
error = e;
catch (err) {
error = err;
}
req.destroy();
assert(error instanceof Error);
Expand Down Expand Up @@ -2132,12 +2233,9 @@ describe("follow-redirects", function () {
throw new Error("no redirects!");
},
};
http.get(options, concatJson(resolve, reject)).on("error", reject);
http.get(options, concatJson(reject)).on("error", resolve);
}))
.then(function () {
assert.fail("request chain should have been aborted");
})
.catch(function (error) {
.then(function (error) {
assert(!redirected);
assert(error instanceof Error);
assert.equal(error.message, "Redirected request failed: no redirects!");
Expand Down

0 comments on commit 7a6567e

Please sign in to comment.