Merge pull request #759 from snyk/fix/make-request-to-primary-instead…

…-of-redirecting fix: make request from secondry instead of redirect [HYB-512]
snyk · May 9, 2024 · 1979a22 · 1979a22
2 parents a71b773 + 28a3b78
commit 1979a22
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 7 deletions.
diff --git 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
diff --git 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
@@ -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);
   });
 });