From 697ff80dad4a7393a530b0ae08e3e0c01c8889f3 Mon Sep 17 00:00:00 2001 From: chris48s <chris48s@users.noreply.github.com> Date: Fri, 11 Jan 2019 21:50:49 +0000 Subject: [PATCH] limit the size of response we will accept (#2726) limit the size of response we will accept --- config/custom-environment-variables.yml | 2 + config/default.yml | 2 + lib/request-handler.js | 37 +++++++++++--- lib/request-handler.spec.js | 57 +++++++++++++++++++++ lib/server.js | 3 ++ package-lock.json | 67 +++++++++++++------------ package.json | 1 + services/base.js | 3 +- services/errors.js | 1 + 9 files changed, 135 insertions(+), 38 deletions(-) diff --git a/config/custom-environment-variables.yml b/config/custom-environment-variables.yml index 5464351c5a..dcdf4a2e85 100644 --- a/config/custom-environment-variables.yml +++ b/config/custom-environment-variables.yml @@ -40,6 +40,8 @@ public: rateLimit: 'RATE_LIMIT' + fetchLimit: 'FETCH_LIMIT' + private: azure_devops_token: 'AZURE_DEVOPS_TOKEN' bintray_user: 'BINTRAY_USER' diff --git a/config/default.yml b/config/default.yml index de33996239..3774c44487 100644 --- a/config/default.yml +++ b/config/default.yml @@ -34,4 +34,6 @@ public: handleInternalErrors: true + fetchLimit: '10MB' + private: {} diff --git a/lib/request-handler.js b/lib/request-handler.js index 357f66237a..52ef958549 100644 --- a/lib/request-handler.js +++ b/lib/request-handler.js @@ -10,7 +10,11 @@ const makeBadge = require('../gh-badges/lib/make-badge') const analytics = require('./analytics') const { makeSend } = require('./result-sender') const queryString = require('query-string') -const { Inaccessible } = require('../services/errors') +const { + Inaccessible, + InvalidResponse, + ShieldsRuntimeError, +} = require('../services/errors') const { setCacheHeaders } = require('../services/cache-headers') // We avoid calling the vendor's server for computation of the information in a @@ -64,6 +68,7 @@ function flattenQueryParams(queryParams) { // the service uses // - cacheLength: An optional badge or category-specific cache length // (in number of seconds) to be used in preference to the default +// - fetchLimitBytes: A limit on the response size we're willing to parse // // For safety, the service must declare the query parameters it wants to use. // Only the declared parameters (and the global parameters) are provided to @@ -83,7 +88,10 @@ function handleRequest(cacheHeaderConfig, handlerOptions) { } const allowedKeys = flattenQueryParams(handlerOptions.queryParams) - const { cacheLength: serviceCacheLengthSeconds } = handlerOptions + const { + cacheLength: serviceCacheLengthSeconds, + fetchLimitBytes, + } = handlerOptions return (queryParams, match, end, ask) => { const reqTime = new Date() @@ -167,7 +175,8 @@ function handleRequest(cacheHeaderConfig, handlerOptions) { options.headers['User-Agent'] = options.headers['User-Agent'] || 'Shields.io' - request(options, (err, res, body) => { + let bufferLength = 0 + const r = request(options, (err, res, body) => { if (res != null && res.headers != null) { const cacheControl = res.headers['cache-control'] if (cacheControl != null) { @@ -181,6 +190,18 @@ function handleRequest(cacheHeaderConfig, handlerOptions) { } callback(err, res, body) }) + r.on('data', chunk => { + bufferLength += chunk.length + if (bufferLength > fetchLimitBytes) { + r.abort() + r.emit( + 'error', + new InvalidResponse({ + prettyMessage: 'Maximum response size exceeded', + }) + ) + } + }) } // Wrapper around `cachingRequest` that returns a promise rather than @@ -189,9 +210,13 @@ function handleRequest(cacheHeaderConfig, handlerOptions) { new Promise((resolve, reject) => { cachingRequest(uri, options, (err, res, buffer) => { if (err) { - // Wrap the error in an Inaccessible so it can be identified - // by the BaseService handler. - reject(new Inaccessible({ underlyingError: err })) + if (err instanceof ShieldsRuntimeError) { + reject(err) + } else { + // Wrap the error in an Inaccessible so it can be identified + // by the BaseService handler. + reject(new Inaccessible({ underlyingError: err })) + } } else { resolve({ res, buffer }) } diff --git a/lib/request-handler.spec.js b/lib/request-handler.spec.js index 8b81871a7c..099e279583 100644 --- a/lib/request-handler.spec.js +++ b/lib/request-handler.spec.js @@ -2,6 +2,7 @@ const { expect } = require('chai') const fetch = require('node-fetch') +const nock = require('nock') const portfinder = require('portfinder') const analytics = require('./analytics') const Camp = require('camp') @@ -24,6 +25,20 @@ function fakeHandler(queryParams, match, sendBadge, request) { sendBadge(format, badgeData) } +function fakeHandlerWithNetworkIo(queryParams, match, sendBadge, request) { + const [, someValue, format] = match + request('https://www.google.com/foo/bar', (err, res, buffer) => { + const badgeData = getBadgeData('testing', queryParams) + if (err) { + badgeData.text[1] = err.prettyMessage + sendBadge(format, badgeData) + return + } + badgeData.text[1] = someValue + sendBadge(format, badgeData) + }) +} + describe('The request handler', function() { before(analytics.load) @@ -78,6 +93,48 @@ describe('The request handler', function() { }) }) + describe('the response size limit', function() { + beforeEach(function() { + camp.route( + /^\/testing\/([^/]+)\.(svg|png|gif|jpg|json)$/, + handleRequest(standardCacheHeaders, { + handler: fakeHandlerWithNetworkIo, + fetchLimitBytes: 100, + }) + ) + }) + + it('should not throw an error if the response <= fetchLimitBytes', async function() { + nock('https://www.google.com') + .get('/foo/bar') + .once() + .reply(200, 'x'.repeat(100)) + const res = await fetch(`${baseUrl}/testing/123.json`) + expect(res.ok).to.be.true + expect(await res.json()).to.deep.equal({ + name: 'testing', + value: '123', + }) + }) + + it('should throw an error if the response is > fetchLimitBytes', async function() { + nock('https://www.google.com') + .get('/foo/bar') + .once() + .reply(200, 'x'.repeat(101)) + const res = await fetch(`${baseUrl}/testing/123.json`) + expect(res.ok).to.be.true + expect(await res.json()).to.deep.equal({ + name: 'testing', + value: 'Maximum response size exceeded', + }) + }) + + afterEach(function() { + nock.cleanAll() + }) + }) + describe('caching', function() { describe('standard query parameters', function() { let handlerCallCount diff --git a/lib/server.js b/lib/server.js index 4452d184b4..ca5c56090d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -1,5 +1,6 @@ 'use strict' +const bytes = require('bytes') const path = require('path') const url = require('url') const Joi = require('joi') @@ -79,6 +80,7 @@ const publicConfigSchema = Joi.object({ }, rateLimit: Joi.boolean().required(), handleInternalErrors: Joi.boolean().required(), + fetchLimit: Joi.string().regex(/^[0-9]+(b|kb|mb|gb|tb)$/i), }).required() const privateConfigSchema = Joi.object({ @@ -182,6 +184,7 @@ module.exports = class Server { handleInternalErrors: config.public.handleInternalErrors, cacheHeaders: config.public.cacheHeaders, profiling: config.public.profiling, + fetchLimitBytes: bytes(config.public.fetchLimit), } ) ) diff --git a/package-lock.json b/package-lock.json index 4cb38f93f5..ed89bd4f19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1321,7 +1321,7 @@ "dependencies": { "chalk": { "version": "1.1.3", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "dev": true, "requires": { @@ -2058,7 +2058,7 @@ }, "util": { "version": "0.10.3", - "resolved": "https://registry.npmjs.org/util/-/util-0.10.3.tgz", + "resolved": "http://registry.npmjs.org/util/-/util-0.10.3.tgz", "integrity": "sha1-evsa/lCAUkZInj23/g7TeTNqwPk=", "dev": true, "requires": { @@ -2241,7 +2241,7 @@ }, "babel-plugin-react-require": { "version": "3.0.0", - "resolved": "https://registry.npmjs.org/babel-plugin-react-require/-/babel-plugin-react-require-3.0.0.tgz", + "resolved": "http://registry.npmjs.org/babel-plugin-react-require/-/babel-plugin-react-require-3.0.0.tgz", "integrity": "sha1-Lk57RJa5OmVKHIAEInbeTk7rIOM=", "dev": true }, @@ -2259,7 +2259,7 @@ }, "babel-plugin-syntax-jsx": { "version": "6.18.0", - "resolved": "https://registry.npmjs.org/babel-plugin-syntax-jsx/-/babel-plugin-syntax-jsx-6.18.0.tgz", + "resolved": "http://registry.npmjs.org/babel-plugin-syntax-jsx/-/babel-plugin-syntax-jsx-6.18.0.tgz", "integrity": "sha1-CvMqmm4Tyno/1QaeYtew9Y0NiUY=", "dev": true }, @@ -2518,7 +2518,7 @@ }, "browserify-rsa": { "version": "4.0.1", - "resolved": "https://registry.npmjs.org/browserify-rsa/-/browserify-rsa-4.0.1.tgz", + "resolved": "http://registry.npmjs.org/browserify-rsa/-/browserify-rsa-4.0.1.tgz", "integrity": "sha1-IeCr+vbyApzy+vsTNWenAdQTVSQ=", "dev": true, "requires": { @@ -2583,7 +2583,7 @@ }, "buffer": { "version": "4.9.1", - "resolved": "https://registry.npmjs.org/buffer/-/buffer-4.9.1.tgz", + "resolved": "http://registry.npmjs.org/buffer/-/buffer-4.9.1.tgz", "integrity": "sha1-bRu2AbB6TvztlwlBMgkwJ8lbwpg=", "dev": true, "requires": { @@ -2630,6 +2630,11 @@ "integrity": "sha1-hZgoeOIbmOHGZCXgPQF0eI9Wnug=", "dev": true }, + "bytes": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/bytes/-/bytes-3.0.0.tgz", + "integrity": "sha1-0ygVQE1olpn4Wk6k+odV3ROpYEg=" + }, "cacache": { "version": "11.3.2", "resolved": "https://registry.npmjs.org/cacache/-/cacache-11.3.2.tgz", @@ -3689,7 +3694,7 @@ "dependencies": { "json5": { "version": "1.0.1", - "resolved": "https://registry.npmjs.org/json5/-/json5-1.0.1.tgz", + "resolved": "http://registry.npmjs.org/json5/-/json5-1.0.1.tgz", "integrity": "sha512-aKS4WQjPenRxiQsC93MNfjx+nbF4PAdYzmd/1JIj8HYzqfbu86beTuNgXDzPknWk0n0uARlyewZo4s++ES36Ow==", "requires": { "minimist": "^1.2.0" @@ -4481,7 +4486,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -5464,7 +5469,7 @@ }, "events": { "version": "1.1.1", - "resolved": "https://registry.npmjs.org/events/-/events-1.1.1.tgz", + "resolved": "http://registry.npmjs.org/events/-/events-1.1.1.tgz", "integrity": "sha1-nr23Y1rQmccNzEwqH1AEKI6L2SQ=", "dev": true }, @@ -5718,7 +5723,7 @@ "dependencies": { "core-js": { "version": "1.2.7", - "resolved": "https://registry.npmjs.org/core-js/-/core-js-1.2.7.tgz", + "resolved": "http://registry.npmjs.org/core-js/-/core-js-1.2.7.tgz", "integrity": "sha1-ZSKUwUZR2yj6k70tX/KYOk8IxjY=", "dev": true }, @@ -5887,7 +5892,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -5985,7 +5990,7 @@ "dependencies": { "chalk": { "version": "1.1.3", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "dev": true, "requires": { @@ -6028,7 +6033,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -7091,7 +7096,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "requires": { "core-util-is": "~1.0.0", @@ -7136,7 +7141,7 @@ }, "htmlescape": { "version": "1.1.1", - "resolved": "https://registry.npmjs.org/htmlescape/-/htmlescape-1.1.1.tgz", + "resolved": "http://registry.npmjs.org/htmlescape/-/htmlescape-1.1.1.tgz", "integrity": "sha1-OgPtwiFLyjtmQko+eVk0lQnLA1E=", "dev": true }, @@ -8274,7 +8279,7 @@ }, "json5": { "version": "0.5.1", - "resolved": "https://registry.npmjs.org/json5/-/json5-0.5.1.tgz", + "resolved": "http://registry.npmjs.org/json5/-/json5-0.5.1.tgz", "integrity": "sha1-Hq3nrMASA0rYTiOWdn6tn6VJWCE=", "dev": true }, @@ -8557,7 +8562,7 @@ }, "globby": { "version": "6.1.0", - "resolved": "https://registry.npmjs.org/globby/-/globby-6.1.0.tgz", + "resolved": "http://registry.npmjs.org/globby/-/globby-6.1.0.tgz", "integrity": "sha1-9abXDoOV4hyFj7BInWTfAkJNUGw=", "dev": true, "requires": { @@ -8570,7 +8575,7 @@ "dependencies": { "pify": { "version": "2.3.0", - "resolved": "https://registry.npmjs.org/pify/-/pify-2.3.0.tgz", + "resolved": "http://registry.npmjs.org/pify/-/pify-2.3.0.tgz", "integrity": "sha1-7RQaasBDqEnqWISY59yosVMw6Qw=", "dev": true } @@ -8649,7 +8654,7 @@ "dependencies": { "chalk": { "version": "1.1.3", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", + "resolved": "http://registry.npmjs.org/chalk/-/chalk-1.1.3.tgz", "integrity": "sha1-qBFcVeSnAv5NFQq9OHKCKn4J/Jg=", "dev": true, "requires": { @@ -9184,7 +9189,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -10208,7 +10213,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -10658,7 +10663,7 @@ }, "convert-source-map": { "version": "1.6.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/convert-source-map/-/convert-source-map-1.6.0.tgz", "integrity": "sha512-eFu7XigvxdZ1ETfbgPBohgyQ/Z++C0eEhTor0qRwBw9unw+L0/6V8wkSuGgzdThkiS5lSpdptOQPD8Ak40a+7A==", "dev": true, "requires": { @@ -10798,7 +10803,7 @@ }, "glob": { "version": "7.1.3", - "resolved": false, + "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.3.tgz", "integrity": "sha512-vcfuiIxogLV4DlGBHIUOwI0IbrJ8HWPc4MU7HzviGeNho/UJDfi6B5p3sHeWIQ0KGIU0Jpxi5ZHxemQfLkkAwQ==", "dev": true, "requires": { @@ -10935,7 +10940,7 @@ }, "istanbul-lib-report": { "version": "2.0.2", - "resolved": false, + "resolved": "https://registry.npmjs.org/istanbul-lib-report/-/istanbul-lib-report-2.0.2.tgz", "integrity": "sha512-rJ8uR3peeIrwAxoDEbK4dJ7cqqtxBisZKCuwkMtMv0xYzaAnsAi3AHrHPAAtNXzG/bcCgZZ3OJVqm1DTi9ap2Q==", "dev": true, "requires": { @@ -10967,7 +10972,7 @@ }, "istanbul-reports": { "version": "2.0.1", - "resolved": false, + "resolved": "https://registry.npmjs.org/istanbul-reports/-/istanbul-reports-2.0.1.tgz", "integrity": "sha512-CT0QgMBJqs6NJLF678ZHcquUAZIoBIUNzdJrRJfpkI9OnzG6MkUfHxbJC3ln981dMswC7/B1mfX3LNkhgJxsuw==", "dev": true, "requires": { @@ -11381,7 +11386,7 @@ }, "safe-buffer": { "version": "5.1.2", - "resolved": false, + "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==", "dev": true }, @@ -12228,7 +12233,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -13315,7 +13320,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -14519,7 +14524,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "requires": { "core-util-is": "~1.0.0", @@ -14740,7 +14745,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -14801,7 +14806,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { diff --git a/package.json b/package.json index a027a6354c..42ea444345 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "url": "https://github.com/badges/shields" }, "dependencies": { + "bytes": "^3.0.0", "camp": "~17.2.2", "chalk": "^2.4.2", "check-node-version": "^3.1.0", diff --git a/services/base.js b/services/base.js index 86aa7f1bbc..ce8eb369a8 100644 --- a/services/base.js +++ b/services/base.js @@ -371,7 +371,7 @@ class BaseService { static register({ camp, handleRequest, githubApiProvider }, serviceConfig) { this.validateDefinition() - const { cacheHeaders: cacheHeaderConfig } = serviceConfig + const { cacheHeaders: cacheHeaderConfig, fetchLimitBytes } = serviceConfig camp.route( this._regex, handleRequest(cacheHeaderConfig, { @@ -395,6 +395,7 @@ class BaseService { sendBadge(format, badgeData) }, cacheLength: this._cacheLength, + fetchLimitBytes, }) ) } diff --git a/services/errors.js b/services/errors.js index 6df664d08f..074e508c38 100644 --- a/services/errors.js +++ b/services/errors.js @@ -100,6 +100,7 @@ class Deprecated extends ShieldsRuntimeError { } module.exports = { + ShieldsRuntimeError, NotFound, InvalidResponse, Inaccessible, -- GitLab