Skip to content

Commit 63d3a62

Browse files
williamsteinwilliamstein
committed
put use of new URL(...) in one place and support deprecated url.parse
1 parent bcf0793 commit 63d3a62

File tree

7 files changed

+153
-129
lines changed

7 files changed

+153
-129
lines changed

‎lib/http-proxy/common.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ProxyTargetDetailed, ServerOptions } from "./index";
22
import { type IncomingMessage as Request } from "http";
33
import { TLSSocket } from "tls";
44
import type { Socket } from "net";
5+
import * as urllib from "url";
56

67
const upgradeHeader = /(^|,)\s*upgrade\s*($|,)/i;
78

@@ -233,10 +234,21 @@ function hasPort(host: string): boolean {
233234
}
234235

235236
function getPath(url?: string): string {
236-
const u = new URL(url ?? "", "http://dummy");
237+
const u = toURL(url);
237238
return `${u.pathname ?? ""}${u.search ?? ""}`;
238239
}
239240

241+
export function toURL(url: URL | urllib.Url | string | undefined): URL {
242+
if (url instanceof URL) {
243+
return url;
244+
} else if (typeof url === "object" && typeof url.href === "string") {
245+
// urllib.Url is deprecated but we support it by converting to URL
246+
return new URL(url.href, "http://dummy.org");
247+
} else {
248+
return new URL(url ?? "", "http://dummy.org");
249+
}
250+
}
251+
240252
// vendor simplified version of https://www.npmjs.com/package/requires-port to
241253
// reduce dep and add typescript.
242254
function required(port: number, protocol: string): boolean {

‎lib/http-proxy/index.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { WS_PASSES } from "./passes/ws-incoming";
55
import { EventEmitter } from "events";
66
import type { Stream } from "stream";
77
import debug from "debug";
8+
import { toURL } from "./common";
89

910
const log = debug("http-proxy-3");
1011

@@ -139,14 +140,14 @@ export class ProxyServer extends EventEmitter {
139140

140141
for (const e of ["target", "forward"]) {
141142
if (typeof requestOptions[e] === "string") {
142-
requestOptions[e] = new URL(requestOptions[e], "http://dummy.org");
143+
requestOptions[e] = toURL(requestOptions[e]);
143144
}
144145
}
145146

146147
if (!requestOptions.target && !requestOptions.forward) {
147148
this.emit(
148149
"error",
149-
new Error("Must provide a proper URL as target or forward"),
150+
new Error("Must set target or forward"),
150151
);
151152
return;
152153
}

‎lib/http-proxy/passes/web-outgoing.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ export function setRedirectHostRewrite(
5454
proxyRes.headers["location"] &&
5555
redirectRegex.test(`${proxyRes.statusCode}`)
5656
) {
57-
const target = new URL(options.target, "http://dummy.org");
57+
const target = common.toURL(options.target);
5858
const location = proxyRes.headers["location"];
5959
if (typeof location != "string") {
6060
return;
6161
}
62-
const u = new URL(location, "http://dummy.org");
62+
const u = common.toURL(location);
6363

6464
// make sure the redirected host matches the target host before rewriting
6565
if (target.host != u.host) {

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "http-proxy-3",
3-
"version": "1.20.2",
3+
"version": "1.20.3",
44
"repository": {
55
"type": "git",
66
"url": "https://github.com/sagemathinc/http-proxy-3.git"

‎test/http/forward-and-target-proxy.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ describe("Example of proxying over HTTP with additional forward proxy to a diffe
7575
it("Makes a request to the proxy and sees that counts go up", async () => {
7676
const before = { ...counts };
7777
const b = await (await fetch(`http://localhost:${ports.proxy}`)).text();
78-
// This b is supposed to be empty, because the
7978
expect(b).toContain("request successfully proxied to");
8079
await wait({
8180
until: () =>

‎test/http/forward-proxy.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ describe("Example of proxying over HTTP with additional forward proxy", () => {
5656
expect(a).toContain("request successfully forwarded to");
5757
const before = numRequests;
5858
const b = await (await fetch(`http://localhost:${proxyPort}`)).text();
59-
// This b is supposed to be empty, because the
59+
// This b is supposed to be empty
6060
expect(b).toContain("");
6161
// Handling of the forward on the remote server is totally decoupled, so we
6262
// just have to wait:
@@ -84,7 +84,6 @@ describe("Example of proxying over HTTP with additional forward proxy", () => {
8484

8585
const before = numRequests;
8686
const b = await (await fetch(`http://localhost:${proxy2Port}`)).text();
87-
// This b is supposed to be empty, because the
8887
expect(b).toContain("request successfully forwarded to");
8988
await wait({ until: () => numRequests >= before + 2 });
9089
// indeed, the remote server did get a request TWICE, once from the

‎test/lib/http-proxy-passes-web-outgoing.test.ts

Lines changed: 133 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -5,156 +5,169 @@ import {
55
writeStatusCode,
66
writeHeaders,
77
} from "../../dist/lib/http-proxy/passes/web-outgoing";
8+
import * as url from "url";
89

910
const state: any = { headers: {} };
1011

11-
describe("#setRedirectHostRewrite", () => {
12-
beforeEach(() => {
13-
state.req = {
14-
headers: {
15-
host: "ext-auto.com",
16-
},
17-
};
18-
state.proxyRes = {
19-
statusCode: 301,
20-
headers: {
21-
location: "http://backend.com/",
22-
},
23-
};
24-
state.options = {
25-
target: "http://backend.com",
26-
};
27-
});
28-
29-
describe("rewrites location host with hostRewrite", () => {
12+
// NOTE: here url.parse("http://backend.com") uses the deprecated url.parse
13+
// function, and we're testing that we still support it.
14+
for (const target of ["http://backend.com", url.parse("http://backend.com")]) {
15+
describe("#setRedirectHostRewrite", () => {
3016
beforeEach(() => {
31-
state.options.hostRewrite = "ext-manual.com";
17+
state.req = {
18+
headers: {
19+
host: "ext-auto.com",
20+
},
21+
};
22+
state.proxyRes = {
23+
statusCode: 301,
24+
headers: {
25+
location: "http://backend.com/",
26+
},
27+
};
28+
state.options = {
29+
target,
30+
};
3231
});
33-
[201, 301, 302, 307, 308].forEach(function (code) {
34-
it("on " + code, () => {
35-
state.proxyRes.statusCode = code;
32+
33+
describe("rewrites location host with hostRewrite", () => {
34+
beforeEach(() => {
35+
state.options.hostRewrite = "ext-manual.com";
36+
});
37+
[201, 301, 302, 307, 308].forEach(function (code) {
38+
it("on " + code, () => {
39+
state.proxyRes.statusCode = code;
40+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
41+
expect(state.proxyRes.headers.location).toEqual(
42+
"http://ext-manual.com/",
43+
);
44+
});
45+
});
46+
47+
it("not on 200", () => {
48+
state.proxyRes.statusCode = 200;
49+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
50+
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
51+
});
52+
53+
it("not when hostRewrite is unset", () => {
54+
delete state.options.hostRewrite;
55+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
56+
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
57+
});
58+
59+
it("takes precedence over autoRewrite", () => {
60+
state.options.autoRewrite = true;
3661
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
3762
expect(state.proxyRes.headers.location).toEqual(
3863
"http://ext-manual.com/",
3964
);
4065
});
41-
});
4266

43-
it("not on 200", () => {
44-
state.proxyRes.statusCode = 200;
45-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
46-
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
47-
});
48-
49-
it("not when hostRewrite is unset", () => {
50-
delete state.options.hostRewrite;
51-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
52-
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
53-
});
54-
55-
it("takes precedence over autoRewrite", () => {
56-
state.options.autoRewrite = true;
57-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
58-
expect(state.proxyRes.headers.location).toEqual("http://ext-manual.com/");
59-
});
67+
it("not when the redirected location does not match target host", () => {
68+
state.proxyRes.statusCode = 302;
69+
state.proxyRes.headers.location = "http://some-other/";
70+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
71+
expect(state.proxyRes.headers.location).toEqual("http://some-other/");
72+
});
6073

61-
it("not when the redirected location does not match target host", () => {
62-
state.proxyRes.statusCode = 302;
63-
state.proxyRes.headers.location = "http://some-other/";
64-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
65-
expect(state.proxyRes.headers.location).toEqual("http://some-other/");
74+
it("not when the redirected location does not match target port", () => {
75+
state.proxyRes.statusCode = 302;
76+
state.proxyRes.headers.location = "http://backend.com:8080/";
77+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
78+
expect(state.proxyRes.headers.location).toEqual(
79+
"http://backend.com:8080/",
80+
);
81+
});
6682
});
6783

68-
it("not when the redirected location does not match target port", () => {
69-
state.proxyRes.statusCode = 302;
70-
state.proxyRes.headers.location = "http://backend.com:8080/";
71-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
72-
expect(state.proxyRes.headers.location).toEqual(
73-
"http://backend.com:8080/",
74-
);
75-
});
76-
});
84+
describe("rewrites location host with autoRewrite", () => {
85+
beforeEach(() => {
86+
state.options.autoRewrite = true;
87+
});
88+
[201, 301, 302, 307, 308].forEach(function (code) {
89+
it("on " + code, () => {
90+
state.proxyRes.statusCode = code;
91+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
92+
expect(state.proxyRes.headers.location).toEqual(
93+
"http://ext-auto.com/",
94+
);
95+
});
96+
});
7797

78-
describe("rewrites location host with autoRewrite", () => {
79-
beforeEach(() => {
80-
state.options.autoRewrite = true;
81-
});
82-
[201, 301, 302, 307, 308].forEach(function (code) {
83-
it("on " + code, () => {
84-
state.proxyRes.statusCode = code;
98+
it("not on 200", () => {
99+
state.proxyRes.statusCode = 200;
85100
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
86-
expect(state.proxyRes.headers.location).toEqual("http://ext-auto.com/");
101+
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
87102
});
88-
});
89103

90-
it("not on 200", () => {
91-
state.proxyRes.statusCode = 200;
92-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
93-
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
94-
});
104+
it("not when autoRewrite is unset", () => {
105+
delete state.options.autoRewrite;
106+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
107+
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
108+
});
95109

96-
it("not when autoRewrite is unset", () => {
97-
delete state.options.autoRewrite;
98-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
99-
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
100-
});
110+
it("not when the redirected location does not match target host", () => {
111+
state.proxyRes.statusCode = 302;
112+
state.proxyRes.headers.location = "http://some-other/";
113+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
114+
expect(state.proxyRes.headers.location).toEqual("http://some-other/");
115+
});
101116

102-
it("not when the redirected location does not match target host", () => {
103-
state.proxyRes.statusCode = 302;
104-
state.proxyRes.headers.location = "http://some-other/";
105-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
106-
expect(state.proxyRes.headers.location).toEqual("http://some-other/");
117+
it("not when the redirected location does not match target port", () => {
118+
state.proxyRes.statusCode = 302;
119+
state.proxyRes.headers.location = "http://backend.com:8080/";
120+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
121+
expect(state.proxyRes.headers.location).toEqual(
122+
"http://backend.com:8080/",
123+
);
124+
});
107125
});
108126

109-
it("not when the redirected location does not match target port", () => {
110-
state.proxyRes.statusCode = 302;
111-
state.proxyRes.headers.location = "http://backend.com:8080/";
112-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
113-
expect(state.proxyRes.headers.location).toEqual(
114-
"http://backend.com:8080/",
115-
);
116-
});
117-
});
127+
describe("rewrites location protocol with protocolRewrite", () => {
128+
beforeEach(() => {
129+
state.options.protocolRewrite = "https";
130+
});
131+
[201, 301, 302, 307, 308].forEach(function (code) {
132+
it("on " + code, () => {
133+
state.proxyRes.statusCode = code;
134+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
135+
expect(state.proxyRes.headers.location).toEqual(
136+
"https://backend.com/",
137+
);
138+
});
139+
});
118140

119-
describe("rewrites location protocol with protocolRewrite", () => {
120-
beforeEach(() => {
121-
state.options.protocolRewrite = "https";
122-
});
123-
[201, 301, 302, 307, 308].forEach(function (code) {
124-
it("on " + code, () => {
125-
state.proxyRes.statusCode = code;
141+
it("not on 200", () => {
142+
state.proxyRes.statusCode = 200;
126143
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
127-
expect(state.proxyRes.headers.location).toEqual("https://backend.com/");
144+
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
128145
});
129-
});
130-
131-
it("not on 200", () => {
132-
state.proxyRes.statusCode = 200;
133-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
134-
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
135-
});
136146

137-
it("not when protocolRewrite is unset", () => {
138-
delete state.options.protocolRewrite;
139-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
140-
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
141-
});
147+
it("not when protocolRewrite is unset", () => {
148+
delete state.options.protocolRewrite;
149+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
150+
expect(state.proxyRes.headers.location).toEqual("http://backend.com/");
151+
});
142152

143-
it("works together with hostRewrite", () => {
144-
state.options.hostRewrite = "ext-manual.com";
145-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
146-
expect(state.proxyRes.headers.location).toEqual(
147-
"https://ext-manual.com/",
148-
);
149-
});
153+
it("works together with hostRewrite", () => {
154+
state.options.hostRewrite = "ext-manual.com";
155+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
156+
expect(state.proxyRes.headers.location).toEqual(
157+
"https://ext-manual.com/",
158+
);
159+
});
150160

151-
it("works together with autoRewrite", () => {
152-
state.options.autoRewrite = true;
153-
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
154-
expect(state.proxyRes.headers.location).toEqual("https://ext-auto.com/");
161+
it("works together with autoRewrite", () => {
162+
state.options.autoRewrite = true;
163+
setRedirectHostRewrite(state.req, {}, state.proxyRes, state.options);
164+
expect(state.proxyRes.headers.location).toEqual(
165+
"https://ext-auto.com/",
166+
);
167+
});
155168
});
156169
});
157-
});
170+
}
158171

159172
describe("#setConnection", () => {
160173
it("set the right connection with 1.0 - `close`", () => {

0 commit comments

Comments
 (0)