diff --git a/lib/rbUtil.js b/lib/rbUtil.js index 4dc6ad747..39327e5d9 100644 --- a/lib/rbUtil.js +++ b/lib/rbUtil.js @@ -54,7 +54,7 @@ function read(req) { }); req.on('end', function() { - return Buffer.concat(chunks); + resolve(Buffer.concat(chunks)); }); }); } @@ -64,7 +64,8 @@ rbUtil.parsePOST = function parsePOST(req) { var readIt = (req.method === 'PUT') || (req.method === 'POST' && req.headers && - /^application\/json/i.test(req.headers['content-type'])); + (/^application\/json/i.test(req.headers['content-type']) + || !req.headers['content-type'])); if (readIt) { return read(req); @@ -72,17 +73,10 @@ rbUtil.parsePOST = function parsePOST(req) { return P.resolve(); } else { // Parse the POST - var headers = req.headers; - if (!headers['content-type']) { - headers = { - 'content-type': 'application/binary' - }; - } - return new P(function(resolve) { // Parse POST data var bboy = new Busboy({ - headers: headers, + headers: req.headers, // Increase the form field size limit from the 1M default. limits: { fieldSize: 15 * 1024 * 1024 } }); diff --git a/lib/server.js b/lib/server.js index d923bb7b4..250837c48 100644 --- a/lib/server.js +++ b/lib/server.js @@ -84,20 +84,24 @@ function handleResponse (opts, req, resp, response) { uri: rBody.uri }; + if (!response.headers['content-type']) { + response.headers['content-type'] = 'application/problem+json'; + } + if (response.status === 404) { if (!body.type) { body.type = 'not_found'; } if (!body.title) { body.title = 'Not found.'; } - if (!response.headers['content-type']) { - response.headers['content-type'] = 'application/problem+json'; - } } + if (response.status >= 400) { if (!body.uri) { body.uri = req.uri; } if (!body.method) { body.method = req.method; } } + if (!body.type) { body.type = 'unknown_error'; } + // Prefix error base URL // XXX: make the prefix configurable body.type = 'https://restbase.org/errors/' + body.type; @@ -168,8 +172,6 @@ function handleResponse (opts, req, resp, response) { // Handle a single request function handleRequest (opts, req, resp) { - var newReq; - // set the request ID early on for external requests req.headers = req.headers || {}; req.headers['x-request-id'] = req.headers['x-request-id'] || rbUtil.generateRequestId(); @@ -189,14 +191,30 @@ function handleRequest (opts, req, resp) { }; reqOpts.log = reqOpts.logger.log.bind(reqOpts.logger); + // Create a new, clean request object + var urlData = rbUtil.parseURL(req.url); + + var newReq = { + uri: new URI(urlData.pathname), + query: urlData.query, + method: req.method.toLowerCase(), + headers: req.headers, + }; // Start off by parsing any POST data with BusBoy return rbUtil.parsePOST(req) + .catch(function(e) { + throw new rbUtil.HTTPError({ + status: 400, + body: { + type: 'invalid_request' + } + }); + }) + // Then process the request .then(function(body) { - // Create a new, clean request object - var urlData = rbUtil.parseURL(req.url); if (/^application\/json/i.test(req.headers['content-type'])) { try { @@ -205,13 +223,8 @@ function handleRequest (opts, req, resp) { reqOpts.log('error/request/json-parsing', e); } } - newReq = { - uri: new URI(urlData.pathname), - query: urlData.query, - method: req.method.toLowerCase(), - headers: req.headers, - body: body - }; + + newReq.body = body; // Quick hack to set up general CORS if (newReq.method === 'options') { diff --git a/test/features/errors/invalid_request.js b/test/features/errors/invalid_request.js new file mode 100644 index 000000000..6b6c9ae46 --- /dev/null +++ b/test/features/errors/invalid_request.js @@ -0,0 +1,41 @@ +'use strict'; + +// mocha defines to avoid JSHint breakage +/* global describe, it, before, beforeEach, after, afterEach */ + +var assert = require('../../utils/assert.js'); +var preq = require('preq'); +var server = require('../../utils/server.js'); + +describe('400 handling', function() { + this.timeout(20000); + + before(function () { return server.start(); }); + + it('should return a proper 400 for an empty POST', function() { + return preq.post({ + uri: server.config.hostPort, + headers: { + 'content-type': 'foo/bar' + }, + }) + .catch(function(e) { + assert.deepEqual(e.status, 400); + assert.contentType(e, 'application/problem+json'); + }); + }); + + it('should return a proper 400 for an invalid POST', function() { + return preq.post({ + uri: server.config.hostPort, + headers: { + 'content-type': 'foo/bar' + }, + body: 'baz' + }) + .catch(function(e) { + assert.deepEqual(e.status, 400); + assert.contentType(e, 'application/problem+json'); + }); + }); +});