Skip to content

Commit

Permalink
Prefer native URL instead of legacy url.resolve.
Browse files Browse the repository at this point in the history
  • Loading branch information
RubenVerborgh committed Dec 30, 2023
1 parent 72bc2a4 commit 1cba8e8
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 22 deletions.
17 changes: 13 additions & 4 deletions index.js
Expand Up @@ -6,6 +6,9 @@ var Writable = require("stream").Writable;
var assert = require("assert");

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var

This comment has been minimized.

Copy link
@RubenVerborgh

RubenVerborgh Jan 22, 2024

Author Collaborator

That would break the build on Node 4.

var debug = require("./debug");

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var


// Whether to use the native URL object or the legacy url module
var hasNativeURL = typeof URL !== "undefined";

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var


// Create handlers that pass events from native requests
var events = ["abort", "aborted", "connect", "error", "socket", "timeout"];

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var

var eventHandlers = Object.create(null);

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var

Expand All @@ -15,12 +18,12 @@ events.forEach(function (event) {
};
});

// Error types with codes
var InvalidUrlError = createErrorType(

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var

"ERR_INVALID_URL",
"Invalid URL",
TypeError
);
// Error types with codes
var RedirectionError = createErrorType(

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var

"ERR_FR_REDIRECTION_FAILURE",
"Redirected request failed"
Expand Down Expand Up @@ -426,7 +429,7 @@ RedirectableRequest.prototype._processResponse = function (response) {
url.format(Object.assign(currentUrlParts, { host: currentHost }));

// Determine the URL of the redirection
var redirectUrl = url.resolve(currentUrl, location);
var redirectUrl = resolveUrl(location, currentUrl);

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var


// Create the redirected request
debug("redirecting to", redirectUrl);
Expand Down Expand Up @@ -494,7 +497,7 @@ function wrap(protocols) {
}
input = parsed;
}
else if (URL && (input instanceof URL)) {
else if (hasNativeURL && (input instanceof URL)) {
input = urlToOptions(input);
}
else {
Expand Down Expand Up @@ -538,9 +541,15 @@ function wrap(protocols) {
return exports;
}

/* istanbul ignore next */
function noop() { /* empty */ }

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

if you aren't using this function, you can delete

This comment has been minimized.

Copy link
@rhuankarlus

rhuankarlus Jan 22, 2024

Jesus Christ @Renanluizssx ...
This function is used as a default value when we don't want to declare a specific function buddy.

This comment has been minimized.

Copy link
@RubenVerborgh

RubenVerborgh Jan 22, 2024

Author Collaborator

Just for my information and out of sheer curiosity, is there any specific purpose as to why people start commenting on random commits?


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

// from https://github.com/nodejs/node/blob/master/lib/internal/url.js
function urlToOptions(urlObject) {
var options = {

This comment has been minimized.

Copy link
@Renanluizssx

Renanluizssx Jan 22, 2024

you could use const instead of var

Expand Down
19 changes: 1 addition & 18 deletions test/test.js
Expand Up @@ -364,7 +364,7 @@ describe("follow-redirects", function () {
switch (error.cause.code) {
// Node 17+
case "ERR_INVALID_URL":
assert.equal(error.message, "Redirected request failed: Invalid URL");
assert.match(error.message, /^Redirected request failed: Invalid URL/);
break;
// Older Node versions
case "ERR_UNESCAPED_CHARACTERS":
Expand All @@ -376,23 +376,6 @@ describe("follow-redirects", function () {
});
});

it("emits an error when url.resolve fails", function () {
app.get("/a", redirectsTo("/b"));
var urlResolve = url.resolve;
url.resolve = function () {
throw new Error();
};

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

it("emits an error when the request fails for another reason", function () {
app.get("/a", function (req, res) {
res.socket.write("HTTP/1.1 301 Moved Permanently\r\n");
Expand Down

0 comments on commit 1cba8e8

Please sign in to comment.