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: allow set rejectUnauthorized = true on urllib.request options #561

Merged
merged 3 commits into from
Dec 7, 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
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@
"cross-env": "^7.0.3",
"eslint": "8",
"eslint-config-egg": "14",
"https-pem": "^3.0.0",
"iconv-lite": "^0.6.3",
"proxy": "^1.0.2",
"selfsigned": "^2.0.1",
"selfsigned": "^2.4.1",
"tar-stream": "^2.2.0",
"tshy": "^3.0.0",
"tshy-after": "^1.0.0",
Expand Down
5 changes: 3 additions & 2 deletions src/HttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ export type ClientOptions = {
*/
cert?: string | Buffer;
/**
* If true, the server certificate is verified against the list of supplied CAs.
* An 'error' event is emitted if verification fails.Default: true.
* If `true`, the server certificate is verified against the list of supplied CAs.
* An 'error' event is emitted if verification fails.
* Default: `true`
*/
rejectUnauthorized?: boolean;
/**
Expand Down
54 changes: 50 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,62 @@ import { HttpClient, HEADER_USER_AGENT } from './HttpClient.js';
import { RequestOptions, RequestURL } from './Request.js';

let httpClient: HttpClient;
let allowH2HttpClient: HttpClient;
let allowUnauthorizedHttpClient: HttpClient;
let allowH2AndUnauthorizedHttpClient: HttpClient;
const domainSocketHttpClients = new LRU(50);

export function getDefaultHttpClient(): HttpClient {
export function getDefaultHttpClient(rejectUnauthorized?: boolean, allowH2?: boolean): HttpClient {
if (rejectUnauthorized === false) {
if (allowH2) {
if (!allowH2AndUnauthorizedHttpClient) {
allowH2AndUnauthorizedHttpClient = new HttpClient({
allowH2,
connect: {
rejectUnauthorized,
},
});
}
return allowH2AndUnauthorizedHttpClient;
}

if (!allowUnauthorizedHttpClient) {
allowUnauthorizedHttpClient = new HttpClient({
connect: {
rejectUnauthorized,
},
});
}
return allowUnauthorizedHttpClient;
}

if (allowH2) {
if (!allowH2HttpClient) {
allowH2HttpClient = new HttpClient({
allowH2,
});
}
return allowH2HttpClient;
}

if (!httpClient) {
httpClient = new HttpClient();
}
return httpClient;
}

export async function request<T = any>(url: RequestURL, options?: RequestOptions) {
interface UrllibRequestOptions extends RequestOptions {
/**
* If `true`, the server certificate is verified against the list of supplied CAs.
* An 'error' event is emitted if verification fails.
* Default: `true`
*/
rejectUnauthorized?: boolean;
/** Allow to use HTTP2 first. Default is `false` */
allowH2?: boolean;
}

export async function request<T = any>(url: RequestURL, options?: UrllibRequestOptions) {
if (options?.socketPath) {
let domainSocketHttpclient = domainSocketHttpClients.get<HttpClient>(options.socketPath);
if (!domainSocketHttpclient) {
Expand All @@ -28,15 +74,15 @@ export async function request<T = any>(url: RequestURL, options?: RequestOptions
return await domainSocketHttpclient.request<T>(url, options);
}

return await getDefaultHttpClient().request<T>(url, options);
return await getDefaultHttpClient(options?.rejectUnauthorized, options?.allowH2).request<T>(url, options);
}

// export curl method is keep compatible with urllib.curl()
// ```ts
// import * as urllib from 'urllib';
// urllib.curl(url);
// ```
export async function curl<T = any>(url: RequestURL, options?: RequestOptions) {
export async function curl<T = any>(url: RequestURL, options?: UrllibRequestOptions) {
return await request<T>(url, options);
}

Expand Down
20 changes: 14 additions & 6 deletions test/HttpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { sensitiveHeaders, createSecureServer } from 'node:http2';
import { PerformanceObserver } from 'node:perf_hooks';
import { setTimeout as sleep } from 'node:timers/promises';
import { describe, it, beforeAll, afterAll } from 'vitest';
import pem from 'https-pem';
import selfsigned from 'selfsigned';
import { HttpClient, RawResponseWithMeta, getGlobalDispatcher } from '../src/index.js';
import { startServer } from './fixtures/server.js';

Expand Down Expand Up @@ -118,7 +118,11 @@ describe('HttpClient.test.ts', () => {
});

it('should not exit after other side closed error', async () => {
const server = createSecureServer(pem);
const pem = selfsigned.generate();
const server = createSecureServer({
key: pem.private,
cert: pem.cert,
});

let count = 0;
server.on('stream', (stream, headers) => {
Expand Down Expand Up @@ -172,7 +176,11 @@ describe('HttpClient.test.ts', () => {
});

it('should auto redirect work', async () => {
const server = createSecureServer(pem);
const pem = selfsigned.generate();
const server = createSecureServer({
key: pem.private,
cert: pem.cert,
});

let count = 0;
server.on('stream', (stream, headers) => {
Expand Down Expand Up @@ -452,13 +460,13 @@ describe('HttpClient.test.ts', () => {
});

it('should allow hostname check', async () => {
let hostname: string;
let hostname = '';
const httpclient = new HttpClient({
checkAddress(ip, family, aHostname) {
checkAddress(_ip, _family, aHostname) {
hostname = aHostname;
return true;
},
lookup(hostname, options, callback) {
lookup(_hostname, _options, callback) {
if (process.version.startsWith('v18')) {
return callback(null, '127.0.0.1', 4);
}
Expand Down
6 changes: 3 additions & 3 deletions test/fixtures/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,10 +370,10 @@ export async function startServer(options?: {
};

if (options?.https) {
const pems = selfsigned.generate();
const pem = selfsigned.generate();
server = createHttpsServer({
key: pems.private,
cert: pems.cert,
key: pem.private,
cert: pem.cert,
}, requestHandler);
} else {
server = createServer(requestHandler);
Expand Down
8 changes: 6 additions & 2 deletions test/options.timeout.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { strict as assert } from 'node:assert';
import { createSecureServer } from 'node:http2';
import { once } from 'node:events';
import pem from 'https-pem';
import selfsigned from 'selfsigned';
import { describe, it, beforeAll, afterAll } from 'vitest';
import urllib, { HttpClientRequestTimeoutError, HttpClient } from '../src/index.js';
import { startServer } from './fixtures/server.js';
Expand Down Expand Up @@ -46,7 +46,11 @@ describe('options.timeout.test.ts', () => {
rejectUnauthorized: false,
},
});
const server = createSecureServer(pem);
const pem = selfsigned.generate();
const server = createSecureServer({
key: pem.private,
cert: pem.cert,
});

server.on('stream', () => {
// wait for timeout
Expand Down
19 changes: 19 additions & 0 deletions test/urllib.options.allowH2.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { strict as assert } from 'node:assert';
import { describe, it } from 'vitest';
import urllib from '../src/index.js';

describe('urllib.options.allowH2.test.ts', () => {
it('should 200 on options.allowH2 = true', async () => {
let response = await urllib.request('https://registry.npmmirror.com', {
allowH2: true,
dataType: 'json',
});
assert.equal(response.status, 200);

response = await urllib.curl('https://registry.npmmirror.com', {
allowH2: true,
dataType: 'json',
});
assert.equal(response.status, 200);
});
});
71 changes: 71 additions & 0 deletions test/urllib.options.rejectUnauthorized-false.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { strict as assert } from 'node:assert';
import { once } from 'node:events';
import { createSecureServer } from 'node:http2';
import { describe, it, beforeAll, afterAll } from 'vitest';
import selfsigned from 'selfsigned';
import urllib, { HttpClient } from '../src/index.js';
import { startServer } from './fixtures/server.js';

describe('urllib.options.rejectUnauthorized-false.test.ts', () => {
let close: any;
let _url: string;
beforeAll(async () => {
const { closeServer, url } = await startServer({ https: true });
close = closeServer;
_url = url;
});

afterAll(async () => {
await close();
});

it('should 200 on options.rejectUnauthorized = false', async () => {
const response = await urllib.request(_url, {
rejectUnauthorized: false,
dataType: 'json',
});
assert.equal(response.status, 200);
assert.equal(response.data.method, 'GET');
});

it('should 200 with H2 on options.rejectUnauthorized = false', async () => {
const pem = selfsigned.generate();
const server = createSecureServer({
key: pem.private,
cert: pem.cert,
});

server.on('stream', (stream, headers) => {
assert.equal(headers[':method'], 'GET');
stream.respond({
'content-type': 'text/plain; charset=utf-8',
'x-custom-h2': 'hello',
':status': 200,
});
stream.end('hello h2!');
});

server.listen(0);
await once(server, 'listening');

const httpClient = new HttpClient({
allowH2: true,
connect: {
rejectUnauthorized: false,
},
});

const url = `https://localhost:${server.address()!.port}`;
const response1 = await httpClient.request(url, {});
assert.equal(response1.status, 200);
assert.equal(response1.data.toString(), 'hello h2!');

const response2 = await urllib.request(url, {
rejectUnauthorized: false,
allowH2: true,
dataType: 'text',
});
assert.equal(response2.status, 200);
assert.equal(response2.data, 'hello h2!');
});
});
Loading