Skip to content

fix: get rid of url.parse in network code #36423

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.

Sign up for GitHub

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

Merged
mxschmitt merged 1 commit into from
Jun 25, 2025
Merged

fix: get rid of url.parse in network code #36423

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 37 additions & 21 deletions packages/playwright-core/src/server/utils/network.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,8 @@ export type HTTPRequestParams = {
export const NET_DEFAULT_TIMEOUT = 30_000;

export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.IncomingMessage) => void, onError: (error: Error) => void): { cancel(error: Error | undefined): void } {
const parsedUrl = url.parse(params.url);
let options: https.RequestOptions = {
...parsedUrl,
const parsedUrl = new URL(params.url);
const options: https.RequestOptions = {
agent: parsedUrl.protocol === 'https:' ? httpsHappyEyeballsAgent : httpHappyEyeballsAgent,
method: params.method || 'GET',
headers: params.headers,
Expand All @@ -51,19 +50,15 @@ export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.Inco

const proxyURL = getProxyForUrl(params.url);
if (proxyURL) {
const parsedProxyURL = url.parse(proxyURL);
const parsedProxyURL = new URL(proxyURL);
if (params.url.startsWith('http:')) {
options = {
path: parsedUrl.href,
host: parsedProxyURL.hostname,
port: parsedProxyURL.port,
headers: options.headers,
method: options.method
};
parsedUrl.pathname = parsedUrl.href;
parsedUrl.host = parsedProxyURL.host;
} else {
(parsedProxyURL as any).secureProxy = parsedProxyURL.protocol === 'https:';

options.agent = new HttpsProxyAgent(parsedProxyURL);
options.agent = new HttpsProxyAgent({
...convertURLtoLegacyUrl(parsedProxyURL),
secureProxy: parsedProxyURL.protocol === 'https:',
});
options.rejectUnauthorized = false;
}
}
Expand All @@ -81,8 +76,8 @@ export function httpRequest(params: HTTPRequestParams, onResponse: (r: http.Inco
}
};
const request = options.protocol === 'https:' ?
https.request(options, requestCallback) :
http.request(options, requestCallback);
https.request(parsedUrl, options, requestCallback) :
http.request(parsedUrl, options, requestCallback);
request.on('error', onError);
if (params.socketTimeout !== undefined) {
request.setTimeout(params.socketTimeout, () => {
Expand Down Expand Up @@ -137,23 +132,27 @@ export function createProxyAgent(proxy?: ProxySettings, forUrl?: URL) {
if (!/^\w+:\/\//.test(proxyServer))
proxyServer = 'http://' + proxyServer;

const proxyOpts = url.parse(proxyServer);
const proxyOpts = new URL(proxyServer);
if (proxyOpts.protocol?.startsWith('socks')) {
return new SocksProxyAgent({
host: proxyOpts.hostname,
port: proxyOpts.port || undefined,
});
}
if (proxy.username)
proxyOpts.auth = `${proxy.username}:${proxy.password || ''}`;
if (proxy.username) {
proxyOpts.username = proxy.username;
proxyOpts.password = proxy.password || '';
}

if (forUrl && ['ws:', 'wss:'].includes(forUrl.protocol)) {
// Force CONNECT method for WebSockets.
return new HttpsProxyAgent(proxyOpts);
// TODO: switch to URL instance instead of legacy object once https-proxy-agent supports it.
return new HttpsProxyAgent(convertURLtoLegacyUrl(proxyOpts));
}

// TODO: We should use HttpProxyAgent conditional on proxyOpts.protocol instead of always using CONNECT method.
return new HttpsProxyAgent(proxyOpts);
// TODO: switch to URL instance instead of legacy object once https-proxy-agent supports it.
return new HttpsProxyAgent(convertURLtoLegacyUrl(proxyOpts));
Copy link
Contributor Author

@mxschmitt mxschmitt Jun 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are still on and old https-proxy-agent version which uses url.parse internally if you pass a string - once we update we can remove the convertURLtoLegacyUrl function and pass the URL object over.

}

export function createHttpServer(requestListener?: (req: http.IncomingMessage, res: http.ServerResponse) => void): http.Server;
Expand Down Expand Up @@ -226,3 +225,20 @@ function decorateServer(server: net.Server) {
return close.call(server, callback);
};
}

function convertURLtoLegacyUrl(url: URL): url.Url {
return {
auth: url.username ? url.username + ':' + url.password : null,
hash: url.hash || null,
host: url.hostname ? url.hostname + ':' + url.port : null,
hostname: url.hostname || null,
href: url.href,
path: url.pathname + url.search,
pathname: url.pathname,
protocol: url.protocol,
search: url.search || null,
slashes: true,
port: url.port || null,
query: url.search.slice(1) || null,
};
}
Loading