diff --git a/CHANGELOG.md b/CHANGELOG.md index 25096e7..e8b7606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Change Log Notable changes will be documented here. +## [0.45.0] +- Load Node.js system certificates in a worker on macOS to avoid blocking the main thread ([microsoft/vscode#333830](https://github.com/microsoft/vscode/issues/333830)) + ## [0.44.0] - Add `createProxyAuthorizationLookup` for reusable Kerberos and Basic proxy authentication handling. diff --git a/package-lock.json b/package-lock.json index 0cba471..3a93c5c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vscode/proxy-agent", - "version": "0.44.0", + "version": "0.45.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@vscode/proxy-agent", - "version": "0.44.0", + "version": "0.45.0", "license": "MIT", "dependencies": { "@tootallnate/once": "^3.0.0", diff --git a/package.json b/package.json index 4846a29..bf6f14e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@vscode/proxy-agent", - "version": "0.44.0", + "version": "0.45.0", "description": "NodeJS http(s) agent implementation for VS Code", "main": "out/index.js", "types": "out/index.d.ts", diff --git a/src/index.ts b/src/index.ts index d018f54..99bab4e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,11 +9,13 @@ import type * as https from 'https'; import * as tls from 'tls'; import * as nodeurl from 'url'; import * as os from 'os'; +import * as path from 'path'; import * as fs from 'fs'; import * as cp from 'child_process'; import * as crypto from 'crypto'; import * as undici from 'undici'; import * as stream from 'stream'; +import { Worker } from 'worker_threads'; import { createPacProxyAgent, getProxyURLFromResolverResult, PacProxyAgent, ProxyResolveType } from './agent'; import type { IncomingHttpHeaders } from 'undici/types/header'; @@ -209,6 +211,10 @@ export interface ResolvedProxyInfo { export function createProxyResolver(params: ProxyAgentParams) { const { getProxyURL, log, proxyResolveTelemetry: proxyResolverTelemetry, env } = params; + if (process.platform === 'darwin' && params.loadSystemCertificatesFromNode() && (params.addCertificatesV1() || params.addCertificatesV2())) { + void getOrLoadAdditionalCertificates(params) + .catch(err => log.error('ProxyResolver#loadSystemCertificates preload error', toErrorMessage(err))); + } let envProxy = proxyFromConfigURL(env.https_proxy || env.HTTPS_PROXY || env.http_proxy || env.HTTP_PROXY); // Not standardized. let envNoProxy = noProxyFromEnv(env.no_proxy || env.NO_PROXY); // Not standardized. @@ -1296,6 +1302,11 @@ export async function getOrLoadAdditionalCertificates(params: ProxyAgentParams) result: undefined }; _certs.set(loadFromNode, cert); + void cert.promise.catch(() => { + if (_certs.get(loadFromNode) === cert) { + _certs.delete(loadFromNode); + } + }); } return _certs.get(loadFromNode)!.promise; } @@ -1322,33 +1333,86 @@ function filterExpiredCertificates(params: CertificateParams, certs: string[]) { return filtered; } -let _systemCertificatesPromise: Promise | undefined; -export async function loadSystemCertificates(params: CertificateParams) { - if (!!params.loadSystemCertificatesFromNode?.()) { // Checking if function exists for backward compatibility. - const start = Date.now(); - const systemCerts = tls.getCACertificates('system'); - params.log.debug(`ProxyResolver#loadSystemCertificates from Node.js count (${Date.now() - start}ms)`, systemCerts.length); - return filterExpiredCertificates(params, systemCerts); - } - if (!_systemCertificatesPromise) { - _systemCertificatesPromise = (async () => { - try { +const _systemCertificatesPromises = new Map>(); +export function loadSystemCertificates(params: CertificateParams) { + const loadFromNode = !!params.loadSystemCertificatesFromNode?.(); // Checking if function exists for backward compatibility. + let systemCertificatesPromise = _systemCertificatesPromises.get(loadFromNode); + if (!systemCertificatesPromise) { + if (loadFromNode) { + systemCertificatesPromise = (async () => { const start = Date.now(); - const certs = await readSystemCertificates(); - params.log.debug(`ProxyResolver#loadSystemCertificates count (${Date.now() - start}ms)`, certs.length); + const certs = await readNodeSystemCertificates(params.log); + params.log.debug(`ProxyResolver#loadSystemCertificates from Node.js count (${Date.now() - start}ms)`, certs.length); return filterExpiredCertificates(params, certs); - } catch (err) { - params.log.error('ProxyResolver#loadSystemCertificates error', toErrorMessage(err)); - return []; + })(); + } else { + systemCertificatesPromise = (async () => { + try { + const start = Date.now(); + const certs = await readSystemCertificates(); + params.log.debug(`ProxyResolver#loadSystemCertificates count (${Date.now() - start}ms)`, certs.length); + return filterExpiredCertificates(params, certs); + } catch (err) { + params.log.error('ProxyResolver#loadSystemCertificates error', toErrorMessage(err)); + return []; + } + })(); + } + _systemCertificatesPromises.set(loadFromNode, systemCertificatesPromise); + void systemCertificatesPromise.catch(() => { + if (_systemCertificatesPromises.get(loadFromNode) === systemCertificatesPromise) { + _systemCertificatesPromises.delete(loadFromNode); } - })(); + }); } - return _systemCertificatesPromise; + return systemCertificatesPromise; } export function resetCaches() { _certs.clear(); - _systemCertificatesPromise = undefined; + _systemCertificatesPromises.clear(); +} + +async function readNodeSystemCertificates(log: Log): Promise { + if (process.platform !== 'darwin') { + return tls.getCACertificates('system'); + } + + try { + return await new Promise((resolve, reject) => { + log.debug('ProxyResolver#loadSystemCertificates starting worker'); + const workerStart = Date.now(); + const worker = new Worker(path.join(__dirname, 'systemCertificatesWorker.js'), { name: 'System certificate loader' }); + worker.unref(); + let settled = false; + worker.once('message', (certs: unknown) => { + settled = true; + if (!isStringArray(certs)) { + void worker.terminate(); + reject(new Error('System certificate worker returned an invalid result')); + return; + } + resolve(certs); + }); + worker.once('error', err => { + settled = true; + reject(err); + }); + worker.once('exit', code => { + log.debug(`ProxyResolver#loadSystemCertificates worker exited (${Date.now() - workerStart}ms)`, code); + if (!settled) { + reject(new Error(`System certificate worker exited with code ${code}`)); + } + }); + }); + } catch (err) { + log.warn('ProxyResolver#loadSystemCertificates worker failed, falling back to main thread', toErrorMessage(err)); + return tls.getCACertificates('system'); + } +} + +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every(item => typeof item === 'string'); } async function readSystemCertificates(): Promise { @@ -1463,5 +1527,3 @@ export function toLogString(args: any[]) { return value; })).join(', ')}]`; } - - diff --git a/src/systemCertificatesWorker.ts b/src/systemCertificatesWorker.ts new file mode 100644 index 0000000..5853e1c --- /dev/null +++ b/src/systemCertificatesWorker.ts @@ -0,0 +1,13 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as tls from 'tls'; +import { parentPort } from 'worker_threads'; + +if (!parentPort) { + throw new Error('System certificate worker must run in a worker thread'); +} + +parentPort.postMessage(tls.getCACertificates('system')); diff --git a/tests/src/certificateLoading.test.ts b/tests/src/certificateLoading.test.ts new file mode 100644 index 0000000..fa17365 --- /dev/null +++ b/tests/src/certificateLoading.test.ts @@ -0,0 +1,39 @@ +import * as assert from 'assert'; +import { getOrLoadAdditionalCertificates, LogLevel, ProxyAgentParams, resetCaches } from '../../src'; + +function createParams(loadAdditionalCertificates: () => Promise): ProxyAgentParams { + const noop = () => { }; + return { + resolveProxy: async () => undefined, + getProxyURL: () => undefined, + getProxySupport: () => 'override', + isAdditionalFetchSupportEnabled: () => true, + isWebSocketPatchEnabled: () => true, + addCertificatesV1: () => true, + addCertificatesV2: () => false, + loadSystemCertificatesFromNode: () => true, + loadAdditionalCertificates, + log: { trace: noop, debug: noop, info: noop, warn: noop, error: noop }, + getLogLevel: () => LogLevel.Off, + proxyResolveTelemetry: noop, + isUseHostProxyEnabled: () => true, + env: {}, + }; +} + +describe('certificate loading', function () { + afterEach(() => resetCaches()); + + it('retries loading additional certificates after a failure', async function () { + let attempts = 0; + const params = createParams(async () => { + if (++attempts === 1) { + throw new Error('Certificate loading failed'); + } + return ['certificate']; + }); + + await assert.rejects(getOrLoadAdditionalCertificates(params), /Certificate loading failed/); + assert.deepStrictEqual(await getOrLoadAdditionalCertificates(params), ['certificate']); + }); +}); diff --git a/tests/src/resolveProxyByURL.test.ts b/tests/src/resolveProxyByURL.test.ts index 48c07b8..a908f56 100644 --- a/tests/src/resolveProxyByURL.test.ts +++ b/tests/src/resolveProxyByURL.test.ts @@ -1,5 +1,5 @@ import * as assert from 'assert'; -import { createProxyResolver, LogLevel, ProxyAgentParams } from '../../src'; +import { createProxyResolver, getOrLoadAdditionalCertificates, loadSystemCertificates, Log, LogLevel, ProxyAgentParams, resetCaches } from '../../src'; function createParams(overrides: Partial): ProxyAgentParams { const noop = () => { }; @@ -24,6 +24,45 @@ function createParams(overrides: Partial): ProxyAgentParams { } describe('resolveProxyByURL', function () { + it('preloads Node.js system certificates on macOS', async function () { + if (process.platform !== 'darwin') { + this.skip(); + } + + resetCaches(); + const debugMessages: string[] = []; + const log: Log = { + trace: () => { }, + debug: message => debugMessages.push(message), + info: () => { }, + warn: () => { }, + error: () => { }, + }; + const params = createParams({ + addCertificatesV1: () => true, + loadSystemCertificatesFromNode: () => true, + loadAdditionalCertificates: () => loadSystemCertificates({ + loadSystemCertificatesFromNode: () => true, + log, + }), + log, + }); + try { + createProxyResolver(params); + const startedDuringResolverCreation = debugMessages.includes('ProxyResolver#loadSystemCertificates starting worker'); + await getOrLoadAdditionalCertificates(params); + assert.deepStrictEqual({ + startedDuringResolverCreation, + finishedLoadingSystemCertificates: debugMessages.some(message => message.startsWith('ProxyResolver#loadSystemCertificates from Node.js count')), + }, { + startedDuringResolverCreation: true, + finishedLoadingSystemCertificates: true, + }); + } finally { + resetCaches(); + } + }); + it('reports localhost as a direct connection', async function () { const { resolveProxyByURL } = createProxyResolver(createParams({})); assert.deepStrictEqual(await resolveProxyByURL('http://localhost:3000/'), {