feat: reject large bodies in the client

Increase the maximum allowed body size to 21MB on the server side, so we
can actually accept files of up to 20MB, and reject requests on the
client side that have a Content-Length of over 20MB.

It's better to do this on the client side as the server will close the
WebSocket connection if too much data is passed through it. This is
quite disruptive and leaves the original HTTP request hanging until it
times out.

This also sets the .nvmrc to match the actual Node version used, and
fixes a typo in the README.

.nvmrc

@@ -1 +1 @@
-12
+16

README.md

@@ -508,7 +508,7 @@ the Broker Client will then, when doing variable replacement, look to see if the
 GITHUB_TOKEN_POOL=token1, token2, token3
 ```
 
-And then the Broker Server would, any time it needed `GITHUB_TOKEN`, instead take an item from the `GITHUB_TOKEN_POOL`.
+And then the Broker Client would, any time it needed `GITHUB_TOKEN`, instead take an item from the `GITHUB_TOKEN_POOL`.
 
 Credentials will be taken in a round-robin fashion, so the first, the second, the third, etc, etc, until it reaches the end
 and then takes the first one again.

lib/relay.js

@@ -176,6 +176,7 @@ function requestHandler(filterRules) {
               metrics.incrementUnableToSizeResponse();
             }
           } else {
+            logContext.ioErrorType = ioResponse.errorType;
             logger.info(logContext, logMsg);
           }
 
@@ -400,6 +401,26 @@ function responseHandler(filterRules, config, io) {
             });
           }
 
+          // all headers converted to lower-case
+          const contentLength = response.headers && response.headers['content-length'];
+          // Note that the other side of the request will also check the length and will also reject it if it's too large
+          // Set to 20MB even though the server is 21MB because the server looks at the total data travelling through the websocket,
+          // not just the size of the body, so allow 1MB for miscellaneous data (e.g., headers, Primus overhead)
+          const maxLength = parseInt(config.socketMaxResponseLength) || 20971520;
+          if (contentLength && contentLength > maxLength) {
+            const errorMessage = `body size of ${contentLength} is greater than max allowed of ${maxLength} bytes`;
+            logError(logContext, {
+              errorMessage
+            });
+            return emit({
+              status: 500,
+              errorType: 'BODY_TOO_LARGE',
+              body: {
+                message: errorMessage
+              }
+            })
+          }
+
           const status = (response && response.statusCode) || 500;
           if (config.RES_BODY_URL_SUB && isJson(response.headers)) {
             const replaced = replaceUrlPartialChunk(responseBody, null, config);

lib/server/socket.js

@@ -12,7 +12,7 @@ module.exports = ({ server, filters, config }) => {
   const ioConfig = {
     transformer: 'engine.io',
     parser: 'EJSON',
-    maxLength: parseInt(config.socketMaxResponseLength) || 20971520, // support up to 20MB in response bodies
+    maxLength: parseInt(config.socketMaxResponseLength) || 22020096, // support up to 21MB in response bodies
     transport: { allowEIO3: true },
     pingInterval: parseInt(config.socketPingInterval) || 30000,
     compression: Boolean(config.socketUseCompression) || false,

test/fixtures/client/filters.json

@@ -108,6 +108,12 @@
       "origin": "http://localhost:${originPort}"
     },
 
+    {
+      "path": "/huge-file",
+      "method": "GET",
+      "origin": "http://localhost:${originPort}"
+    },
+
     {
       "path": "/echo-param-protected/:param",
       "method": "GET",
@@ -165,6 +171,11 @@
       "method": "GET"
     },
 
+    {
+      "path": "/huge-file",
+      "method": "GET"
+    },
+
     {
       "path": "/echo-query/filtered",
       "method": "GET",

test/functional/server-client.test.js

@@ -53,7 +53,7 @@ test('proxy requests originating from behind the broker server', (t) => {
   server.io.on('connection', (socket) => {
     socket.on('identify', (clientData) => {
       const token = clientData.token;
-      t.plan(30);
+      t.plan(31);
 
       t.test('identification', (t) => {
         const filters = require(`${clientRootPath}/${ACCEPT}`);
@@ -447,6 +447,15 @@ test('proxy requests originating from behind the broker server', (t) => {
         });
       });
 
+      t.test('reject responses that are too large', (t) => {
+        const url = `http://localhost:${serverPort}/broker/${token}/huge-file`;
+        request({ url, method: 'get' }, (err, res) => {
+          t.equal(res.statusCode, 500, '500 statusCode');
+          t.equal(res.body, '{"message":"body size of 20971532 is greater than max allowed of 20971520 bytes"}', 'error returned');
+          t.end();
+        });
+      });
+
       t.test('allow request to git client with valid param', (t) => {
         const url = `http://localhost:${serverPort}/broker/${token}/snykgit/echo-query`;
         const qs = { proxyMe: 'please' };

test/utils.ts

@@ -92,6 +92,13 @@ export function createTestServer(echoServerPort = port()) {
     },
   );
 
+  echoServerRoutes.get(
+    '/huge-file',
+    (req, res) => {
+      res.json({ data: "a".repeat(20971521) });
+    },
+  );
+
   echoServerRoutes.all('*', (req, res) => {
     res.send(false);
   });