diff --git a/src/handlers/response-interceptor.ts b/src/handlers/response-interceptor.ts index c2cd4423..cacadfb0 100644 --- a/src/handlers/response-interceptor.ts +++ b/src/handlers/response-interceptor.ts @@ -100,11 +100,18 @@ function decompress( * Copy original headers * https://github.com/apache/superset/blob/9773aba522e957ed9423045ca153219638a85d2f/superset-frontend/webpack.proxy-config.js#L78 */ -function copyHeaders(originalResponse, response): void { +function copyHeaders( + originalResponse: http.IncomingMessage, + response: TRes, +): void { debug('copy original response headers'); - response.statusCode = originalResponse.statusCode; - response.statusMessage = originalResponse.statusMessage; + if (originalResponse.statusCode) { + response.statusCode = originalResponse.statusCode; + } + if (originalResponse.statusMessage) { + response.statusMessage = originalResponse.statusMessage; + } if (response.setHeader) { let keys = Object.keys(originalResponse.headers); @@ -115,15 +122,17 @@ function copyHeaders(originalResponse, response): void { keys.forEach((key) => { let value = originalResponse.headers[key]; - if (key === 'set-cookie') { + if (key === 'set-cookie' && value) { // remove cookie domain value = Array.isArray(value) ? value : [value]; value = value.map((x) => x.replace(/Domain=[^;]+?/i, '')); } - response.setHeader(key, value); + response.setHeader(key, value as number | string | string[]); }); } else { - response.headers = originalResponse.headers; + if ('headers' in response) { + response.headers = originalResponse.headers; + } } } diff --git a/src/http-proxy-middleware.ts b/src/http-proxy-middleware.ts index 02bc758e..252bcd6a 100644 --- a/src/http-proxy-middleware.ts +++ b/src/http-proxy-middleware.ts @@ -9,8 +9,8 @@ import { Debug as debug } from './debug.js'; import { getPlugins } from './get-plugins.js'; import { getLogger } from './logger.js'; import { matchPathFilter } from './path-filter.js'; -import * as PathRewriter from './path-rewriter.js'; -import * as Router from './router.js'; +import { createPathRewriter } from './path-rewriter.js'; +import { getTarget } from './router.js'; import type { Filter, Logger, Options, RequestHandler } from './types.js'; import { getFunctionName } from './utils/function.js'; @@ -22,7 +22,7 @@ export class HttpProxyMiddleware< private serverOnCloseSubscribed = false; private proxyOptions: Options; private proxy: ProxyServer; - private pathRewriter; + private pathRewriter: ReturnType>; private logger: Logger; constructor(options: Options) { @@ -35,7 +35,7 @@ export class HttpProxyMiddleware< this.registerPlugins(this.proxy, this.proxyOptions); - this.pathRewriter = PathRewriter.createPathRewriter(this.proxyOptions.pathRewrite); // returns undefined when "pathRewrite" is not provided + this.pathRewriter = createPathRewriter(this.proxyOptions.pathRewrite); // returns undefined when "pathRewrite" is not provided // https://github.com/chimurai/http-proxy-middleware/issues/19 // expose function to upgrade externally @@ -181,7 +181,7 @@ export class HttpProxyMiddleware< * @param {Object} req * @return {Object} proxy options */ - private prepareProxyRequest = async (req: http.IncomingMessage) => { + private prepareProxyRequest = async (req: TReq) => { const newProxyOptions = Object.assign({}, this.proxyOptions); // Apply in order: @@ -194,11 +194,11 @@ export class HttpProxyMiddleware< }; // Modify option.target when router present. - private applyRouter = async (req: http.IncomingMessage, options: Options) => { + private applyRouter = async (req: TReq, options: Options) => { let newTarget; if (options.router) { - newTarget = await Router.getTarget(req, options); + newTarget = await getTarget(req, options); if (newTarget) { debug('router new target: "%s"', newTarget); @@ -208,9 +208,12 @@ export class HttpProxyMiddleware< }; // rewrite path - private applyPathRewrite = async (req: http.IncomingMessage, pathRewriter) => { + private applyPathRewrite = async ( + req: TReq, + pathRewriter: ReturnType>, + ) => { if (pathRewriter) { - const path = await pathRewriter(req.url, req); + const path = await pathRewriter(req.url as string, req); if (typeof path === 'string') { debug('pathRewrite new path: %s', path); diff --git a/src/path-rewriter.ts b/src/path-rewriter.ts index 9d097b92..3fa23a65 100644 --- a/src/path-rewriter.ts +++ b/src/path-rewriter.ts @@ -1,7 +1,10 @@ +import type { IncomingMessage } from 'node:http'; + import isPlainObject from 'is-plain-obj'; import { Debug } from './debug.js'; import { ERRORS } from './errors.js'; +import type { PathRewriteConfig } from './types.js'; const debug = Debug.extend('path-rewriter'); @@ -9,11 +12,10 @@ type RewriteRule = { regex: RegExp; value: string }; /** * Create rewrite function, to cache parsed rewrite rules. - * - * @param {Object} rewriteConfig - * @return {Function} Function to rewrite paths; This function should accept `path` (request.url) as parameter */ -export function createPathRewriter(rewriteConfig) { +export function createPathRewriter( + rewriteConfig: PathRewriteConfig | undefined, +) { let rulesCache: RewriteRule[]; if (!isValidRewriteConfig(rewriteConfig)) { @@ -28,7 +30,7 @@ export function createPathRewriter(rewriteConfig) { return rewritePath; } - function rewritePath(path) { + function rewritePath(path: string) { let result = path; for (const rule of rulesCache) { @@ -43,7 +45,9 @@ export function createPathRewriter(rewriteConfig) { } } -function isValidRewriteConfig(rewriteConfig) { +function isValidRewriteConfig( + rewriteConfig: PathRewriteConfig | undefined, +): boolean { if (typeof rewriteConfig === 'function') { return true; } else if (isPlainObject(rewriteConfig)) { @@ -55,7 +59,7 @@ function isValidRewriteConfig(rewriteConfig) { } } -function parsePathRewriteRules(rewriteConfig: Record) { +function parsePathRewriteRules(rewriteConfig: PathRewriteConfig | undefined) { const rules: RewriteRule[] = []; if (isPlainObject(rewriteConfig)) { diff --git a/src/router.ts b/src/router.ts index 0f5954fb..9f2d5fcc 100644 --- a/src/router.ts +++ b/src/router.ts @@ -1,10 +1,17 @@ +import type * as http from 'node:http'; + +import type { ProxyServerOptions } from 'httpxy'; import isPlainObject from 'is-plain-obj'; import { Debug } from './debug.js'; +import type { Options } from './index.js'; const debug = Debug.extend('router'); -export async function getTarget(req, config) { +export async function getTarget< + TReq extends http.IncomingMessage = http.IncomingMessage, + TRes extends http.ServerResponse = http.ServerResponse, +>(req: TReq, config: Options) { let newTarget; const router = config.router; @@ -17,10 +24,13 @@ export async function getTarget(req, config) { return newTarget; } -function getTargetFromProxyTable(req, table) { +function getTargetFromProxyTable( + req: TReq, + table: Record, +) { let result; - const host = req.headers.host; - const path = req.url; + const host = req.headers.host ?? ''; + const path = req.url ?? ''; const hostAndPath = host + path; @@ -45,6 +55,6 @@ function getTargetFromProxyTable(req, table) { return result; } -function containsPath(v) { +function containsPath(v: string) { return v.indexOf('/') > -1; } diff --git a/src/types.ts b/src/types.ts index 7de4696b..28e3f97d 100644 --- a/src/types.ts +++ b/src/types.ts @@ -58,6 +58,11 @@ export interface OnProxyEvent< export type Logger = Pick; +export type PathRewriteConfig = + | { [regexp: string]: string } + | ((path: string, req: TReq) => string | undefined) + | ((path: string, req: TReq) => Promise); + export interface Options< TReq extends http.IncomingMessage = http.IncomingMessage, TRes extends http.ServerResponse = http.ServerResponse, @@ -82,10 +87,8 @@ export interface Options< * ``` * @link https://github.com/chimurai/http-proxy-middleware/blob/master/recipes/pathRewrite.md */ - pathRewrite?: - | { [regexp: string]: string } - | ((path: string, req: TReq) => string | undefined) - | ((path: string, req: TReq) => Promise); + pathRewrite?: PathRewriteConfig; + /** * Access the internal http-proxy server instance to customize behavior * @@ -141,7 +144,7 @@ export interface Options< * @link https://github.com/chimurai/http-proxy-middleware/blob/master/recipes/router.md */ router?: - | { [hostOrPath: string]: ProxyServerOptions['target'] } + | Record | ((req: TReq) => ProxyServerOptions['target']) | ((req: TReq) => Promise); /** diff --git a/test/e2e/express-error-middleware.spec.ts b/test/e2e/express-error-middleware.spec.ts index 22a473e1..eaf908f2 100644 --- a/test/e2e/express-error-middleware.spec.ts +++ b/test/e2e/express-error-middleware.spec.ts @@ -1,3 +1,4 @@ +import type express from 'express'; import request from 'supertest'; import { describe, expect, it } from 'vitest'; @@ -12,12 +13,12 @@ describe('express error middleware', () => { router: (req) => undefined, // Trigger "Error: Must provide a proper URL as target" }); - const errorMiddleware = (err, req, res, next) => { + const errorMiddleware: express.ErrorRequestHandler = (err, req, res, next) => { httpProxyError = err; res.status(504).send('Something broke!'); }; - const app = createApp(proxyMiddleware, errorMiddleware); + const app = createApp(proxyMiddleware, errorMiddleware as any); const response = await request(app).get('/get').expect(504); expect(httpProxyError?.message).toBe('Must provide a proper URL as target'); diff --git a/test/e2e/express-router.spec.ts b/test/e2e/express-router.spec.ts index 00309d3f..56cfbbde 100644 --- a/test/e2e/express-router.spec.ts +++ b/test/e2e/express-router.spec.ts @@ -2,7 +2,7 @@ import express from 'express'; import request from 'supertest'; import { beforeEach, describe, expect, it } from 'vitest'; -import type { Options } from '../../src/index.js'; +import type { Filter, Options } from '../../src/index.js'; import { createProxyMiddleware } from './test-kit.js'; describe('Usage in Express', () => { @@ -19,11 +19,11 @@ describe('Usage in Express', () => { // sub route config const sub = express.Router(); - function filter(pathname, req) { + const filter: Filter = (pathname, req) => { const urlFilter = new RegExp('^/sub/api'); const match = urlFilter.test(pathname); return match; - } + }; /** * Mount proxy without 'path' in sub route @@ -50,8 +50,8 @@ describe('Usage in Express', () => { }); }); - function jsonMiddleware(data) { - return (req, res) => { + function jsonMiddleware(data: Record) { + return (req: express.Request, res: express.Response) => { res.json(data); }; } diff --git a/test/e2e/http-proxy-middleware.spec.ts b/test/e2e/http-proxy-middleware.spec.ts index 7852ab7c..834640c4 100644 --- a/test/e2e/http-proxy-middleware.spec.ts +++ b/test/e2e/http-proxy-middleware.spec.ts @@ -7,7 +7,7 @@ import { getLocal } from 'mockttp'; import request from 'supertest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Logger } from '../../src/types.js'; +import type { Filter, Logger } from '../../src/types.js'; import { createApp, createAppWithPath, createProxyMiddleware, fixRequestBody } from './test-kit.js'; describe('E2E http-proxy-middleware', () => { @@ -135,7 +135,7 @@ describe('E2E http-proxy-middleware', () => { describe('custom pathFilter matcher/filter', () => { it('should have response body: "HELLO WEB"', async () => { - const filter = (path, req) => { + const filter: Filter = (path, req) => { return true; }; @@ -154,7 +154,7 @@ describe('E2E http-proxy-middleware', () => { }); it('should not proxy when filter returns false', async () => { - const filter = (path, req) => { + const filter: Filter = (path, req) => { return false; }; @@ -174,7 +174,7 @@ describe('E2E http-proxy-middleware', () => { it('should not proxy when filter throws Error', async () => { const myError = new Error('MY_ERROR'); - const filter = (path, req) => { + const filter: Filter = (path, req) => { throw myError; }; diff --git a/test/e2e/router.spec.ts b/test/e2e/router.spec.ts index 5c5560b2..2603ee04 100644 --- a/test/e2e/router.spec.ts +++ b/test/e2e/router.spec.ts @@ -150,7 +150,7 @@ describe('E2E router', () => { }); describe('router with proxyTable', () => { - let agent; + let agent: request.Agent; beforeEach(() => { const app = createAppWithPath( diff --git a/test/e2e/test-kit.ts b/test/e2e/test-kit.ts index d024bd6e..65ae391e 100644 --- a/test/e2e/test-kit.ts +++ b/test/e2e/test-kit.ts @@ -7,7 +7,7 @@ export { fixRequestBody, } from '../../src/index.js'; -export function createApp(...middlewares): Express { +export function createApp(...middlewares: RequestHandler[]): Express { const app = express(); app.use(...middlewares); return app; diff --git a/test/types.spec.ts b/test/types.spec.ts index 5687595a..b40b3bf1 100644 --- a/test/types.spec.ts +++ b/test/types.spec.ts @@ -92,12 +92,12 @@ describe('http-proxy-middleware TypeScript Types', () => { }); it('should have router Type with function', () => { - options = { router: (path) => '/path' }; + options = { router: (req) => '/path' }; expect(options).toBeDefined(); }); it('should have router Type with async function', () => { - options = { router: async (path) => '/path' }; + options = { router: async (req) => '/path' }; expect(options).toBeDefined(); }); }); @@ -183,6 +183,14 @@ describe('http-proxy-middleware TypeScript Types', () => { middleware({ router: (req) => req.params, pathFilter: (pathname, req) => !!req.params, + pathRewrite: (path, req) => { + req.params; + + // @ts-expect-error: should error when request is typed as `any` + req.unknownProperty; + + return path; + }, on: { error(error, req, res, target) { req.params; diff --git a/test/unit/path-filter.spec.ts b/test/unit/path-filter.spec.ts index 021110de..7f2bf343 100644 --- a/test/unit/path-filter.spec.ts +++ b/test/unit/path-filter.spec.ts @@ -1,11 +1,12 @@ -import type * as http from 'node:http'; +import { IncomingMessage } from 'node:http'; +import { Socket } from 'node:net'; import { beforeEach, describe, expect, it } from 'vitest'; import { matchPathFilter } from '../../src/path-filter.js'; describe('Path Filter', () => { - const fakeReq = {} as http.IncomingMessage; + const fakeReq = new IncomingMessage(new Socket()); describe('String path matching', () => { let result; @@ -179,10 +180,10 @@ describe('Path Filter', () => { }); describe('Use function for matching', () => { - const testFunctionAsPathFilter = (val) => { + const testFunctionAsPathFilter = (val: any) => { return matchPathFilter(fn, 'http://localhost/api/foo/bar', fakeReq); - function fn(path, req) { + function fn(path: string, req: any) { return val; } }; @@ -204,7 +205,7 @@ describe('Path Filter', () => { }); describe('Test invalid pathFilters', () => { - let testPathFilter; + let testPathFilter: (pathFilter: any) => () => void; beforeEach(() => { testPathFilter = (pathFilter) => { diff --git a/test/unit/path-rewriter.spec.ts b/test/unit/path-rewriter.spec.ts index 95dad027..280934fa 100644 --- a/test/unit/path-rewriter.spec.ts +++ b/test/unit/path-rewriter.spec.ts @@ -1,11 +1,16 @@ +import { IncomingMessage } from 'node:http'; +import { Socket } from 'node:net'; + import { beforeEach, describe, expect, it } from 'vitest'; import { createPathRewriter } from '../../src/path-rewriter.js'; +import type { PathRewriteConfig } from '../../src/types.js'; describe('Path rewriting', () => { - let rewriter; - let result; - let config; + const fakeReq = new IncomingMessage(new Socket()); + let rewriter: Exclude, undefined>; + let result: ReturnType; + let config: PathRewriteConfig; describe('Rewrite rules configuration and usage', () => { beforeEach(() => { @@ -20,37 +25,37 @@ describe('Path rewriting', () => { }); beforeEach(() => { - rewriter = createPathRewriter(config); + rewriter = createPathRewriter(config) as any; }); it('should rewrite path', () => { - result = rewriter('/api/old/index.json'); + result = rewriter('/api/old/index.json', fakeReq); expect(result).toBe('/api/new/index.json'); }); it('should remove path', () => { - result = rewriter('/remove/old/index.json'); + result = rewriter('/remove/old/index.json', fakeReq); expect(result).toBe('/old/index.json'); }); it('should leave path intact', () => { - result = rewriter('/foo/bar/index.json'); + result = rewriter('/foo/bar/index.json', fakeReq); expect(result).toBe('/foo/bar/index.json'); }); it('should not rewrite path when config-key does not match url with test(regex)', () => { - result = rewriter('/invalid/bar/foo.json'); + result = rewriter('/invalid/bar/foo.json', fakeReq); expect(result).toBe('/path/new/bar/foo.json'); expect(result).not.toBe('/invalid/new/bar/foo.json'); }); it('should rewrite path when config-key does match url with test(regex)', () => { - result = rewriter('/valid/foo/bar.json'); + result = rewriter('/valid/foo/bar.json', fakeReq); expect(result).toBe('/path/new/foo/bar.json'); }); it('should return first match when similar paths are configured', () => { - result = rewriter('/some/specific/path/bar.json'); + result = rewriter('/some/specific/path/bar.json', fakeReq); expect(result).toBe('/awe/some/specific/path/bar.json'); }); }); @@ -63,89 +68,65 @@ describe('Path rewriting', () => { }); beforeEach(() => { - rewriter = createPathRewriter(config); + rewriter = createPathRewriter(config) as any; }); it('should add base path to requests', () => { - result = rewriter('/api/books/123'); + result = rewriter('/api/books/123', fakeReq); expect(result).toBe('/extra/base/path/api/books/123'); }); }); describe('Rewrite function', () => { - beforeEach(() => { - rewriter = (fn) => { - const rewriteFn = createPathRewriter(fn); - const requestPath = '/123/456'; - return rewriteFn(requestPath); - }; - }); + const originalRequestPath = '/123/456'; it('should return unmodified path', () => { - const rewriteFn = (path) => { + rewriter = createPathRewriter((path) => { return path; - }; + }) as any; - expect(rewriter(rewriteFn)).toBe('/123/456'); + expect(rewriter(originalRequestPath, fakeReq)).toBe('/123/456'); }); it('should return alternative path', () => { - const rewriteFn = (path) => { + rewriter = createPathRewriter((path) => { return '/foo/bar'; - }; + }) as any; - expect(rewriter(rewriteFn)).toBe('/foo/bar'); + expect(rewriter(originalRequestPath, fakeReq)).toBe('/foo/bar'); }); it('should return replaced path', () => { - const rewriteFn = (path) => { + rewriter = createPathRewriter((path) => { return path.replace('/456', '/789'); - }; + }) as any; - expect(rewriter(rewriteFn)).toBe('/123/789'); + expect(rewriter(originalRequestPath, fakeReq)).toBe('/123/789'); }); // Same tests as the above three, but async it('is async and should return unmodified path', () => { - const rewriteFn = async (path) => { - const promise = new Promise((resolve, reject) => { - resolve(path); - }); - const changed = await promise; - return changed; - }; + rewriter = createPathRewriter(async (path) => path) as any; - return expect(rewriter(rewriteFn)).resolves.toBe('/123/456'); + return expect(rewriter(originalRequestPath, fakeReq)).resolves.toBe('/123/456'); }); it('is async and should return alternative path', () => { - const rewriteFn = async (path) => { - const promise = new Promise((resolve, reject) => { - resolve('/foo/bar'); - }); - const changed = await promise; - return changed; - }; + rewriter = createPathRewriter(async (path) => '/foo/bar') as any; - return expect(rewriter(rewriteFn)).resolves.toBe('/foo/bar'); + return expect(rewriter(originalRequestPath, fakeReq)).resolves.toBe('/foo/bar'); }); it('is async and should return replaced path', () => { - const rewriteFn = async (path) => { - const promise = new Promise((resolve, reject) => { - resolve(path.replace('/456', '/789')); - }); - const changed = await promise; - return changed; - }; + rewriter = createPathRewriter(async (path) => path.replace('/456', '/789')) as any; - return expect(rewriter(rewriteFn)).resolves.toBe('/123/789'); + return expect(rewriter(originalRequestPath, fakeReq)).resolves.toBe('/123/789'); }); }); describe('Invalid configuration', () => { - let badFn; + let badFn: (cfg?: PathRewriteConfig) => () => void; beforeEach(() => { badFn = (cfg) => { @@ -157,15 +138,15 @@ describe('Path rewriting', () => { it('should return undefined when no config is provided', () => { expect(badFn()()).toBeUndefined(); - expect(badFn(null)()).toBeUndefined(); + expect(badFn(null as unknown as PathRewriteConfig)()).toBeUndefined(); expect(badFn(undefined)()).toBeUndefined(); }); it('should throw when bad config is provided', () => { - expect(badFn(123)).toThrow(Error); - expect(badFn('abc')).toThrow(Error); - expect(badFn([])).toThrow(Error); - expect(badFn([1, 2, 3])).toThrow(Error); + expect(badFn(123 as unknown as PathRewriteConfig)).toThrow(Error); + expect(badFn('abc' as unknown as PathRewriteConfig)).toThrow(Error); + expect(badFn([] as unknown as PathRewriteConfig)).toThrow(Error); + expect(badFn([1, 2, 3] as unknown as PathRewriteConfig)).toThrow(Error); }); it('should not throw when empty Object config is provided', () => { @@ -177,7 +158,7 @@ describe('Path rewriting', () => { }); it('should not throw when async function config is provided', () => { - expect(badFn(async () => {})).not.toThrow(Error); + expect(badFn((async () => {}) as unknown as any)).not.toThrow(Error); }); }); }); diff --git a/test/unit/router.spec.ts b/test/unit/router.spec.ts index 62acec15..d3e7f65a 100644 --- a/test/unit/router.spec.ts +++ b/test/unit/router.spec.ts @@ -1,20 +1,23 @@ +import { IncomingMessage } from 'node:http'; +import { Socket } from 'node:net'; + import { beforeEach, describe, expect, it } from 'vitest'; import { getTarget } from '../../src/router.js'; +import type { Options } from '../../src/types.js'; describe('router unit test', () => { - let fakeReq; - let config; - let result; - let proxyOptionWithRouter; + let fakeReq: IncomingMessage; + let config: Options; + let result: ReturnType; + let proxyOptionWithRouter: Options; beforeEach(() => { - fakeReq = { - headers: { - host: 'localhost', - }, - url: '/', + fakeReq = new IncomingMessage(new Socket()); + fakeReq.headers = { + host: 'localhost', }; + fakeReq.url = '/'; config = { target: 'http://localhost:6000', @@ -22,7 +25,7 @@ describe('router unit test', () => { }); describe('router.getTarget from function', () => { - let request; + let request: IncomingMessage; beforeEach(() => { proxyOptionWithRouter = { @@ -48,7 +51,7 @@ describe('router unit test', () => { }); describe('router.getTarget from async function', () => { - let request; + let request: IncomingMessage; beforeEach(() => { proxyOptionWithRouter = { diff --git a/tsconfig.json b/tsconfig.json index ebfa44e2..5b2d9feb 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -11,7 +11,7 @@ "incremental": true, "declaration": true, "strict": true, - "noImplicitAny": false, + "noImplicitAny": true, "skipLibCheck": true }, "include": ["./src"]