From 7410d45a919bacadc2995d6173f3f7e7340f6e50 Mon Sep 17 00:00:00 2001 From: Artem Zakharchenko Date: Sun, 8 Sep 2024 13:40:13 +0200 Subject: [PATCH] feat(fetch): support following redirect responses (#627) --- src/interceptors/fetch/index.ts | 36 ++- .../fetch/utils/createNetworkError.ts | 5 + .../fetch/utils/followRedirect.ts | 107 ++++++++ src/utils/responseUtils.ts | 4 + .../compliance/fetch-follow-redirects.test.ts | 244 ++++++++++++++++++ .../fetch-request-body-used.test.ts | 2 +- 6 files changed, 391 insertions(+), 7 deletions(-) create mode 100644 src/interceptors/fetch/utils/createNetworkError.ts create mode 100644 src/interceptors/fetch/utils/followRedirect.ts create mode 100644 test/modules/fetch/compliance/fetch-follow-redirects.test.ts diff --git a/src/interceptors/fetch/index.ts b/src/interceptors/fetch/index.ts index 3aac3124..0f373375 100644 --- a/src/interceptors/fetch/index.ts +++ b/src/interceptors/fetch/index.ts @@ -7,6 +7,9 @@ import { emitAsync } from '../../utils/emitAsync' import { handleRequest } from '../../utils/handleRequest' import { canParseUrl } from '../../utils/canParseUrl' import { createRequestId } from '../../createRequestId' +import { RESPONSE_STATUS_CODES_WITH_REDIRECT } from '../../utils/responseUtils' +import { createNetworkError } from './utils/createNetworkError' +import { followFetchRedirect } from './utils/followRedirect' export class FetchInterceptor extends Interceptor { static symbol = Symbol('fetch') @@ -68,6 +71,33 @@ export class FetchInterceptor extends Interceptor { response, }) + /** + * Undici's handling of following redirect responses. + * Treat the "manual" redirect mode as a regular mocked response. + * This way, the client can manually follow the redirect it receives. + * @see https://github.com/nodejs/undici/blob/a6dac3149c505b58d2e6d068b97f4dc993da55f0/lib/web/fetch/index.js#L1173 + */ + if (RESPONSE_STATUS_CODES_WITH_REDIRECT.has(response.status)) { + // Reject the request promise if its `redirect` is set to `error` + // and it receives a mocked redirect response. + if (request.redirect === 'error') { + responsePromise.reject(createNetworkError('unexpected redirect')) + return + } + + if (request.redirect === 'follow') { + followFetchRedirect(request, response).then( + (response) => { + responsePromise.resolve(response) + }, + (reason) => { + responsePromise.reject(reason) + } + ) + return + } + } + if (this.emitter.listenerCount('response') > 0) { this.logger.info('emitting the "response" event...') @@ -154,9 +184,3 @@ export class FetchInterceptor extends Interceptor { }) } } - -function createNetworkError(cause: unknown) { - return Object.assign(new TypeError('Failed to fetch'), { - cause, - }) -} diff --git a/src/interceptors/fetch/utils/createNetworkError.ts b/src/interceptors/fetch/utils/createNetworkError.ts new file mode 100644 index 00000000..bf58f070 --- /dev/null +++ b/src/interceptors/fetch/utils/createNetworkError.ts @@ -0,0 +1,5 @@ +export function createNetworkError(cause?: unknown) { + return Object.assign(new TypeError('Failed to fetch'), { + cause, + }) +} diff --git a/src/interceptors/fetch/utils/followRedirect.ts b/src/interceptors/fetch/utils/followRedirect.ts new file mode 100644 index 00000000..e577d57b --- /dev/null +++ b/src/interceptors/fetch/utils/followRedirect.ts @@ -0,0 +1,107 @@ +import { createNetworkError } from './createNetworkError' + +const REQUEST_BODY_HEADERS = [ + 'content-encoding', + 'content-language', + 'content-location', + 'content-type', + 'content-length', +] + +const kRedirectCount = Symbol('kRedirectCount') + +/** + * @see https://github.com/nodejs/undici/blob/a6dac3149c505b58d2e6d068b97f4dc993da55f0/lib/web/fetch/index.js#L1210 + */ +export async function followFetchRedirect( + request: Request, + response: Response +): Promise { + if (response.status !== 303 && request.body != null) { + return Promise.reject(createNetworkError()) + } + + const requestUrl = new URL(request.url) + + let locationUrl: URL + try { + locationUrl = new URL(response.headers.get('location')!) + } catch (error) { + return Promise.reject(createNetworkError(error)) + } + + if ( + !(locationUrl.protocol === 'http:' || locationUrl.protocol === 'https:') + ) { + return Promise.reject( + createNetworkError('URL scheme must be a HTTP(S) scheme') + ) + } + + if (Reflect.get(request, kRedirectCount) > 20) { + return Promise.reject(createNetworkError('redirect count exceeded')) + } + + Object.defineProperty(request, kRedirectCount, { + value: (Reflect.get(request, kRedirectCount) || 0) + 1, + }) + + if ( + request.mode === 'cors' && + (locationUrl.username || locationUrl.password) && + !sameOrigin(requestUrl, locationUrl) + ) { + return Promise.reject( + createNetworkError('cross origin not allowed for request mode "cors"') + ) + } + + const requestInit: RequestInit = {} + + if ( + ([301, 302].includes(response.status) && request.method === 'POST') || + (response.status === 303 && !['HEAD', 'GET'].includes(request.method)) + ) { + requestInit.method = 'GET' + requestInit.body = null + + REQUEST_BODY_HEADERS.forEach((headerName) => { + request.headers.delete(headerName) + }) + } + + if (!sameOrigin(requestUrl, locationUrl)) { + request.headers.delete('authorization') + request.headers.delete('proxy-authorization') + request.headers.delete('cookie') + request.headers.delete('host') + } + + /** + * @note Undici "safely" extracts the request body. + * I suspect we cannot dispatch this request again + * since its body has been read and the stream is locked. + */ + + requestInit.headers = request.headers + return fetch(new Request(locationUrl, requestInit)) +} + +/** + * @see https://github.com/nodejs/undici/blob/a6dac3149c505b58d2e6d068b97f4dc993da55f0/lib/web/fetch/util.js#L761 + */ +function sameOrigin(left: URL, right: URL): boolean { + if (left.origin === right.origin && left.origin === 'null') { + return true + } + + if ( + left.protocol === right.protocol && + left.hostname === right.hostname && + left.port === right.port + ) { + return true + } + + return false +} diff --git a/src/utils/responseUtils.ts b/src/utils/responseUtils.ts index 4e40ef44..496a7228 100644 --- a/src/utils/responseUtils.ts +++ b/src/utils/responseUtils.ts @@ -8,6 +8,10 @@ export const RESPONSE_STATUS_CODES_WITHOUT_BODY = new Set([ 101, 103, 204, 205, 304, ]) +export const RESPONSE_STATUS_CODES_WITH_REDIRECT = new Set([ + 301, 302, 303, 307, 308, +]) + /** * Returns a boolean indicating whether the given response status * code represents a response that cannot have a body. diff --git a/test/modules/fetch/compliance/fetch-follow-redirects.test.ts b/test/modules/fetch/compliance/fetch-follow-redirects.test.ts new file mode 100644 index 00000000..cc3bc4ff --- /dev/null +++ b/test/modules/fetch/compliance/fetch-follow-redirects.test.ts @@ -0,0 +1,244 @@ +// @vitest-environment node +import { it, expect, beforeAll, afterEach, afterAll } from 'vitest' +import { HttpServer } from '@open-draft/test-server/http' +import { FetchInterceptor } from '../../../../src/interceptors/fetch' + +const interceptor = new FetchInterceptor() + +const httpServer = new HttpServer((app) => { + app.get('/original', (req, res) => + res.writeHead(302, { Location: httpServer.http.url('/redirected') }).end() + ) + app.get('/redirected', (req, res) => res.send('redirected')) +}) + +beforeAll(async () => { + interceptor.apply() + await httpServer.listen() +}) + +afterEach(() => { + interceptor.removeAllListeners() +}) + +afterAll(async () => { + interceptor.dispose() + await httpServer.close() +}) + +it('follows a bypassed redirect response', async () => { + const response = await fetch(httpServer.http.url('/original')) + + expect(response.status).toBe(200) + await expect(response.text()).resolves.toBe('redirected') +}) + +it('follows a mocked redirect to the original server', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect(httpServer.http.url('/redirected'), 302) + ) + } + }) + + const response = await fetch(httpServer.http.url('/original')) + + expect(response.status).toBe(200) + await expect(response.text()).resolves.toBe('redirected') +}) + +it('follows a mocked redirect to a mocked response', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect(httpServer.http.url('/redirected'), 302) + ) + } + + if (request.url.endsWith('/redirected')) { + return controller.respondWith(new Response('mocked response')) + } + }) + + const response = await fetch(httpServer.http.url('/original')) + + expect(response.status).toBe(200) + await expect(response.text()).resolves.toBe('mocked response') +}) + +it('returns the redirect response as-is for a request with "manual" redirect mode', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect(httpServer.http.url('/redirected'), 301) + ) + } + }) + + const response = await fetch(httpServer.http.url('/original'), { + redirect: 'manual', + }) + + expect(response.status).toBe(301) + expect(response.headers.get('location')).toBe( + httpServer.http.url('/redirected') + ) +}) + +it('throws a network error on a redirect for a request with "error" redirect mode', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect(httpServer.http.url('/redirected'), 301) + ) + } + }) + + await expect( + fetch(httpServer.http.url('/original'), { + redirect: 'error', + }) + ).rejects.toThrow('Failed to fetch') +}) + +it('throws a network error on a non-303 redirect with a body', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect(httpServer.http.url('/redirected'), 301) + ) + } + }) + + await expect( + fetch(httpServer.http.url('/original'), { + method: 'POST', + body: 'hello world', + }) + ).rejects.toThrow('Failed to fetch') +}) + +it('throws a network error on redirects to a non-HTTP scheme', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith(Response.redirect('wss://localhost', 302)) + } + }) + + await expect(fetch(httpServer.http.url('/original'))).rejects.toThrow( + 'Failed to fetch' + ) +}) + +it('throws on a redirect with credentials for a "cors" request', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect('http://user:password@localhost', 302) + ) + } + }) + + await expect( + fetch(httpServer.http.url('/original'), { mode: 'cors' }) + ).rejects.toThrow('Failed to fetch') +}) + +it('coerces a 301/302 redirect for a POST request to a GET request', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect('http://localhost/redirected', 301) + ) + } + + if (request.method === 'GET' && request.url.endsWith('/redirected')) { + // Infer response body from the request body. + return controller.respondWith( + new Response(request.clone().body, { headers: request.headers }) + ) + } + }) + + const response = await fetch(httpServer.http.url('/original'), { + method: 'POST', + headers: { + 'content-language': 'en-US', + 'content-location': 'http://localhost/redirected', + 'content-type': 'application/json', + 'content-length': '0', + 'x-other-header': 'value', + }, + }) + + expect(response.status).toBe(200) + // Must remove body-related request headers. + expect(Array.from(response.headers)).toEqual([['x-other-header', 'value']]) + // Non-GET/HEAD request body of a 303 redirect must be null. + expect(response.body).toBeNull() +}) + +it('coerces a 303 redirect to a non-HEAD/GET request to a GET request', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect('http://localhost/redirected', 303) + ) + } + + if (request.method === 'GET' && request.url.endsWith('/redirected')) { + // Infer response body from the request body. + return controller.respondWith( + new Response(request.clone().body, { headers: request.headers }) + ) + } + }) + + const response = await fetch(httpServer.http.url('/original'), { + method: 'POST', + headers: { + 'content-language': 'en-US', + 'content-location': 'http://localhost/redirected', + 'content-type': 'application/json', + 'content-length': '0', + 'x-other-header': 'value', + }, + }) + + expect(response.status).toBe(200) + // Must remove body-related request headers. + expect(Array.from(response.headers)).toEqual([['x-other-header', 'value']]) + // Non-GET/HEAD request body of a 303 redirect must be null. + expect(response.body).toBeNull() +}) + +it('deletes sensitive request headers for a cross-origin redirect', async () => { + interceptor.on('request', ({ request, controller }) => { + if (request.url.endsWith('/original')) { + return controller.respondWith( + Response.redirect('https://example.com/redirected', 303) + ) + } + + if (request.url.endsWith('/redirected')) { + return controller.respondWith( + new Response(null, { headers: request.headers }) + ) + } + }) + + const response = await fetch(httpServer.http.url('/original'), { + headers: { + authorization: 'Bearer TOKEN', + 'proxy-authorization': 'Bearer PROXY_TOKEN', + cookie: 'a=1', + host: 'localhost', + 'x-other-header': 'value', + }, + }) + + expect(response.status).toBe(200) + expect(Array.from(response.headers)).toEqual([['x-other-header', 'value']]) + expect(response.body).toBeNull() +}) diff --git a/test/modules/fetch/compliance/fetch-request-body-used.test.ts b/test/modules/fetch/compliance/fetch-request-body-used.test.ts index 14ff1582..214f5321 100644 --- a/test/modules/fetch/compliance/fetch-request-body-used.test.ts +++ b/test/modules/fetch/compliance/fetch-request-body-used.test.ts @@ -18,7 +18,7 @@ beforeAll(async () => { }) afterEach(() => { - interceptor['emitter'].removeAllListeners() + interceptor.removeAllListeners() }) afterAll(async () => {