Skip to content

Commit d6af0cc

Browse files
committed
fix: server crash by SyntaxError from express JSON parser
1 parent 52d6797 commit d6af0cc

File tree

3 files changed

+46
-2
lines changed

3 files changed

+46
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717

1818
- Sanitization of logged queries, when running in production, if literal values contain parentheses
1919
- Read GraphiQL HTML file only once on startup, and only if GraphiQL is enabled, to avoid unnecessary file system access
20+
- Server crash by SyntaxError from express JSON parser
2021

2122
### Removed
2223

lib/logger.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,20 @@ const LOG = cds.log('graphql')
88
const { parse, visit, Kind, print } = require('graphql')
99

1010
const _isEmptyObject = o => Object.keys(o).length === 0
11+
class InvalidJSON extends Error {}
12+
InvalidJSON.prototype.name = 'Invalid JSON body'
13+
InvalidJSON.prototype.status = 400
1114

12-
router.use(express.json({ ...cds.env.server.body_parser })) //> required by logger below
13-
15+
router.use(function jsonBodyParser(req, res, next) {
16+
express.json({ ...cds.env.server.body_parser }) (req, res, function http_body_parser_next(err) {
17+
if (err) { // Need to wrap, as a rethrow would crash the server
18+
let e = new InvalidJSON(err.message)
19+
return next(e)
20+
}
21+
next()
22+
})
23+
})
24+
1425
router.use(function queryLogger(req, _, next) {
1526
let query = req.body?.query || (req.query.query && decodeURIComponent(req.query.query))
1627
// Only log requests that contain a query

test/tests/http.test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
const cds = require('@sap/cds')
2+
const path = require('path')
3+
const util = require('util')
4+
const { axios } = cds
5+
.test('serve', 'srv/empty-service.cds')
6+
.in(path.join(__dirname, '../resources/empty-csn-definitions'))
7+
const _format = e => util.formatWithOptions({ colors: false, depth: null }, ...(Array.isArray(e) ? e : [e]))
8+
9+
let _error = []
10+
11+
describe('GraphQL express json parser error scenario', () => {
12+
beforeEach(() => {
13+
console.warn = (...s) => _error.push(s) // eslint-disable-line no-console
14+
})
15+
16+
afterEach(() => {
17+
_error = []
18+
})
19+
test('should trigger InvalidJSON for malformed JSON', async () => {
20+
try {
21+
response = await axios.request({
22+
method: 'POST',
23+
url: `/graphql`,
24+
data: '{ some_value'
25+
})
26+
} catch (err) {
27+
expect(err.status).toBe(400)
28+
expect(err.response.data.error.message).toBe(`Unexpected token '"', ""{ some_value"" is not valid JSON`)
29+
expect(_format(_error[0])).toContain('InvalidJSON')
30+
}
31+
})
32+
})

0 commit comments

Comments
 (0)