From 6df5063b14868ff21499a051e5122fa7211be6ed Mon Sep 17 00:00:00 2001 From: guibwl <228474838@qq.com> Date: Fri, 29 Dec 2023 19:15:57 +0800 Subject: [PATCH] feat: add support for HTTP redirect (#341) Corepack will now follow the `Location` header when receiving a 30x HTTP status code. This should allow Corepack to work with proxies that redirect to a different URL. --- sources/httpUtils.ts | 32 ++++++++------ tests/httpUtils.test.ts | 95 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 12 deletions(-) create mode 100644 tests/httpUtils.test.ts diff --git a/sources/httpUtils.ts b/sources/httpUtils.ts index 67cf98910..94598dfbe 100644 --- a/sources/httpUtils.ts +++ b/sources/httpUtils.ts @@ -1,6 +1,6 @@ -import {UsageError} from 'clipanion'; -import {RequestOptions} from 'https'; -import {IncomingMessage} from 'http'; +import {UsageError} from 'clipanion'; +import {RequestOptions} from 'https'; +import {IncomingMessage, ClientRequest} from 'http'; export async function fetchUrlStream(url: string, options: RequestOptions = {}) { if (process.env.COREPACK_ENABLE_NETWORK === `0`) @@ -13,17 +13,25 @@ export async function fetchUrlStream(url: string, options: RequestOptions = {}) const proxyAgent = new ProxyAgent(); return new Promise((resolve, reject) => { - const request = https.get(url, {...options, agent: proxyAgent}, response => { - const statusCode = response.statusCode; - if (statusCode != null && statusCode >= 200 && statusCode < 300) - return resolve(response); + const createRequest = (url: string) => { + const request: ClientRequest = https.get(url, {...options, agent: proxyAgent}, response => { + const statusCode = response.statusCode; - return reject(new Error(`Server answered with HTTP ${statusCode} when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); - }); + if ([301, 302, 307, 308].includes(statusCode as number) && response.headers.location) + return createRequest(response.headers.location as string); - request.on(`error`, err => { - reject(new Error(`Error when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); - }); + if (statusCode != null && statusCode >= 200 && statusCode < 300) + return resolve(response); + + return reject(new Error(`Server answered with HTTP ${statusCode} when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); + }); + + request.on(`error`, err => { + reject(new Error(`Error when performing the request to ${url}; for troubleshooting help, see https://github.com/nodejs/corepack#troubleshooting`)); + }); + }; + + createRequest(url); }); } diff --git a/tests/httpUtils.test.ts b/tests/httpUtils.test.ts new file mode 100644 index 000000000..4676be3cf --- /dev/null +++ b/tests/httpUtils.test.ts @@ -0,0 +1,95 @@ +import {jest, describe, beforeEach, beforeAll, it, expect} from '@jest/globals'; + +import {fetchUrlStream} from '../sources/httpUtils'; + + +describe(`http utils fetchUrlStream`, () => { + const getUrl = (statusCode: number | string, redirectCode?: number | string) => + `https://registry.example.org/answered/${statusCode}${redirectCode ? `?redirectCode=${redirectCode}` : ``}`; + + const httpsGetFn = jest.fn((url: string, _, callback: (response: any) => void) => { + const parsedURL = new URL(url); + const statusCode = parsedURL.pathname.slice(parsedURL.pathname.lastIndexOf(`/`) + 1); + const response = {url, statusCode: +statusCode}; + const errorCallbacks: Array<(err: string) => void> = []; + + if ([301, 302, 307, 308].includes(+statusCode)) { + const redirectCode = parsedURL.searchParams.get(`redirectCode`)!; + // mock response.headers.location + if (redirectCode) { + Reflect.set(response, `headers`, {location: getUrl(redirectCode)}); + } + } + + // handle request.on('error', err => ...) + if (statusCode === `error`) + process.nextTick(() => errorCallbacks.forEach(cb => cb(`Test internal error`))); + else + callback(response); + + return { + on: (type: string, callback: (err: string) => void) => { + if (type === `error`) { + errorCallbacks.push(callback); + } + }, + }; + }); + + beforeAll(() => { + jest.doMock(`https`, () => ({ + get: httpsGetFn, + Agent: class Agent {}, + })); + }); + + beforeEach(() => { + httpsGetFn.mockClear(); + }); + + it(`correct response answered statusCode should be >= 200 and < 300`, async () => { + await expect(fetchUrlStream(getUrl(200))).resolves.toMatchObject({ + statusCode: 200, + }); + + await expect(fetchUrlStream(getUrl(299))).resolves.toMatchObject({ + statusCode: 299, + }); + + expect(httpsGetFn).toHaveBeenCalledTimes(2); + }); + + it(`bad response`, async () => { + await expect(fetchUrlStream(getUrl(300))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(199))).rejects.toThrowError(); + }); + + it(`redirection with correct response`, async () => { + await expect(fetchUrlStream(getUrl(301, 200))).resolves.toMatchObject({ + statusCode: 200, + }); + + expect(httpsGetFn).toHaveBeenCalledTimes(2); + + await expect(fetchUrlStream(getUrl(308, 299))).resolves.toMatchObject({ + statusCode: 299, + }); + + expect(httpsGetFn).toHaveBeenCalledTimes(4); + }); + + it(`redirection with bad response`, async () => { + await expect(fetchUrlStream(getUrl(301, 300))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(308, 199))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(301, 302))).rejects.toThrowError(); + await expect(fetchUrlStream(getUrl(307))).rejects.toThrowError(); + }); + + it(`rejects with error`, async () => { + await expect(fetchUrlStream(getUrl(`error`))).rejects.toThrowError(); + }); + + it(`rejects when redirection with error`, async () => { + await expect(fetchUrlStream(getUrl(307, `error`))).rejects.toThrowError(); + }); +});