Skip to content

Commit

Permalink
fix: allow set rejectUnauthorized = true on urllib.request options (#561
Browse files Browse the repository at this point in the history
)

closes #531

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

- **New Features**
- Enhanced HTTP client configuration with options for handling HTTP/2
and unauthorized requests.
- Introduced new tests for validating behavior with `rejectUnauthorized`
set to false.

- **Bug Fixes**
  - Improved error handling and address validation in HTTP client tests.

- **Documentation**
- Updated comments for clarity and consistency in the `HttpClient`
class.

- **Chores**
- Updated dependency management for improved security and functionality.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
fengmk2 authored Dec 7, 2024
1 parent 9dde585 commit 88785e1
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 19 deletions.
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!');
});
});

0 comments on commit 88785e1

Please sign in to comment.