Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: configurable payload limit using cds.server.body_parser.limit #177

Merged
merged 16 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.max_request_body_size`

### Changed

- To improve performance, binary payloads are no longer validated to check if they are properly base64 or base64url encoded
Expand Down
6 changes: 5 additions & 1 deletion lib/GraphQLAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,12 @@ function GraphQLAdapter(options) {
? require(path.join(cds.root, options.errorFormatter))
: defaultErrorFormatter

const bodyParserOptions = {}
const { max_request_body_size } = cds.env.server
if (max_request_body_size) bodyParserOptions.limit = max_request_body_size

router
.use(express.json()) //> required by logger below
.use(express.json(bodyParserOptions)) //> required by logger below
.use(queryLogger)

if (options.graphiql) router.use(graphiql)
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
1 change: 1 addition & 0 deletions test/resources/bookshop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"index.js"
],
"dependencies": {
"@cap-js/graphql": "*",
"@sap/cds": ">=5.9",
"express": "^4.17.1"
},
Expand Down
4 changes: 4 additions & 0 deletions test/resources/custom-handlers/package.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
{
"Note: Programmatic configuration of GraphQL protocol adapter in server.js": "",
"__dependencies": {
"@cap-js/graphql": "*"
},
"devDependencies": {
"@cap-js/sqlite": "*"
}
Expand Down
8 changes: 8 additions & 0 deletions test/resources/types/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
{
"dependencies": {
"@cap-js/graphql": "*"
},
"devDependencies": {
"@cap-js/sqlite": "*"
},
"cds": {
"server": {
"max_request_body_size": "110KB"
}
}
}
6 changes: 0 additions & 6 deletions test/resources/types/server.js

This file was deleted.

44 changes: 23 additions & 21 deletions test/tests/types.test.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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)
})
})

Expand All @@ -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 () => {
Expand All @@ -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)
Expand Down
Loading