From 28a3b7873be1c268fa16cde3e5dd1963a54f9e9d Mon Sep 17 00:00:00 2001 From: Antoine Arlaud Date: Wed, 8 May 2024 23:24:43 +0200 Subject: [PATCH] fix: make request from secondry instead of redirect --- .../routesHandlers/httpRequestHandler.ts | 27 ++++++-- package-lock.json | 34 ++++++++++ package.json | 1 + test/unit/old-client-redirect.test.ts | 68 ++++++++++++++++++- 4 files changed, 123 insertions(+), 7 deletions(-) diff --git a/lib/server/routesHandlers/httpRequestHandler.ts b/lib/server/routesHandlers/httpRequestHandler.ts index 91e65ad72..1e97cdf25 100644 --- a/lib/server/routesHandlers/httpRequestHandler.ts +++ b/lib/server/routesHandlers/httpRequestHandler.ts @@ -4,8 +4,10 @@ import { getDesensitizedToken } from '../utils/token'; import { getSocketConnections } from '../socket'; import { incrementHttpRequestsTotal } from '../../common/utils/metrics'; import { hostname } from 'node:os'; +import { makeStreamingRequestToDownstream } from '../../common/http/request'; +import { PostFilterPreparedRequest } from '../../common/relay/prepareRequest'; -export const overloadHttpRequestWithConnectionDetailsMiddleware = ( +export const overloadHttpRequestWithConnectionDetailsMiddleware = async ( req: Request, res: Response, next: NextFunction, @@ -25,10 +27,27 @@ export const overloadHttpRequestWithConnectionDetailsMiddleware = ( localHostname.endsWith('-1') && localHostname.match(regex) ) { - logger.debug({}, 'redirecting to primary'); - const url = new URL(`http://${req.host}${req.url}`); + const url = new URL(`http://${req.hostname}${req.url}`); url.searchParams.append('connection_role', 'primary'); - return res.redirect(url.toString()); + logger.debug({}, 'Making request to primary'); + const postFilterPreparedRequest: PostFilterPreparedRequest = { + url: url.toString(), + headers: req.headers, + method: req.method, + }; + if (req.body) { + postFilterPreparedRequest.body = JSON.stringify(req.body); + } + try { + const httpResponse = await makeStreamingRequestToDownstream( + postFilterPreparedRequest, + ); + res.writeHead(httpResponse.statusCode ?? 500, httpResponse.headers); + return httpResponse.pipe(res); + } catch (err) { + logger.error({ err }, `Error in HTTP middleware: ${err}`); + res.status(500).send('Error forwarding request to primary'); + } } else { logger.warn({ desensitizedToken }, 'no matching connection found'); return res.status(404).json({ ok: false }); diff --git a/package-lock.json b/package-lock.json index 2c22394af..7ce91a77f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -52,6 +52,7 @@ "@types/minimist": "1.2.5", "@types/node": "^18.15.11", "@types/prettier": "2.6.0", + "@types/supertest": "^6.0.2", "@typescript-eslint/eslint-plugin": "^5.30.4", "@typescript-eslint/parser": "^5.30.4", "detect-port": "^1.5.1", @@ -2401,6 +2402,12 @@ "resolved": "https://registry.npmjs.org/@types/cookie/-/cookie-0.4.1.tgz", "integrity": "sha512-XW/Aa8APYr6jSVVA1y/DEIZX0/GMKLEVekNG727R8cs56ahETkRAy/3DR7+fJyh7oUgGwNQaRfXCun0+KbWY7Q==" }, + "node_modules/@types/cookiejar": { + "version": "2.1.5", + "resolved": "https://registry.npmjs.org/@types/cookiejar/-/cookiejar-2.1.5.tgz", + "integrity": "sha512-he+DHOWReW0nghN24E1WUqM0efK4kI9oTqDm6XmK8ZPe2djZ90BSNdGnIyCLzCPw7/pogPlGbzI2wHGGmi4O/Q==", + "dev": true + }, "node_modules/@types/cookies": { "version": "0.9.0", "resolved": "https://registry.npmjs.org/@types/cookies/-/cookies-0.9.0.tgz", @@ -2561,6 +2568,12 @@ "@types/koa": "*" } }, + "node_modules/@types/methods": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/@types/methods/-/methods-1.1.4.tgz", + "integrity": "sha512-ymXWVrDiCxTBE3+RIrrP533E70eA+9qu7zdWoHuOmGujkYtzf4HQF96b8nwHLqhuf4ykX61IGRIB38CC6/sImQ==", + "dev": true + }, "node_modules/@types/mime": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/@types/mime/-/mime-1.3.5.tgz", @@ -2632,6 +2645,27 @@ "integrity": "sha512-9aEbYZ3TbYMznPdcdr3SmIrLXwC/AKZXQeCf9Pgao5CKb8CyHuEX5jzWPTkvregvhRJHcpRO6BFoGW9ycaOkYw==", "dev": true }, + "node_modules/@types/superagent": { + "version": "8.1.7", + "resolved": "https://registry.npmjs.org/@types/superagent/-/superagent-8.1.7.tgz", + "integrity": "sha512-NmIsd0Yj4DDhftfWvvAku482PZum4DBW7U51OvS8gvOkDDY0WT1jsVyDV3hK+vplrsYw8oDwi9QxOM7U68iwww==", + "dev": true, + "dependencies": { + "@types/cookiejar": "^2.1.5", + "@types/methods": "^1.1.4", + "@types/node": "*" + } + }, + "node_modules/@types/supertest": { + "version": "6.0.2", + "resolved": "https://registry.npmjs.org/@types/supertest/-/supertest-6.0.2.tgz", + "integrity": "sha512-137ypx2lk/wTQbW6An6safu9hXmajAifU/s7szAHLN/FeIm5w7yR0Wkl9fdJMRSHwOn4HLAI0DaB2TOORuhPDg==", + "dev": true, + "dependencies": { + "@types/methods": "^1.1.4", + "@types/superagent": "^8.1.0" + } + }, "node_modules/@types/yargs": { "version": "17.0.32", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.32.tgz", diff --git a/package.json b/package.json index 07b8fc0b6..9993487bc 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "@types/minimist": "1.2.5", "@types/node": "^18.15.11", "@types/prettier": "2.6.0", + "@types/supertest": "^6.0.2", "@typescript-eslint/eslint-plugin": "^5.30.4", "@typescript-eslint/parser": "^5.30.4", "detect-port": "^1.5.1", diff --git a/test/unit/old-client-redirect.test.ts b/test/unit/old-client-redirect.test.ts index 6fe00d799..3cf0c7ac6 100644 --- a/test/unit/old-client-redirect.test.ts +++ b/test/unit/old-client-redirect.test.ts @@ -1,6 +1,12 @@ +import bodyParser from 'body-parser'; import { overloadHttpRequestWithConnectionDetailsMiddleware } from '../../lib/server/routesHandlers/httpRequestHandler'; import express from 'express'; import request from 'supertest'; +import nock from 'nock'; +import path from 'path'; +import { readFileSync } from 'node:fs'; + +const fixtures = path.resolve(__dirname, '..', 'fixtures'); jest.mock('../../lib/server/socket', () => { const originalModule = jest.requireActual('../../lib/server/socket'); @@ -28,7 +34,16 @@ jest.mock('node:os', () => { describe('Testing older clients specific logic', () => { it('Testing the old client redirected to primary from secondary pods', async () => { + nock(`http://127.0.0.1`) + .persist() + .get( + '/broker/7fe7a57b-aa0d-416a-97fc-472061737e25/path?connection_role=primary', + ) + .reply(() => { + return [200, { test: 'value' }]; + }); const app = express(); + app.use(bodyParser.json()); app.all( '/broker/:token/*', overloadHttpRequestWithConnectionDetailsMiddleware, @@ -37,9 +52,56 @@ describe('Testing older clients specific logic', () => { const response = await request(app).get( '/broker/7fe7a57b-aa0d-416a-97fc-472061737e25/path', ); - expect(response.status).toEqual(302); - expect(response.headers['location']).toEqual( - 'http://127.0.0.1/broker/7fe7a57b-aa0d-416a-97fc-472061737e25/path?connection_role=primary', + expect(response.status).toEqual(200); + expect(response.body).toEqual({ test: 'value' }); + }); + it('Testing the old client redirected to primary from secondary pods - POST request', async () => { + nock(`http://127.0.0.1`) + .persist() + .post( + '/broker/7fe7a57b-aa0d-416a-97fc-472061737e25/path?connection_role=primary', + ) + .reply((_uri, requestBody) => { + return [200, requestBody]; + }); + const app = express(); + app.use(bodyParser.json()); + app.all( + '/broker/:token/*', + overloadHttpRequestWithConnectionDetailsMiddleware, + ); + + const response = await request(app) + .post('/broker/7fe7a57b-aa0d-416a-97fc-472061737e25/path') + .send({ test: 'value2' }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual({ test: 'value2' }); + }); + it('Testing the old client redirected to primary from secondary pods - get request', async () => { + const fileJson = JSON.parse( + readFileSync(`${fixtures}/accept/ghe.json`).toString(), ); + nock(`http://127.0.0.1`) + .persist() + .get( + '/broker/7fe7a57b-aa0d-416a-97fc-472061737e25/file?connection_role=primary', + ) + .reply(() => { + return [200, fileJson]; + }); + const app = express(); + app.use(bodyParser.json()); + app.all( + '/broker/:token/*', + overloadHttpRequestWithConnectionDetailsMiddleware, + ); + + const response = await request(app).get( + '/broker/7fe7a57b-aa0d-416a-97fc-472061737e25/file', + ); + + expect(response.status).toEqual(200); + expect(response.body).toEqual(fileJson); }); });