From 14da6cb368b01bc0c12caef700656b1be147c9a4 Mon Sep 17 00:00:00 2001 From: Mark Wylde Date: Mon, 24 May 2021 22:19:46 +1000 Subject: [PATCH] #49: Fix error on invalid request body (#54) --- lib/httpHandler.js | 65 ++++++++++++++++++++++++++++++++++----------- package-lock.json | 18 ++++++------- package.json | 2 +- test/integration.js | 38 ++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 25 deletions(-) diff --git a/lib/httpHandler.js b/lib/httpHandler.js index bd7c5ad..5114c33 100644 --- a/lib/httpHandler.js +++ b/lib/httpHandler.js @@ -8,6 +8,14 @@ const validateAlphaNumericDashDot = require('../utils/validateAlphaNumericDashDo const selectRandomItemFromArray = require('../utils/selectRandomItemFromArray'); const orderByFields = require('../utils/orderByFields'); +function handleInvalidRequestBody (error) { + if (error.message.includes('Unexpected token')) { + throw Object.assign(new Error('request body not valid json'), { statusCode: 400 }); + } else { + throw Object.assign(new Error('empty request body not allowed'), { statusCode: 400 }); + } +} + const { COMMAND, STATUS, @@ -158,7 +166,8 @@ async function handleCount (state, request, response, { collectionId, url }) { } async function handlePost (state, request, response, { collectionId }) { - const body = await finalStream(request).then(JSON.parse); + const body = await finalStream(request).then(JSON.parse) + .catch(handleInvalidRequestBody); const documents = Array.isArray(body) ? body : [body]; @@ -189,7 +198,8 @@ async function handlePost (state, request, response, { collectionId }) { } async function handlePutOne (state, request, response, { collectionId, resourceId }) { - const body = await finalStream(request).then(JSON.parse); + const body = await finalStream(request).then(JSON.parse) + .catch(handleInvalidRequestBody); const responses = await askOnAllNodes(state, { [COMMAND]: PUT, @@ -216,7 +226,8 @@ async function handlePutOne (state, request, response, { collectionId, resourceI } async function handlePutAll (state, request, response, { collectionId, url }) { - const body = await finalStream(request).then(JSON.parse); + const body = await finalStream(request).then(JSON.parse) + .catch(handleInvalidRequestBody); const responses = await askOnAllNodes(state, { [COMMAND]: PUT, @@ -238,7 +249,8 @@ async function handlePutAll (state, request, response, { collectionId, url }) { } async function handlePatchOne (state, request, response, { collectionId, resourceId }) { - const body = await finalStream(request).then(JSON.parse); + const body = await finalStream(request).then(JSON.parse) + .catch(handleInvalidRequestBody); const responses = await askOnAllNodes(state, { [COMMAND]: PATCH, @@ -265,7 +277,8 @@ async function handlePatchOne (state, request, response, { collectionId, resourc } async function handlePatchAll (state, request, response, { collectionId, url }) { - const body = await finalStream(request).then(JSON.parse); + const body = await finalStream(request).then(JSON.parse) + .catch(handleInvalidRequestBody); const responses = await askOnAllNodes(state, { [COMMAND]: PATCH, @@ -370,6 +383,18 @@ async function handleUnlock (state, request, response, { resourceId }) { writeResponse(200, {}, response); } +function handleError (request, response) { + return error => { + if (error.statusCode) { + writeResponse(error.statusCode, { error: error.message }, response); + return; + } + + console.log(error); + writeResponse(500, { error: 'unexpected server error' }, response); + }; +} + function handleSystem (state, request, response, { url }) { const [,, collectionId, resourceId] = url.pathname.split('/'); @@ -426,52 +451,62 @@ function handleExternal (state, request, response) { } if (request.method === 'GET' && resourceId) { - handleGetOne(state, request, response, { collectionId, resourceId, url }); + handleGetOne(state, request, response, { collectionId, resourceId, url }) + .catch(handleError(request, response)); return; } if (request.method === 'GET' && url.searchParams.get('count') && !resourceId) { - handleCount(state, request, response, { collectionId, url }); + handleCount(state, request, response, { collectionId, url }) + .catch(handleError(request, response)); return; } if (request.method === 'GET' && !resourceId) { - handleGetAll(state, request, response, { collectionId, url }); + handleGetAll(state, request, response, { collectionId, url }) + .catch(handleError(request, response)); return; } if (request.method === 'POST') { - handlePost(state, request, response, { collectionId, resourceId }); + handlePost(state, request, response, { collectionId, resourceId }) + .catch(handleError(request, response)); return; } if (request.method === 'PUT' && resourceId) { - handlePutOne(state, request, response, { collectionId, resourceId }); + handlePutOne(state, request, response, { collectionId, resourceId }) + .catch(handleError(request, response)); return; } if (request.method === 'PUT' && !resourceId) { - handlePutAll(state, request, response, { collectionId, url }); + handlePutAll(state, request, response, { collectionId, url }) + .catch(handleError(request, response)); return; } if (request.method === 'PATCH' && resourceId) { - handlePatchOne(state, request, response, { collectionId, resourceId }); + handlePatchOne(state, request, response, { collectionId, resourceId }) + .catch(handleError(request, response)); return; } if (request.method === 'PATCH' && !resourceId) { - handlePatchAll(state, request, response, { collectionId, url }); + handlePatchAll(state, request, response, { collectionId, url }) + .catch(handleError(request, response)); return; } if (request.method === 'DELETE' && resourceId) { - handleDeleteOne(state, request, response, { collectionId, resourceId }); + handleDeleteOne(state, request, response, { collectionId, resourceId }) + .catch(handleError(request, response)); return; } if (request.method === 'DELETE' && !resourceId) { - handleDeleteAll(state, request, response, { collectionId, url }); + handleDeleteAll(state, request, response, { collectionId, url }) + .catch(handleError(request, response)); return; } diff --git a/package-lock.json b/package-lock.json index 186b020..9501425 100644 --- a/package-lock.json +++ b/package-lock.json @@ -480,11 +480,11 @@ } }, "npm": { - "version": "7.13.0", - "resolved": "https://registry.npmjs.org/npm/-/npm-7.13.0.tgz", - "integrity": "sha512-6D9tWuUN4ef9Mi0o4Gwkv92SKsd+AS8QF/xtdWCNpX5aLlvb1x3juyiPvzPxuisxFUq2S3fZBMNehEt+Aae9Hg==", + "version": "7.14.0", + "resolved": "https://registry.npmjs.org/npm/-/npm-7.14.0.tgz", + "integrity": "sha512-GSG9/rSau8vGfkOmrmseRVYXoEjo3NPNsoM4nwvI1uWlKdzmlZ8UCw7FqCUrlQ5u0C5dRR7MG9EJUCV8LZegLA==", "requires": { - "@npmcli/arborist": "^2.5.0", + "@npmcli/arborist": "^2.6.0", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^2.2.0", "@npmcli/run-script": "^1.8.5", @@ -493,7 +493,7 @@ "ansistyles": "~0.1.3", "archy": "~1.0.0", "byte-size": "^7.0.1", - "cacache": "^15.0.6", + "cacache": "^15.1.0", "chalk": "^4.1.0", "chownr": "^2.0.0", "cli-columns": "^3.1.2", @@ -530,7 +530,7 @@ "npm-package-arg": "^8.1.2", "npm-pick-manifest": "^6.1.1", "npm-profile": "^5.0.3", - "npm-registry-fetch": "^10.1.1", + "npm-registry-fetch": "^10.1.2", "npm-user-validate": "^1.0.1", "npmlog": "~4.1.2", "opener": "^1.5.2", @@ -554,7 +554,7 @@ }, "dependencies": { "@npmcli/arborist": { - "version": "2.5.0", + "version": "2.6.0", "requires": { "@npmcli/installed-package-contents": "^1.0.7", "@npmcli/map-workspaces": "^1.0.2", @@ -792,7 +792,7 @@ "version": "7.0.1" }, "cacache": { - "version": "15.0.6", + "version": "15.1.0", "requires": { "@npmcli/move-file": "^1.0.1", "chownr": "^2.0.0", @@ -1554,7 +1554,7 @@ } }, "npm-registry-fetch": { - "version": "10.1.1", + "version": "10.1.2", "requires": { "lru-cache": "^6.0.0", "make-fetch-happen": "^8.0.9", diff --git a/package.json b/package.json index ed0012f..bb184cf 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "lockbase": "^1.0.9", "minimist": "^1.2.5", "ndjson-fe": "^1.2.7", - "npm": "^7.13.0", + "npm": "^7.14.0", "qs": "^6.10.1", "reconnecting-websocket": "^4.4.0", "server-destroy": "^1.0.1", diff --git a/test/integration.js b/test/integration.js index 0d79888..a465665 100644 --- a/test/integration.js +++ b/test/integration.js @@ -49,11 +49,49 @@ function rootMethodNotAllowed (method) { }; } +function validateBodyExists (method) { + return async t => { + t.plan(2); + + const node = await canhazdb({ host: 'localhost', port: 7071, queryPort: 8071, tls, single: true }); + + const request = await httpRequest(`${node.url}/exampleCollection`, { method }); + + await node.close(); + + t.deepEqual(request.data, { error: 'empty request body not allowed' }); + t.equal(request.status, 400); + }; +} + +function validateBodyJson (method) { + return async t => { + t.plan(2); + + const node = await canhazdb({ host: 'localhost', port: 7071, queryPort: 8071, tls, single: true }); + + const request = await httpRequest(`${node.url}/exampleCollection`, { method, data: 'not json' }); + + await node.close(); + + t.deepEqual(request.data, { error: 'request body not valid json' }); + t.equal(request.status, 400); + }; +} + test('post: root pathname', rootMethodNotAllowed('post')); test('put: root pathname', rootMethodNotAllowed('put')); test('patch: root pathname', rootMethodNotAllowed('patch')); test('delete: root pathname', rootMethodNotAllowed('delete')); +test('post: body exists', validateBodyExists('post')); +test('put: body exists', validateBodyExists('put')); +test('patch: body exists', validateBodyExists('patch')); + +test('post: body is json', validateBodyJson('post')); +test('put: body is json', validateBodyJson('put')); +test('patch: body is json', validateBodyJson('patch')); + test('post: and get some data', async t => { t.plan(3);