From 06c7bcb0df657f900df8679246522a30325e9679 Mon Sep 17 00:00:00 2001 From: Marcel Schwarz Date: Mon, 29 Jul 2024 10:45:25 +0200 Subject: [PATCH] feat: configurable payload limit using `cds.server.body_parser.limit` (#177) --- CHANGELOG.md | 2 + lib/GraphQLAdapter.js | 2 +- package.json | 4 +- test/resources/bookshop/package.json | 1 + test/resources/custom-handlers/package.json | 4 ++ test/resources/types/package.json | 10 +++++ test/resources/types/server.js | 6 --- test/tests/types.test.js | 44 +++++++++++---------- 8 files changed, 43 insertions(+), 30 deletions(-) delete mode 100644 test/resources/types/server.js diff --git a/CHANGELOG.md b/CHANGELOG.md index f85f12ba..f5bd5c99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Support for configuring request payload limit using global flag `cds.server.body_parser.limit` + ### Changed - To improve performance, binary payloads are no longer validated to check if they are properly base64 or base64url encoded diff --git a/lib/GraphQLAdapter.js b/lib/GraphQLAdapter.js index edcab379..80182cdc 100644 --- a/lib/GraphQLAdapter.js +++ b/lib/GraphQLAdapter.js @@ -20,7 +20,7 @@ function GraphQLAdapter(options) { : defaultErrorFormatter router - .use(express.json()) //> required by logger below + .use(express.json({ ...cds.env.server.body_parser })) //> required by logger below .use(queryLogger) if (options.graphiql) router.use(graphiql) diff --git a/package.json b/package.json index dfcd9db3..dd1629f5 100644 --- a/package.json +++ b/package.json @@ -40,10 +40,10 @@ }, "devDependencies": { "@cap-js/graphql": "file:.", + "@cap-js/sqlite": "^1", "axios": "^1", "express": "^4.17.1", "jest": "^29.3.1", - "semver": "^7.4.0", - "@cap-js/sqlite": "^1" + "semver": "^7.4.0" } } diff --git a/test/resources/bookshop/package.json b/test/resources/bookshop/package.json index 5e7113e1..3f604834 100644 --- a/test/resources/bookshop/package.json +++ b/test/resources/bookshop/package.json @@ -10,6 +10,7 @@ "index.js" ], "dependencies": { + "@cap-js/graphql": "*", "@sap/cds": ">=5.9", "express": "^4.17.1" }, diff --git a/test/resources/custom-handlers/package.json b/test/resources/custom-handlers/package.json index 580c138c..e31f286b 100644 --- a/test/resources/custom-handlers/package.json +++ b/test/resources/custom-handlers/package.json @@ -1,4 +1,8 @@ { + "Note: Programmatic configuration of GraphQL protocol adapter in server.js": "", + "__dependencies": { + "@cap-js/graphql": "*" + }, "devDependencies": { "@cap-js/sqlite": "*" } diff --git a/test/resources/types/package.json b/test/resources/types/package.json index 717be87e..5a74c9a2 100644 --- a/test/resources/types/package.json +++ b/test/resources/types/package.json @@ -1,5 +1,15 @@ { "dependencies": { + "@cap-js/graphql": "*" + }, + "devDependencies": { "@cap-js/sqlite": "*" + }, + "cds": { + "server": { + "body_parser": { + "limit": "110KB" + } + } } } \ No newline at end of file diff --git a/test/resources/types/server.js b/test/resources/types/server.js deleted file mode 100644 index 6939c6d6..00000000 --- a/test/resources/types/server.js +++ /dev/null @@ -1,6 +0,0 @@ -const cds = require('@sap/cds') -const path = require('path') -const protocols = cds.env.protocols ??= {} -if (!protocols.graphql) protocols.graphql = { - path: '/graphql', impl: path.join(__dirname, '../../../index.js') -} \ No newline at end of file diff --git a/test/tests/types.test.js b/test/tests/types.test.js index 53ae3315..fe81ccf1 100644 --- a/test/tests/types.test.js +++ b/test/tests/types.test.js @@ -1,13 +1,19 @@ +const consumers = require('node:stream/consumers') + const { gql } = require('../util') const _toBase64Url = value => value.replace(/\//g, '_').replace(/\+/g, '-') -const _getTestBuffer = length => { - const testString = 'Test String! ' - let string = testString.repeat(Math.ceil(length / testString.length)).slice(0, length) - return Buffer.from(string) +const _getTestString = length => { + const testString = 'This is a test string! ' + return testString.repeat(Math.ceil(length / testString.length)).slice(0, length) } +const _getTestBuffer = length => Buffer.from(_getTestString(length)) + +// e.g. base64 string with length 96 -> requires buffer with length 72 +const _neededBufferLengthForBase64StringWithLength = length => Math.ceil((length * 6) / 8) + const _getMutationForFieldWithLiteralValue = (field, value, quoted) => ({ query: gql` mutation { @@ -48,9 +54,7 @@ describe('graphql - types parsing and validation', () => { // Prevent axios from throwing errors for non 2xx status codes axios.defaults.validateStatus = false - beforeEach(async () => { - await data.reset() - }) + beforeEach(data.reset) describe('cds.Binary', () => { const field = 'myBinary' @@ -1002,12 +1006,9 @@ describe('graphql - types parsing and validation', () => { }) // Note: maps to same type as cds.Binary - // REVISIT: express-graphql limits request body size to 100kb by default: - // - https://github.com/graphql/express-graphql/issues/346 - // - https://github.com/graphql/express-graphql/blob/28e4c2924ea6984bf918465cefdadae340d8780e/src/parseBody.ts#L96 - describe.skip('cds.LargeBinary', () => { + describe('cds.LargeBinary', () => { const field = 'myLargeBinary' - const buffer = _getTestBuffer(500000) // 500 KB + const buffer = _getTestBuffer(_neededBufferLengthForBase64StringWithLength(105000)) // 105 KB as base64 string describe('input literal', () => { test('cds.LargeBinary is correctly parsed from large input literal base64 encoded string value', async () => { @@ -1018,7 +1019,8 @@ describe('graphql - types parsing and validation', () => { expect(response.data).toEqual({ data }) const result = await SELECT.one.from('sap.cds.graphql.types.MyEntity').columns(field) - expect(result[field]).toEqual(buffer) + const bufferFromDB = await consumers.buffer(result[field]) + expect(bufferFromDB).toEqual(buffer) }) test('cds.LargeBinary is correctly parsed from large input literal base64url encoded string value', async () => { @@ -1029,7 +1031,8 @@ describe('graphql - types parsing and validation', () => { expect(response.data).toEqual({ data }) const result = await SELECT.one.from('sap.cds.graphql.types.MyEntity').columns(field) - expect(result[field]).toEqual(buffer) + const bufferFromDB = await consumers.buffer(result[field]) + expect(bufferFromDB).toEqual(buffer) }) }) @@ -1042,7 +1045,8 @@ describe('graphql - types parsing and validation', () => { expect(response.data).toEqual({ data }) const result = await SELECT.one.from('sap.cds.graphql.types.MyEntity').columns(field) - expect(result[field]).toEqual(buffer) + const bufferFromDB = await consumers.buffer(result[field]) + expect(bufferFromDB).toEqual(buffer) }) test('cds.LargeBinary is correctly parsed from large variable base64url encoded string value', async () => { @@ -1053,18 +1057,16 @@ describe('graphql - types parsing and validation', () => { expect(response.data).toEqual({ data }) const result = await SELECT.one.from('sap.cds.graphql.types.MyEntity').columns(field) - expect(result[field]).toEqual(buffer) + const bufferFromDB = await consumers.buffer(result[field]) + expect(bufferFromDB).toEqual(buffer) }) }) }) // Note: maps to same type as cds.String - // REVISIT: express-graphql limits request body size to 100kb by default: - // - https://github.com/graphql/express-graphql/issues/346 - // - https://github.com/graphql/express-graphql/blob/28e4c2924ea6984bf918465cefdadae340d8780e/src/parseBody.ts#L96 - describe.skip('cds.LargeString', () => { + describe('cds.LargeString', () => { const field = 'myLargeString' - const value = 'This is a test string! '.repeat(100000) + const value = _getTestString(105000) // 105 KB test('cds.LargeString is correctly parsed from input literal', async () => { const body = _getMutationForFieldWithLiteralValue(field, value, true)