From 0c853a372b82f490ca2c8b2ab1cea8c8226bfb32 Mon Sep 17 00:00:00 2001 From: Fabien 'egg' O'Carroll Date: Thu, 16 Dec 2021 13:59:39 +0200 Subject: [PATCH] Supported restricting limit="all" in get helper (#13903) refs https://github.com/TryGhost/Team/issues/1251 With sites that have a huge number of resources, using limit="all" can cause OOM errors at the Node level. Administrators now have the ability to cap limit="all" requests via config. This only affects the get helper used in themes, not the API, this is by design as themes have less visibility of issues. --- core/frontend/helpers/get.js | 4 ++ test/unit/frontend/helpers/get.test.js | 74 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/core/frontend/helpers/get.js b/core/frontend/helpers/get.js index e10ae3afdf..0d00540540 100644 --- a/core/frontend/helpers/get.js +++ b/core/frontend/helpers/get.js @@ -109,6 +109,10 @@ function parseOptions(globals, data, options) { options.filter = resolvePaths(globals, data, options.filter); } + if (options.limit === 'all' && config.get('getHelperLimitAllMax')) { + options.limit = config.get('getHelperLimitAllMax'); + } + return options; } diff --git a/test/unit/frontend/helpers/get.test.js b/test/unit/frontend/helpers/get.test.js index e1d25263c1..333a80b5e6 100644 --- a/test/unit/frontend/helpers/get.test.js +++ b/test/unit/frontend/helpers/get.test.js @@ -8,6 +8,7 @@ const get = require('../../../../core/frontend/helpers/get'); const models = require('../../../../core/server/models'); const api = require('../../../../core/server/api'); +const proxy = require('../../../../core/frontend/services/proxy'); describe('{{#get}} helper', function () { let fn; @@ -314,4 +315,77 @@ describe('{{#get}} helper', function () { }).catch(done); }); }); + + describe('limit=all max', function () { + let browseStub; + + beforeEach(function () { + browseStub = sinon.stub().resolves(); + sinon.stub(api.v2, 'postsPublic').get(() => { + return { + browse: browseStub + }; + }); + sinon.stub(api.v3, 'postsPublic').get(() => { + return { + browse: browseStub + }; + }); + sinon.stub(api.canary, 'postsPublic').get(() => { + return { + browse: browseStub + }; + }); + }); + + it('Behaves normally without config', async function () { + locals = {root: {_locals: {apiVersion: 'v2'}}}; + await get.call( + {}, + 'posts', + {hash: {limit: 'all'}, data: locals, fn: fn, inverse: inverse} + ); + browseStub.firstCall.args[0].limit.should.eql('all'); + locals = {root: {_locals: {apiVersion: 'v3'}}}; + await get.call( + {}, + 'posts', + {hash: {limit: 'all'}, data: locals, fn: fn, inverse: inverse} + ); + browseStub.secondCall.args[0].limit.should.eql('all'); + locals = {root: {_locals: {apiVersion: 'canary'}}}; + await get.call( + {}, + 'posts', + {hash: {limit: 'all'}, data: locals, fn: fn, inverse: inverse} + ); + browseStub.thirdCall.args[0].limit.should.eql('all'); + }); + + it('Replaces "all" with "getHelperLimitAllMax" config, if present', async function () { + sinon.stub(proxy.config, 'get').withArgs('getHelperLimitAllMax').returns(2); + + locals = {root: {_locals: {apiVersion: 'v2'}}}; + await get.call( + {}, + 'posts', + {hash: {limit: 'all'}, data: locals, fn: fn, inverse: inverse} + ); + browseStub.firstCall.args[0].limit.should.eql(2); + locals = {root: {_locals: {apiVersion: 'v3'}}}; + await get.call( + {}, + 'posts', + {hash: {limit: 'all'}, data: locals, fn: fn, inverse: inverse} + ); + browseStub.secondCall.args[0].limit.should.eql(2); + locals = {root: {_locals: {apiVersion: 'canary'}}}; + await get.call( + {}, + 'posts', + {hash: {limit: 'all'}, data: locals, fn: fn, inverse: inverse} + ); + browseStub.thirdCall.args[0].limit.should.eql(2); + }); + }); });