Skip to content
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.

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

fix: replace deprecated url.parse() with new URL() #1803

Merged
merged 1 commit into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 2 additions & 4 deletions src/node/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
* Module dependencies.
*/

// eslint-disable-next-line node/no-deprecated-api
const { parse } = require('url');
const { CookieJar } = require('cookiejar');
const { CookieAccessInfo } = require('cookiejar');
const methods = require('methods');
Expand Down Expand Up @@ -55,7 +53,7 @@ class Agent extends AgentBase {
_saveCookies (res) {
const cookies = res.headers['set-cookie'];
if (cookies) {
const url = parse(res.request?.url || '');
const url = new URL(res.request?.url || '');
this.jar.setCookies(cookies, url.hostname, url.pathname);
}
}
Expand All @@ -67,7 +65,7 @@ class Agent extends AgentBase {
* @api private
*/
_attachCookies (request_) {
const url = parse(request_.url);
const url = new URL(request_.url);
const access = new CookieAccessInfo(
url.hostname,
url.pathname,
Expand Down
4 changes: 1 addition & 3 deletions src/node/http2wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
const Stream = require('stream');
const net = require('net');
const tls = require('tls');
// eslint-disable-next-line node/no-deprecated-api
const { parse } = require('url');

const {
HTTP2_HEADER_PATH,
Expand Down Expand Up @@ -75,7 +73,7 @@
}
}

setNoDelay(bool) {

Check warning on line 76 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (14.x)

'bool' is defined but never used

Check warning on line 76 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (16.x)

'bool' is defined but never used

Check warning on line 76 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (18.x)

'bool' is defined but never used
// We can not use setNoDelay with HTTP/2.
// Node 10 limits http2session.socket methods to ones safe to use with HTTP/2.
// See also https://nodejs.org/api/http2.html#http2_http2session_socket
Expand All @@ -97,7 +95,7 @@

const frame = this.session.request(headers);

frame.once('response', (headers, flags) => {

Check warning on line 98 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (14.x)

'flags' is defined but never used

Check warning on line 98 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (16.x)

'flags' is defined but never used

Check warning on line 98 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (18.x)

'flags' is defined but never used
headers = this.mapToHttpHeader(headers);
frame.headers = headers;
frame.statusCode = headers[HTTP2_HEADER_STATUS];
Expand Down Expand Up @@ -145,7 +143,7 @@
case HTTP2_HEADER_HOST:
key = HTTP2_HEADER_AUTHORITY;
value = /^http:\/\/|^https:\/\//.test(value)
? parse(value).host
? new URL(value).host
: value;
break;
default:
Expand Down Expand Up @@ -181,7 +179,7 @@
frame.end(data);
}

abort(data) {

Check warning on line 182 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (14.x)

'data' is defined but never used

Check warning on line 182 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (16.x)

'data' is defined but never used

Check warning on line 182 in src/node/http2wrapper.js

View workflow job for this annotation

GitHub Actions / test (18.x)

'data' is defined but never used
const frame = this.getFrame();
frame.close(NGHTTP2_CANCEL);
this.session.destroy();
Expand Down
9 changes: 5 additions & 4 deletions test/node/basic-auth.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const assert = require('assert');
const URL = require('url');
const { format } = require('url');
const request = require('../support/client');
const getSetup = require('../support/setup');

Expand All @@ -14,11 +14,12 @@ describe('Basic auth', () => {

describe('when credentials are present in url', () => {
it('should set Authorization', (done) => {
const new_url = URL.parse(base);
new_url.auth = 'tobi:learnboost';
const new_url = new URL(base);
new_url.username = 'tobi';
new_url.password = 'learnboost';
new_url.pathname = '/basic-auth';

request.get(URL.format(new_url)).end((error, res) => {
request.get(format(new_url)).end((error, res) => {
assert.equal(res.status, 200);
done();
});
Expand Down
3 changes: 1 addition & 2 deletions test/node/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ const assert = require('assert');
const fs = require('fs');
const { EventEmitter } = require('events');
const { StringDecoder } = require('string_decoder');
const url = require('url');
const getSetup = require('../support/setup');
const request = require('../support/client');

Expand All @@ -30,7 +29,7 @@ describe('[node] request', () => {

describe('with an object', () => {
it('should format the url', () =>
request.get(url.parse(`${base}/login`)).then((res) => {
request.get(new URL(`${base}/login`)).then((res) => {
assert.ok(res.ok);
}));
});
Expand Down
3 changes: 1 addition & 2 deletions test/node/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ if (!process.env.HTTP2_TEST) {
}

const assert = require('assert');
const url = require('url');
const request = require('../..');
const getSetup = require('../support/setup');

Expand All @@ -29,7 +28,7 @@ describe('request.get().http2()', () => {

it('should format the url', () =>
request
.get(url.parse(`${base}/login`))
.get(new URL(`${base}/login`))
.http2()
.then((res) => {
assert(res.ok);
Expand Down
7 changes: 3 additions & 4 deletions test/node/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const assert = require('assert');

const url = require('url');
const https = require('https');
const fs = require('fs');
const express = require('../support/express');
Expand Down Expand Up @@ -128,7 +127,7 @@ describe('https', () => {
assert.ifError(error);
assert(res.ok);
assert.strictEqual('Safe and secure!', res.text);
agent.get(url.parse(testEndpoint)).end((error, res) => {
agent.get(new URL(testEndpoint)).end((error, res) => {
assert.ifError(error);
assert(res.ok);
assert.strictEqual('Safe and secure!', res.text);
Expand Down Expand Up @@ -221,7 +220,7 @@ describe('https', () => {
assert.ifError(error);
assert(res.ok);
assert.strictEqual('Safe and secure!', res.text);
agent.get(url.parse(testEndpoint)).end((error, res) => {
agent.get(new URL(testEndpoint)).end((error, res) => {
assert.ifError(error);
assert(res.ok);
assert.strictEqual('Safe and secure!', res.text);
Expand All @@ -235,7 +234,7 @@ describe('https', () => {
assert.ifError(error);
assert(res.ok);
assert.strictEqual('Safe and secure!', res.text);
agent.get(url.parse(testEndpoint)).end((error, res) => {
agent.get(new URL(testEndpoint)).end((error, res) => {
assert.ifError(error);
assert(res.ok);
assert.strictEqual('Safe and secure!', res.text);
Expand Down
5 changes: 2 additions & 3 deletions test/node/redirects.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';

const URL = require('url');
const assert = require('assert');
const getSetup = require('../support/setup');
const request = require('../support/client');
Expand Down Expand Up @@ -112,7 +111,7 @@ describe('request', () => {

it('should follow Location with IP override', () => {
const redirects = [];
const url = URL.parse(base);
const url = new URL(base);
return request
.get(`http://redir.example.com:${url.port || '80'}${url.pathname}`)
.connect({
Expand All @@ -130,7 +129,7 @@ describe('request', () => {

it('should follow Location with IP:port override', () => {
const redirects = [];
const url = URL.parse(base);
const url = new URL(base);
return request
.get(`http://redir.example.com:9999${url.pathname}`)
.connect({
Expand Down
2 changes: 1 addition & 1 deletion test/use.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('subclass', () => {
NewRequest.prototype = Object.create(OriginalRequest.prototype);
request.Request = NewRequest;

const request_ = request.agent().del('/');
const request_ = request.agent().del('http://test.com/');
assert(request_ instanceof NewRequest);
assert(request_ instanceof OriginalRequest);
});
Expand Down
Loading