From f2a0af329969f8a70b3967a969146758a73d8b0e Mon Sep 17 00:00:00 2001 From: Steven Chim <655241+chimurai@users.noreply.github.com> Date: Fri, 10 May 2024 17:22:36 +0200 Subject: [PATCH] fix(logger-plugin): log target port when router option is used (#1001) * fix(logger-plugin): log target port when router option is used * refactor(logger-plugin): extract getPort logic and add some tests * ci(spellcheck): add jsonplaceholder.typicode.com --- CHANGELOG.md | 1 + cspell.json | 2 ++ src/plugins/default/logger-plugin.ts | 16 +++++++++++++-- src/utils/logger-plugin.ts | 11 +++++++++++ test/e2e/http-proxy-middleware.spec.ts | 27 ++++++++++++++++++++++++-- test/unit/utils/logger-plugin.spec.ts | 23 ++++++++++++++++++++++ 6 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 src/utils/logger-plugin.ts create mode 100644 test/unit/utils/logger-plugin.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ac70212..b2966dca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - refactor(errors): improve pathFilter error message - fix(logger-plugin): fix missing target port - ci(package): npm package provenance +- fix(logger-plugin): log target port when router option is used ## [v3.0.0](https://github.com/chimurai/http-proxy-middleware/releases/tag/v3.0.0) diff --git a/cspell.json b/cspell.json index 7db6159f..56456824 100644 --- a/cspell.json +++ b/cspell.json @@ -28,6 +28,7 @@ "fastify", "globbing", "gzipped", + "jsonplaceholder", "lcov", "Lenna", "lipsum", @@ -45,6 +46,7 @@ "restream", "snyk", "streamify", + "typicode", "vhosted", "websockets", "xfwd" diff --git a/src/plugins/default/logger-plugin.ts b/src/plugins/default/logger-plugin.ts index 5e368030..a05b3662 100644 --- a/src/plugins/default/logger-plugin.ts +++ b/src/plugins/default/logger-plugin.ts @@ -2,6 +2,7 @@ import { URL } from 'url'; import { Plugin } from '../../types'; import { getLogger } from '../../logger'; import type { IncomingMessage } from 'node:http'; +import { getPort } from '../../utils/logger-plugin'; type ExpressRequest = { /** Express req.baseUrl */ @@ -43,8 +44,19 @@ export const loggerPlugin: Plugin = (proxyServer, options) => { const originalUrl = req.originalUrl ?? `${req.baseUrl || ''}${req.url}`; // construct targetUrl - const target = new URL(options.target as URL); - target.pathname = proxyRes.req.path; + const port = getPort(proxyRes.req?.agent?.sockets); + + const obj = { + protocol: proxyRes.req.protocol, + host: proxyRes.req.host, + pathname: proxyRes.req.path, + } as URL; + + const target = new URL(`${obj.protocol}//${obj.host}${obj.pathname}`); + + if (port) { + target.port = port; + } const targetUrl = target.toString(); const exchange = `[HPM] ${req.method} ${originalUrl} -> ${targetUrl} [${proxyRes.statusCode}]`; diff --git a/src/utils/logger-plugin.ts b/src/utils/logger-plugin.ts new file mode 100644 index 00000000..869e5827 --- /dev/null +++ b/src/utils/logger-plugin.ts @@ -0,0 +1,11 @@ +import type { Agent } from 'node:http'; + +export type Sockets = Pick; + +/** + * Get port from target + * Using proxyRes.req.agent.sockets to determine the target port + */ +export function getPort(sockets?: Sockets): string | undefined { + return Object.keys(sockets || {})?.[0]?.split(':')[1]; +} diff --git a/test/e2e/http-proxy-middleware.spec.ts b/test/e2e/http-proxy-middleware.spec.ts index 96f2cd80..c08890a7 100644 --- a/test/e2e/http-proxy-middleware.spec.ts +++ b/test/e2e/http-proxy-middleware.spec.ts @@ -4,6 +4,7 @@ import { Mockttp, getLocal, CompletedRequest } from 'mockttp'; import type * as http from 'http'; import type * as express from 'express'; import * as bodyParser from 'body-parser'; +import type { Logger } from '../../src/types'; describe('E2E http-proxy-middleware', () => { describe('http-proxy-middleware creation', () => { @@ -433,15 +434,18 @@ describe('E2E http-proxy-middleware', () => { describe('option.logger', () => { let logMessages: string[]; + let customLogger: Logger; beforeEach(() => { logMessages = []; - const customLogger = { + customLogger = { info: (message: string) => logMessages.push(message), warn: (message: string) => logMessages.push(message), error: (message: string) => logMessages.push(message), }; + }); + it('should have logged messages', async () => { agent = request( createApp( createProxyMiddleware({ @@ -451,9 +455,28 @@ describe('E2E http-proxy-middleware', () => { }), ), ); + + await mockTargetServer.forGet('/api/foo/bar').thenReply(200); + await agent.get(`/api/foo/bar`).expect(200); + + expect(logMessages).not.toBeUndefined(); + expect(logMessages.length).toBe(1); + expect(logMessages.at(0)).toBe( + `[HPM] GET /api/foo/bar -> http://localhost:${mockTargetServer.port}/api/foo/bar [200]`, + ); }); - it('should have logged messages', async () => { + it('should have logged messages when router used', async () => { + agent = request( + createApp( + createProxyMiddleware({ + router: () => `http://localhost:${mockTargetServer.port}`, + pathFilter: '/api', + logger: customLogger, + }), + ), + ); + await mockTargetServer.forGet('/api/foo/bar').thenReply(200); await agent.get(`/api/foo/bar`).expect(200); diff --git a/test/unit/utils/logger-plugin.spec.ts b/test/unit/utils/logger-plugin.spec.ts new file mode 100644 index 00000000..c10dcae8 --- /dev/null +++ b/test/unit/utils/logger-plugin.spec.ts @@ -0,0 +1,23 @@ +import { getPort, type Sockets } from '../../../src/utils/logger-plugin'; + +describe('getPort()', () => { + it('should return port from proxyRes.req.agent.sockets', () => { + const sockets = { + 'jsonplaceholder.typicode.com:80:': [], + } as unknown as Sockets; + + expect(getPort(sockets)).toBe('80'); + }); + + it('should handle missing "sockets" from proxyRes?.req?.agent?.sockets', () => { + const sockets = undefined; + + expect(getPort(sockets)).toBe(undefined); + }); + + it('should handle empty "sockets" from proxyRes?.req?.agent?.sockets', () => { + const sockets = {} as unknown as Sockets; + + expect(getPort(sockets)).toBe(undefined); + }); +});