From 9bb51aff1b305c3ce5bd38103f52ae2154125dfd Mon Sep 17 00:00:00 2001 From: domenic Date: Wed, 25 Apr 2012 16:17:45 -0400 Subject: [PATCH 01/42] Adding `restify.realizeUrl` for replacing URL params with values from a hash. Example: `restify.realizeUrl('/foo/:bar/:baz', {bar: "BAR", baz: "BAZ"}) === '/foo/BAR/BAZ'` In the process factored out `sanitizePath` method from request.js into a new utils.js module. --- lib/index.js | 16 ++++++++++++++++ lib/request.js | 21 +-------------------- lib/utils.js | 22 ++++++++++++++++++++++ test/index.test.js | 23 +++++++++++++++++++++++ 4 files changed, 62 insertions(+), 20 deletions(-) create mode 100644 lib/utils.js create mode 100644 test/index.test.js diff --git a/lib/index.js b/lib/index.js index 6f1ef2d94..c89b62155 100644 --- a/lib/index.js +++ b/lib/index.js @@ -10,6 +10,7 @@ var clients = require('./clients'); var errors = require('./errors'); var plugins = require('./plugins'); +var sanitizePath = require('./utils').sanitizePath; ///--- Globals @@ -177,6 +178,21 @@ module.exports = { }, + /** + * Returns a string representation of a URL pattern , with its parameters + * filled in by the passed hash. + * + * If a key is not found in the hash for a param, it is left alone. + * + * @param {Object} a hash of parameter names to values for substitution. + */ + realizeUrl: function realizeUrl(pattern, params) { + return sanitizePath(pattern.replace(/\/:([^/]+)/g, function (wholeMatch, key) { + return params.hasOwnProperty(key) ? '/' + params[key] : wholeMatch; + })); + }, + + HttpClient: HttpClient, JsonClient: JsonClient, StringClient: StringClient diff --git a/lib/request.js b/lib/request.js index 1ea904479..103e508d8 100644 --- a/lib/request.js +++ b/lib/request.js @@ -9,6 +9,7 @@ var mime = require('mime'); var qs = require('qs'); var uuid = require('node-uuid'); +var sanitizePath = require('./utils').sanitizePath; ///--- Globals @@ -19,26 +20,6 @@ var Request = http.IncomingMessage; ///--- Helpers -/** - * Cleans up sloppy URL paths, like /foo////bar/// to /foo/bar. - * - * @param {String} path the HTTP resource path. - * @return {String} Cleaned up form of path. - */ -function sanitizePath(path) { - assert.ok(path); - - // Be nice like apache and strip out any //my//foo//bar///blah - path = path.replace(/\/\/+/g, '/'); - - // Kill a trailing '/' - if (path.lastIndexOf('/') === (path.length - 1) && path.length > 1) - path = path.substr(0, path.length - 1); - - return path; -} - - // The following three functions are courtesy of expressjs // as is req.accepts(), and req.is() below. // diff --git a/lib/utils.js b/lib/utils.js new file mode 100644 index 000000000..935891197 --- /dev/null +++ b/lib/utils.js @@ -0,0 +1,22 @@ +// Copyright 2012 Mark Cavage, Inc. All rights reserved. + +var assert = require('assert'); + +/** + * Cleans up sloppy URL paths, like /foo////bar/// to /foo/bar. + * + * @param {String} path the HTTP resource path. + * @return {String} Cleaned up form of path. + */ +exports.sanitizePath = function sanitizePath(path) { + assert.ok(path); + + // Be nice like apache and strip out any //my//foo//bar///blah + path = path.replace(/\/\/+/g, '/'); + + // Kill a trailing '/' + if (path.lastIndexOf('/') === (path.length - 1) && path.length > 1) + path = path.substr(0, path.length - 1); + + return path; +}; diff --git a/test/index.test.js b/test/index.test.js new file mode 100644 index 000000000..e0ba1cd6c --- /dev/null +++ b/test/index.test.js @@ -0,0 +1,23 @@ +// Copyright 2012 Mark Cavage, Inc. All rights reserved. + +var test = require('tap').test; + + +var restify = require('../lib/index'); + + + +///--- Tests + +test('realize', function (t) { + var pattern = '/foo/:bar/:baz'; + + t.equal(restify.realizeUrl(pattern, {}), '/foo/:bar/:baz'); + t.equal(restify.realizeUrl(pattern, {bar: 'BAR'}), '/foo/BAR/:baz'); + t.equal(restify.realizeUrl(pattern, {bar: 'BAR', baz: 'BAZ'}), '/foo/BAR/BAZ'); + t.equal(restify.realizeUrl(pattern, {bar: 'BAR', baz: 'BAZ', quux: 'QUUX'}), '/foo/BAR/BAZ'); + + t.equal(restify.realizeUrl('/foo////bar///:baz', {baz: 'BAZ'}), '/foo/bar/BAZ'); + + t.end(); +}); From daa1047a0f7d76f95ee3eb65d2578025aa8131dd Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Sat, 5 May 2012 20:45:48 +0100 Subject: [PATCH 02/42] Fix defaultResponseHeaders setter --- lib/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/index.js b/lib/index.js index c89b62155..67d5a4bab 100644 --- a/lib/index.js +++ b/lib/index.js @@ -216,5 +216,5 @@ module.exports.__defineSetter__('defaultResponseHeaders', function (f) { throw new TypeError('defaultResponseHeaders must be a function'); } - http.ServerResponse.prototype.defaultHeaders = f; + http.ServerResponse.prototype.defaultResponseHeaders = f; }); From ea84cc32c2bbec21db33ca3c5492fcb53cab4743 Mon Sep 17 00:00:00 2001 From: Dave Pacheco Date: Fri, 11 May 2012 15:50:27 -0700 Subject: [PATCH 03/42] workaround joyent/node#3257 --- lib/clients/string_client.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/clients/string_client.js b/lib/clients/string_client.js index 4d15e11f5..164a3f4f0 100644 --- a/lib/clients/string_client.js +++ b/lib/clients/string_client.js @@ -98,10 +98,11 @@ StringClient.prototype.write = function write(options, body, callback) { if (body) { self.log.trace('sending body -> %s', body); - req.write(body); + req.end(body); + } else { + req.end(); } - req.end(); return req.once('result', self.parse(req, callback)); }); }; From 002461c6da0fd23272cd8a5e037023c2a5917481 Mon Sep 17 00:00:00 2001 From: Dave Pacheco Date: Fri, 11 May 2012 16:53:40 -0700 Subject: [PATCH 04/42] fix json_body_parser comment --- lib/plugins/json_body_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/json_body_parser.js b/lib/plugins/json_body_parser.js index 590b1e403..b331e4db5 100644 --- a/lib/plugins/json_body_parser.js +++ b/lib/plugins/json_body_parser.js @@ -17,7 +17,7 @@ var InvalidContentError = errors.InvalidContentError; /** * Returns a plugin that will parse the HTTP request body IFF the - * contentType is application/x-www-form-urlencoded. + * contentType is application/json. * * If req.params already contains a given key, that key is skipped and an * error is logged. From 8fff0076f6db6c7e4a29bb161188c79b3e82e960 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Mon, 14 May 2012 09:21:37 -0700 Subject: [PATCH 05/42] test for GH-141 --- test/server.test.js | 47 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/server.test.js b/test/server.test.js index 6f13b7f90..72e1da3d7 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -949,6 +949,53 @@ test('GH-109 RegExp flags not honored', function (t) { }); +test('GH-141 return next(err) not working', function (t) { + var server = restify.createServer({ dtrace: DTRACE, log: LOGGER }); + server.use(restify.authorizationParser()); + server.use(function authenticate(req, res, next) { + if (req.username !== 'admin' || + !req.authorization.basic || + req.authorization.basic.password !== 'admin') { + return next(new restify.NotAuthorizedError('invalid credentials')); + } + return next(); + }); + + server.get('/', function (req, res, next) { + res.send(200, req.username); + return next(); + }); + + server.listen(PORT, function () { + var opts = { + hostname: 'localhost', + port: PORT, + path: '/', + method: 'GET', + agent: false, + headers: { + accept: 'text/plain', + authorization: 'Basic ' + new Buffer('admin:foo').toString('base64') + } + }; + http.request(opts, function (res) { + t.equal(res.statusCode, 403); + var body = ''; + res.setEncoding('utf8'); + res.on('data', function (chunk) { + body += chunk; + }); + res.on('end', function () { + t.equal(body, 'invalid credentials'); + server.close(function () { + t.end(); + }); + }); + }).end(); + }); +}); + + // // Disabled, as Heroku (travis) doesn't allow us to write to /tmp // From 08468a05205f2e806e55ec974becb8c91db49424 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Mon, 14 May 2012 13:30:21 -0700 Subject: [PATCH 06/42] lint cleanup --- CHANGES.md | 3 +++ lib/index.js | 18 +++++++++--------- lib/response.js | 2 +- test/index.test.js | 17 +++++++++++------ 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index cee98ded0..f7c0414a3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,9 @@ ## 1.4.2 (not yet released) +- Add Route.realize( Domenic Denicola) +- defaultResponseHeaders setter was setting the wrong method (Harry Marr) +- Workaround joyent/node#3257 (Dave Pacheco) - logging typo (Pedro Candel) - response `beforeSend` event (Paul Bouzakis) diff --git a/lib/index.js b/lib/index.js index 67d5a4bab..1564d6b8b 100644 --- a/lib/index.js +++ b/lib/index.js @@ -179,16 +179,16 @@ module.exports = { /** - * Returns a string representation of a URL pattern , with its parameters - * filled in by the passed hash. - * - * If a key is not found in the hash for a param, it is left alone. - * - * @param {Object} a hash of parameter names to values for substitution. - */ + * Returns a string representation of a URL pattern , with its parameters + * filled in by the passed hash. + * + * If a key is not found in the hash for a param, it is left alone. + * + * @param {Object} a hash of parameter names to values for substitution. + */ realizeUrl: function realizeUrl(pattern, params) { - return sanitizePath(pattern.replace(/\/:([^/]+)/g, function (wholeMatch, key) { - return params.hasOwnProperty(key) ? '/' + params[key] : wholeMatch; + return sanitizePath(pattern.replace(/\/:([^/]+)/g, function (match, key) { + return params.hasOwnProperty(key) ? '/' + params[key] : match; })); }, diff --git a/lib/response.js b/lib/response.js index 3ed8b7e46..6533c860f 100644 --- a/lib/response.js +++ b/lib/response.js @@ -398,7 +398,7 @@ Response.prototype.send = function send(body) { // serialization var data = body ? this.format(body) : null; this.emit('beforeSend', data); - + this.defaultResponseHeaders(data); this.writeHead(this.statusCode, this.headers); if (data && !head && this.statusCode !== 204 && this.statusCode !== 304) { diff --git a/test/index.test.js b/test/index.test.js index e0ba1cd6c..1dc925ee5 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -2,22 +2,27 @@ var test = require('tap').test; - var restify = require('../lib/index'); +///--- Globals + +var realizeUrl = restify.realizeUrl; + + ///--- Tests test('realize', function (t) { var pattern = '/foo/:bar/:baz'; - t.equal(restify.realizeUrl(pattern, {}), '/foo/:bar/:baz'); - t.equal(restify.realizeUrl(pattern, {bar: 'BAR'}), '/foo/BAR/:baz'); - t.equal(restify.realizeUrl(pattern, {bar: 'BAR', baz: 'BAZ'}), '/foo/BAR/BAZ'); - t.equal(restify.realizeUrl(pattern, {bar: 'BAR', baz: 'BAZ', quux: 'QUUX'}), '/foo/BAR/BAZ'); + t.equal(realizeUrl(pattern, {}), '/foo/:bar/:baz'); + t.equal(realizeUrl(pattern, {bar: 'BAR'}), '/foo/BAR/:baz'); + t.equal(realizeUrl(pattern, {bar: 'BAR', baz: 'BAZ'}), '/foo/BAR/BAZ'); + t.equal(realizeUrl(pattern, {bar: 'BAR', baz: 'BAZ', quux: 'QUUX'}), + '/foo/BAR/BAZ'); - t.equal(restify.realizeUrl('/foo////bar///:baz', {baz: 'BAZ'}), '/foo/bar/BAZ'); + t.equal(realizeUrl('/foo////bar///:baz', {baz: 'BAZ'}), '/foo/bar/BAZ'); t.end(); }); From 44f3969b9b0992a1562cb75fe1593a5a44f7072f Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Mon, 14 May 2012 13:30:41 -0700 Subject: [PATCH 07/42] version bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7d16a0fe5..82e01e657 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "name": "restify", "homepage": "http://mcavage.github.com/node-restify", "description": "REST framework", - "version": "1.4.1", + "version": "1.4.2", "repository": { "type": "git", "url": "git://github.com/mcavage/node-restify.git" From d493a63fae47f37d8cfb844abf99c575354f1d01 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Mon, 14 May 2012 13:32:32 -0700 Subject: [PATCH 08/42] prep for future dev --- CHANGES.md | 4 +++- package.json | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f7c0414a3..2abec58eb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,8 @@ # restify Changelog -## 1.4.2 (not yet released) +## 1.4.3 (not yet released) + +## 1.4.2 - Add Route.realize( Domenic Denicola) - defaultResponseHeaders setter was setting the wrong method (Harry Marr) diff --git a/package.json b/package.json index 82e01e657..7d828bf6d 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "name": "restify", "homepage": "http://mcavage.github.com/node-restify", "description": "REST framework", - "version": "1.4.2", + "version": "1.4.3", "repository": { "type": "git", "url": "git://github.com/mcavage/node-restify.git" From 0d421c3ae6f24745227684c0d72055b0c0bcbd46 Mon Sep 17 00:00:00 2001 From: Paul Bouzakis Date: Thu, 24 May 2012 12:00:05 -0700 Subject: [PATCH 09/42] Add support for authority certificates --- lib/server.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/server.js b/lib/server.js index 0002b25ba..90e0ae115 100644 --- a/lib/server.js +++ b/lib/server.js @@ -135,10 +135,11 @@ function Server(options) { if (options.certificate && options.key) { secure = true; - this.server = https.createServer({ - cert: options.certificate, - key: options.key - }); + var httpsOptions = { cert: options.certificate, key: options.key }; + if (options.ca) + httpsOptions.ca = options.ca; + + this.server = https.createServer(httpsOptions); } else { this.server = http.createServer(); } From c7c7d4e33e19f5541d8e1e7ec5a06ba6cfb316ec Mon Sep 17 00:00:00 2001 From: Simon Sturmer Date: Mon, 28 May 2012 22:30:43 +1000 Subject: [PATCH 10/42] Add http error 415 Unsupported Media Type --- lib/plugins/body_parser.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/plugins/body_parser.js b/lib/plugins/body_parser.js index f65e08326..8340cd127 100644 --- a/lib/plugins/body_parser.js +++ b/lib/plugins/body_parser.js @@ -4,6 +4,10 @@ var jsonParser = require('./json_body_parser'); var formParser = require('./form_body_parser'); var multipartParser = require('./multipart_parser'); +var errors = require('../errors'); + +var UnsupportedMediaTypeError = errors.UnsupportedMediaTypeError; + function bodyParser(options) { @@ -21,6 +25,8 @@ function bodyParser(options) { return parseForm(req, res, next); } else if (req.contentType === 'multipart/form-data') { return parseMultipart(req, res, next); + } else { + return next(new UnsupportedMediaTypeError('Unsupported Content-Type: ' + req.contentType)); } return next(); From 289773c88cd881b3c39a52b810cb636ad70bdabf Mon Sep 17 00:00:00 2001 From: Simon Sturmer Date: Fri, 1 Jun 2012 20:37:04 +1000 Subject: [PATCH 11/42] add request body length validation to form_body_parser; add test --- lib/plugins/form_body_parser.js | 11 +++++++-- test/server.test.js | 43 +++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/lib/plugins/form_body_parser.js b/lib/plugins/form_body_parser.js index d9a8d48ba..5d3e0de1b 100644 --- a/lib/plugins/form_body_parser.js +++ b/lib/plugins/form_body_parser.js @@ -11,6 +11,7 @@ var errors = require('../errors'); var BadDigestError = errors.BadDigestError; var InvalidContentError = errors.InvalidContentError; +var RequestEntityTooLargeError = errors.RequestEntityTooLargeError; @@ -41,10 +42,13 @@ function urlEncodedBodyParser(options) { if (req.header('content-md5')) hash = crypto.createHash('md5'); + var bytesReceived = 0, maxBodySize = options.maxBodySize || 0; req.body = ''; - req.setEncoding('utf8'); req.on('data', function (chunk) { - req.body += chunk; + bytesReceived += chunk.length; + if (maxBodySize && bytesReceived > maxBodySize) + return; + req.body += chunk.toString('utf8'); if (hash) hash.update(chunk); }); @@ -52,6 +56,9 @@ function urlEncodedBodyParser(options) { return next(err); }); req.on('end', function () { + if (maxBodySize && bytesReceived > maxBodySize) + return next(new RequestEntityTooLargeError('Request body size exceeds ' + maxBodySize)); + if (!req.body) return next(); diff --git a/test/server.test.js b/test/server.test.js index 72e1da3d7..0d330db1f 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -1036,3 +1036,46 @@ test('GH-141 return next(err) not working', function (t) { // }); // + + +test('GH-149 limit request body size', function (t) { + var server = restify.createServer(); + server.use(restify.bodyParser({maxBodySize: 1024})); + + server.post('/', function (req, res, next) { + res.send(200, {length: req.body.length}); + return next(); + }); + + server.listen(PORT, function () { + var opts = { + hostname: 'localhost', + port: PORT, + path: '/', + method: 'POST', + agent: false, + headers: { + 'accept': 'application/json', + 'content-type': 'application/x-www-form-urlencoded', + 'transfer-encoding': 'chunked' + } + }; + var req = http.request(opts, function (res) { + t.equal(res.statusCode, 413); + var body = ''; + res.setEncoding('utf8'); + res.on('data', function (chunk) { + body += chunk; + }); + res.on('end', function () { + //throw new Error(body); + //t.equal(body, 'invalid credentials'); + server.close(function () { + t.end(); + }); + }); + }); + req.write(new Array(1028).join('x')); + req.end(); + }); +}); From 5b0d7efab73e09fe8b38cc8fc37d9f62b4142cc7 Mon Sep 17 00:00:00 2001 From: Simon Sturmer Date: Fri, 1 Jun 2012 20:57:15 +1000 Subject: [PATCH 12/42] only parse request body for POST and PUT --- lib/plugins/body_parser.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/plugins/body_parser.js b/lib/plugins/body_parser.js index 8340cd127..79a84c6d6 100644 --- a/lib/plugins/body_parser.js +++ b/lib/plugins/body_parser.js @@ -16,6 +16,9 @@ function bodyParser(options) { var parseMultipart = multipartParser(options); return function parseBody(req, res, next) { + if (req.method !== 'POST' && req.method !== 'PUT') + return next(); + if (req.contentLength === 0 && !req.chunked) return next(); From de903ce1f9f85b5a913472f3f11828dbfc9ec712 Mon Sep 17 00:00:00 2001 From: Simon Sturmer Date: Fri, 1 Jun 2012 21:21:58 +1000 Subject: [PATCH 13/42] add req body length validation to json_body_parser; add test --- lib/plugins/body_parser.js | 3 +++ lib/plugins/json_body_parser.js | 11 ++++++-- test/server.test.js | 47 ++++++++++++++++++++++++++++++--- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/lib/plugins/body_parser.js b/lib/plugins/body_parser.js index 8340cd127..79a84c6d6 100644 --- a/lib/plugins/body_parser.js +++ b/lib/plugins/body_parser.js @@ -16,6 +16,9 @@ function bodyParser(options) { var parseMultipart = multipartParser(options); return function parseBody(req, res, next) { + if (req.method !== 'POST' && req.method !== 'PUT') + return next(); + if (req.contentLength === 0 && !req.chunked) return next(); diff --git a/lib/plugins/json_body_parser.js b/lib/plugins/json_body_parser.js index b331e4db5..f2717cd71 100644 --- a/lib/plugins/json_body_parser.js +++ b/lib/plugins/json_body_parser.js @@ -10,6 +10,7 @@ var errors = require('../errors'); var BadDigestError = errors.BadDigestError; var InvalidContentError = errors.InvalidContentError; +var RequestEntityTooLargeError = errors.RequestEntityTooLargeError; @@ -40,10 +41,13 @@ function jsonBodyParser(options) { if (req.header('content-md5')) hash = crypto.createHash('md5'); + var bytesReceived = 0, maxBodySize = options.maxBodySize || 0; req.body = ''; - req.setEncoding('utf8'); req.on('data', function (chunk) { - req.body += chunk; + bytesReceived += chunk.length; + if (maxBodySize && bytesReceived > maxBodySize) + return; + req.body += chunk.toString('utf8'); if (hash) hash.update(chunk); }); @@ -51,6 +55,9 @@ function jsonBodyParser(options) { return next(err); }); req.on('end', function () { + if (maxBodySize && bytesReceived > maxBodySize) + return next(new RequestEntityTooLargeError('Request body size exceeds ' + maxBodySize)); + if (!req.body) return next(); diff --git a/test/server.test.js b/test/server.test.js index 0d330db1f..ac688f0a7 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -1038,7 +1038,7 @@ test('GH-141 return next(err) not working', function (t) { // -test('GH-149 limit request body size', function (t) { +test('GH-149 limit request body size (form)', function (t) { var server = restify.createServer(); server.use(restify.bodyParser({maxBodySize: 1024})); @@ -1068,14 +1068,53 @@ test('GH-149 limit request body size', function (t) { body += chunk; }); res.on('end', function () { - //throw new Error(body); - //t.equal(body, 'invalid credentials'); server.close(function () { t.end(); }); }); }); - req.write(new Array(1028).join('x')); + req.write(new Array(1026).join('x')); + req.end(); + }); +}); + + +test('GH-149 limit request body size (json)', function (t) { + var server = restify.createServer(); + server.use(restify.bodyParser({maxBodySize: 1024})); + + server.post('/', function (req, res, next) { + res.send(200, {length: req.body.length}); + return next(); + }); + + server.listen(PORT, function () { + var opts = { + hostname: 'localhost', + port: PORT, + path: '/', + method: 'POST', + agent: false, + headers: { + 'accept': 'application/json', + 'content-type': 'application/json', + 'transfer-encoding': 'chunked' + } + }; + var req = http.request(opts, function (res) { + t.equal(res.statusCode, 413); + var body = ''; + res.setEncoding('utf8'); + res.on('data', function (chunk) { + body += chunk; + }); + res.on('end', function () { + server.close(function () { + t.end(); + }); + }); + }); + req.write('{"a":[' + new Array(512).join('1,') + '0]}'); req.end(); }); }); From 93cb90bce5b01d7aa575c4718956315320938848 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Sat, 2 Jun 2012 01:38:38 +0000 Subject: [PATCH 14/42] lint/style cleanup from pull requests --- lib/plugins/body_parser.js | 3 ++- lib/plugins/form_body_parser.js | 3 ++- lib/plugins/json_body_parser.js | 3 ++- test/server.test.js | 10 ---------- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/lib/plugins/body_parser.js b/lib/plugins/body_parser.js index 79a84c6d6..35e0638ac 100644 --- a/lib/plugins/body_parser.js +++ b/lib/plugins/body_parser.js @@ -29,7 +29,8 @@ function bodyParser(options) { } else if (req.contentType === 'multipart/form-data') { return parseMultipart(req, res, next); } else { - return next(new UnsupportedMediaTypeError('Unsupported Content-Type: ' + req.contentType)); + return next(new UnsupportedMediaTypeError('Unsupported Content-Type: ' + + req.contentType)); } return next(); diff --git a/lib/plugins/form_body_parser.js b/lib/plugins/form_body_parser.js index 5d3e0de1b..f7a50a00b 100644 --- a/lib/plugins/form_body_parser.js +++ b/lib/plugins/form_body_parser.js @@ -57,7 +57,8 @@ function urlEncodedBodyParser(options) { }); req.on('end', function () { if (maxBodySize && bytesReceived > maxBodySize) - return next(new RequestEntityTooLargeError('Request body size exceeds ' + maxBodySize)); + return next(new RequestEntityTooLargeError('Request body size exceeds ' + + maxBodySize)); if (!req.body) return next(); diff --git a/lib/plugins/json_body_parser.js b/lib/plugins/json_body_parser.js index f2717cd71..70c883993 100644 --- a/lib/plugins/json_body_parser.js +++ b/lib/plugins/json_body_parser.js @@ -56,7 +56,8 @@ function jsonBodyParser(options) { }); req.on('end', function () { if (maxBodySize && bytesReceived > maxBodySize) - return next(new RequestEntityTooLargeError('Request body size exceeds ' + maxBodySize)); + return next(new RequestEntityTooLargeError('Request body size exceeds ' + + maxBodySize)); if (!req.body) return next(); diff --git a/test/server.test.js b/test/server.test.js index ac688f0a7..9623a7432 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -1062,11 +1062,6 @@ test('GH-149 limit request body size (form)', function (t) { }; var req = http.request(opts, function (res) { t.equal(res.statusCode, 413); - var body = ''; - res.setEncoding('utf8'); - res.on('data', function (chunk) { - body += chunk; - }); res.on('end', function () { server.close(function () { t.end(); @@ -1103,11 +1098,6 @@ test('GH-149 limit request body size (json)', function (t) { }; var req = http.request(opts, function (res) { t.equal(res.statusCode, 413); - var body = ''; - res.setEncoding('utf8'); - res.on('data', function (chunk) { - body += chunk; - }); res.on('end', function () { server.close(function () { t.end(); From 8a901e8182ecd427a1ca930fba3d0911ead905a2 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Sat, 2 Jun 2012 01:43:37 +0000 Subject: [PATCH 15/42] GH-146 Allow setting of regex flags even if path is a string --- lib/route.js | 2 +- lib/server.js | 1 + test/server.test.js | 26 ++++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/route.js b/lib/route.js index 115f34f20..cfd76af99 100644 --- a/lib/route.js +++ b/lib/route.js @@ -176,7 +176,7 @@ function Route(options) { self._url = url.parse(u).pathname; self.pattern = '^'; - self.flags = ''; + self.flags = options.flags || ''; self.params = []; self._url.split('/').forEach(function (fragment) { if (!fragment.length) diff --git a/lib/server.js b/lib/server.js index 90e0ae115..4a164e0c3 100644 --- a/lib/server.js +++ b/lib/server.js @@ -418,6 +418,7 @@ Server.prototype._addRoute = function _addRoute(method, options, handlers) { log: self.log, method: method, url: options.path || options.url, + flags: options.flags, handlers: chain, name: options.name, version: options.version || self.version, diff --git a/test/server.test.js b/test/server.test.js index 9623a7432..ffb141157 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -527,6 +527,32 @@ test('RegExp ok', function (t) { }); +test('path+flags ok', function (t) { + var server = restify.createServer({ dtrace: DTRACE, log: LOGGER }); + + server.get({path: '/foo', flags: 'i'}, function tester(req, res, next) { + res.send('hi there'); + return next(); + }); + + server.listen(PORT, function () { + var opts = { + hostname: 'localhost', + port: PORT, + path: '/FOO', + method: 'GET', + agent: false + }; + http.request(opts, function (res) { + t.equal(res.statusCode, 200); + server.close(function () { + t.end(); + }); + }).end(); + }); +}); + + test('GH-56 streaming with filed (download)', function (t) { var server = restify.createServer({ dtrace: DTRACE, log: LOGGER }); From 8541e8fcf2fb8fc5da0be00296ebe36eb703317f Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Sat, 2 Jun 2012 01:47:40 +0000 Subject: [PATCH 16/42] update changelog --- CHANGES.md | 6 ++++++ package.json | 1 + 2 files changed, 7 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 2abec58eb..23791501e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,12 @@ ## 1.4.3 (not yet released) +- GH-149 allow setting of max body size (and return 413) (Simon Sturmer) +- GH-146 allow setting of route regex flags when path is not a RegExp +- Support SSL CAs (Paul Bouzakis) +- body parser should return 415 when content-type not known (Simon Sturmer) + + ## 1.4.2 - Add Route.realize( Domenic Denicola) diff --git a/package.json b/package.json index 7d828bf6d..be1b06db0 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "Isaac Schlueter", "Andrew Sliwinski", "Matt Smillie", + "Simon Sturmer", "Diego Torres", "Mike Williams" ], From 6577369e6af2db5d49ebf053abd7744c79210031 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Mon, 4 Jun 2012 13:15:22 -0300 Subject: [PATCH 17/42] Send JSON by default for `HttpError`s. --- lib/errors/http_error.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/errors/http_error.js b/lib/errors/http_error.js index 99d906429..fc2e010dd 100644 --- a/lib/errors/http_error.js +++ b/lib/errors/http_error.js @@ -53,7 +53,7 @@ function HttpError(code, message, body, constructorOpt) { code = parseInt(code, 10); this.message = message || ''; - this.body = body || message || ''; + this.body = body || (message ? { message: message } : ''); this.statusCode = this.httpCode = code; } util.inherits(HttpError, Error); From af820e668cdfa0f04b1bc92412a3b16f628533cb Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Mon, 4 Jun 2012 13:37:39 -0300 Subject: [PATCH 18/42] Add `rejectUnknown` option to body parser. Defaults to `true`. Makes the behavior introduced in c7c7d4e33e19f5541d8e1e7ec5a06ba6cfb316ec optional, by explicitly setting `false`. --- lib/plugins/body_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/body_parser.js b/lib/plugins/body_parser.js index 35e0638ac..25e4622d6 100644 --- a/lib/plugins/body_parser.js +++ b/lib/plugins/body_parser.js @@ -28,7 +28,7 @@ function bodyParser(options) { return parseForm(req, res, next); } else if (req.contentType === 'multipart/form-data') { return parseMultipart(req, res, next); - } else { + } else if (options.rejectUnknown !== false) { return next(new UnsupportedMediaTypeError('Unsupported Content-Type: ' + req.contentType)); } From 691f9ecf58861fec54d299d3d1f9b14d123f0d14 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Mon, 4 Jun 2012 15:28:43 -0300 Subject: [PATCH 19/42] Make bodyParser play well with PATCH too. --- lib/plugins/body_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/body_parser.js b/lib/plugins/body_parser.js index 25e4622d6..735389f9d 100644 --- a/lib/plugins/body_parser.js +++ b/lib/plugins/body_parser.js @@ -16,7 +16,7 @@ function bodyParser(options) { var parseMultipart = multipartParser(options); return function parseBody(req, res, next) { - if (req.method !== 'POST' && req.method !== 'PUT') + if (req.method !== 'POST' && req.method !== 'PUT' && req.method !== 'PATCH') return next(); if (req.contentLength === 0 && !req.chunked) From 62a067cdc91f16f1d3792a62a302745bb645f589 Mon Sep 17 00:00:00 2001 From: Dan Tamas Date: Wed, 13 Jun 2012 18:27:35 +0200 Subject: [PATCH 20/42] Fix for https://github.com/mcavage/node-restify/issues/158 charSet does not gets set. --- lib/response.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/response.js b/lib/response.js index 6533c860f..25eaf8071 100644 --- a/lib/response.js +++ b/lib/response.js @@ -344,13 +344,14 @@ Response.prototype.defaultResponseHeaders = function defaultHeaders(data) { this.setHeader('Content-MD5', hash.digest('base64')); } - if (!this.header('Content-Type') && this.contentType) { - var type = this.contentType; + if (this.header('Content-Type') || this.contentType) { + var type = this.header('Content-Type') || this.contentType; if (this.charSet) type += '; charset=' + this.charSet; this.setHeader('Content-Type', type); } + var now = new Date(); if (!this.getHeader('Date')) this.setHeader('Date', httpDate(now)); From a3d8dad77e18d342b9e629f221fc113683902170 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Fri, 15 Jun 2012 04:27:46 +0000 Subject: [PATCH 21/42] 1.4.3 final --- CHANGES.md | 9 ++++++++- lib/response.js | 2 +- package.json | 17 +++++++++-------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 23791501e..598621f4b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,7 +1,14 @@ # restify Changelog -## 1.4.3 (not yet released) +## 1.4.4 (not yet released) +## 1.4.3 + +- update dependencies to latest (notably dtrace-provider) +- GH-158 res.charSet broken (Tamas Daniel) +- GH-154 bodyParser work with PATCH (Domenic Denicola) +- GH-153 bodyParser can reject or allow unknown content-types (Domenic Denicola) +- GH-152 Send JSON on HttpError (Domenic Denicola) - GH-149 allow setting of max body size (and return 413) (Simon Sturmer) - GH-146 allow setting of route regex flags when path is not a RegExp - Support SSL CAs (Paul Bouzakis) diff --git a/lib/response.js b/lib/response.js index 25eaf8071..8311635ab 100644 --- a/lib/response.js +++ b/lib/response.js @@ -344,7 +344,7 @@ Response.prototype.defaultResponseHeaders = function defaultHeaders(data) { this.setHeader('Content-MD5', hash.digest('base64')); } - if (this.header('Content-Type') || this.contentType) { + if (this.header('Content-Type') || this.contentType) { var type = this.header('Content-Type') || this.contentType; if (this.charSet) type += '; charset=' + this.charSet; diff --git a/package.json b/package.json index be1b06db0..1de9fd2db 100644 --- a/package.json +++ b/package.json @@ -2,6 +2,7 @@ "author": "Mark Cavage ", "contributors": [ "Dominic Barnes", + "Tamas Daniel", "Domenic Denicola", "Paul Bouzakis", "Shaun Berryman", @@ -35,22 +36,22 @@ "node": ">=0.6" }, "dependencies": { - "async": "0.1.18", - "bunyan": "0.6.8", + "async": "0.1.22", + "bunyan": "0.8.0", "byline": "2.0.2", - "formidable": "1.0.9", - "dtrace-provider": "0.0.6", + "formidable": "1.0.11", + "dtrace-provider": "0.0.8", "http-signature": "0.9.9", - "lru-cache": "1.0.5", + "lru-cache": "1.1.0", "mime": "1.2.5", "node-uuid": "1.3.3", - "qs": "0.4.2", + "qs": "0.5.0", "retry": "0.6.0", - "semver": "1.0.13" + "semver": "1.0.14" }, "devDependencies": { "filed": "0.0.6", - "tap": "0.2.4" + "tap": "0.2.5" }, "optionalDependencies": { "dtrace-provider": "0.0.6" From dec58eb365577bb928e5402997ebeca620736b95 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Mon, 18 Jun 2012 19:10:03 +0000 Subject: [PATCH 22/42] GH-160 use "bin": "./bin/report-latency" instead of "directories": { "bin": ... } --- CHANGES.md | 2 ++ package.json | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 598621f4b..2ce6f3cfc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,8 @@ ## 1.4.4 (not yet released) +- GH-160 don't rely on npm's directories: "bin". + ## 1.4.3 - update dependencies to latest (notably dtrace-provider) diff --git a/package.json b/package.json index 1de9fd2db..e11e94315 100644 --- a/package.json +++ b/package.json @@ -28,9 +28,8 @@ "url": "git://github.com/mcavage/node-restify.git" }, "main": "lib/index.js", - "directories": { - "bin": "./bin", - "lib": "./lib" + "bin": { + "report-latency": "./bin/report-latency" }, "engines": { "node": ">=0.6" From 82ac2dbd58cc2c82d5cfdde5c1027263fa45cf64 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Thu, 21 Jun 2012 15:27:38 -0300 Subject: [PATCH 23/42] Don't default contentLength to zero. --- lib/request.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/request.js b/lib/request.js index 103e508d8..6eab0a3f9 100644 --- a/lib/request.js +++ b/lib/request.js @@ -88,7 +88,7 @@ module.exports = { var _url = url.parse(req.url); req.chunked = req.headers['transfer-encoding'] === 'chunked'; - req.contentLength = req.headers['content-length'] || 0; + req.contentLength = req.headers['content-length']; req.contentType = parseContentType(req); req.href = _url.href; req.id = req.headers['x-request-id'] || uuid(); From 54ea0a6c5f494b020de3a758477688d27d99cc4c Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Thu, 21 Jun 2012 20:46:37 -0300 Subject: [PATCH 24/42] Get rid of ETag quotes and W/ prefix for ETag header. It was only getting rid of them for If-Match/If-None-Match, which meant the comparison never evaluated to true unless the client sent malformed If-Match/If-None-Match headers (i.e. ones with double quotes). --- lib/plugins/conditional_request.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/plugins/conditional_request.js b/lib/plugins/conditional_request.js index d2087a4b2..ac302549c 100644 --- a/lib/plugins/conditional_request.js +++ b/lib/plugins/conditional_request.js @@ -34,6 +34,9 @@ function conditionalRequest() { var matched = false; if (typeof (etag) === 'string' && etag.length !== 0) { + etag = + etag.replace(/^W\//, '').replace(/^"(\w*)"$/, '$1'); + if (req.headers['if-match']) { // RFC: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.24 From 8d322981ff2c08e3de854f204a592fb4e82244f3 Mon Sep 17 00:00:00 2001 From: Petter Rasmussen Date: Thu, 28 Jun 2012 01:32:17 +0200 Subject: [PATCH 25/42] Parse content-length as integer --- lib/request.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/request.js b/lib/request.js index 6eab0a3f9..709b5360d 100644 --- a/lib/request.js +++ b/lib/request.js @@ -79,6 +79,10 @@ function parseKeepAlive(req) { return req.httpVersion === '1.0' ? false : true; } +function parseContentLength(req) { + var length = req.headers['content-length']; + return (length === undefined) ? null : parseInt(length, 10); +} ///--- API @@ -88,7 +92,7 @@ module.exports = { var _url = url.parse(req.url); req.chunked = req.headers['transfer-encoding'] === 'chunked'; - req.contentLength = req.headers['content-length']; + req.contentLength = parseContentLength(req); req.contentType = parseContentType(req); req.href = _url.href; req.id = req.headers['x-request-id'] || uuid(); From 85f32436c920101c8877462d11445ca44bc32832 Mon Sep 17 00:00:00 2001 From: Petter Rasmussen Date: Thu, 28 Jun 2012 01:41:01 +0200 Subject: [PATCH 26/42] Check that options is not empty --- lib/plugins/body_parser.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/body_parser.js b/lib/plugins/body_parser.js index 735389f9d..08f33daf7 100644 --- a/lib/plugins/body_parser.js +++ b/lib/plugins/body_parser.js @@ -28,7 +28,7 @@ function bodyParser(options) { return parseForm(req, res, next); } else if (req.contentType === 'multipart/form-data') { return parseMultipart(req, res, next); - } else if (options.rejectUnknown !== false) { + } else if (options && options.rejectUnknown !== false) { return next(new UnsupportedMediaTypeError('Unsupported Content-Type: ' + req.contentType)); } From 375e45a1aa21fb83d19ae7424bf9c1a26a5ca865 Mon Sep 17 00:00:00 2001 From: Iuri Aranda Date: Mon, 9 Jul 2012 15:44:18 +0200 Subject: [PATCH 27/42] Added option `keepExtensions` to bodyParser plugin The `keepExtensions` option is useful if you want the uploaded files to include the extensions of the original files. Default is `false`. --- docs/index.restdown | 7 ++++--- lib/plugins/multipart_parser.js | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/index.restdown b/docs/index.restdown index cde73be68..379c059ce 100644 --- a/docs/index.restdown +++ b/docs/index.restdown @@ -647,10 +647,11 @@ supported. server.use(restify.bodyParser({ mapParams: false })); -You can pass in an options object; currently the only parameter honored is -`mapParams`, which you can set to `false` to disable copying k/v pairs from +You can pass in an options object; currently the only two parameters honored are +`mapParams` and `keepExtensions`. You can set `mapParams` to `false` to disable copying k/v pairs from the request body into `req.params`; instead, `req.body` will be overwritten with -the parsed object. The default is to map params into `req.params`. +the parsed object. The default is to map params into `req.params`. The `keepExtensions` option is +useful if you want the uploaded files to include the extensions of the original files. Default is `false`. ### Throttle diff --git a/lib/plugins/multipart_parser.js b/lib/plugins/multipart_parser.js index 27e186928..32145562a 100644 --- a/lib/plugins/multipart_parser.js +++ b/lib/plugins/multipart_parser.js @@ -36,6 +36,7 @@ function multipartBodyParser(options) { return next(); var form = new formidable.IncomingForm(); + form.keepExtensions = options.keepExtensions ? true : false; return form.parse(req, function (err, fields, files) { if (err) From 8847a9c8eb5743ad72ad982131dda046208824a7 Mon Sep 17 00:00:00 2001 From: Jonathan Wiepert Date: Mon, 9 Jul 2012 12:01:06 -0400 Subject: [PATCH 28/42] set on multipart file uploads --- CHANGES.md | 1 + lib/plugins/multipart_parser.js | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 2ce6f3cfc..40359448e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,7 @@ ## 1.4.4 (not yet released) - GH-160 don't rely on npm's directories: "bin". +- set `req.params.files` on multipart file uploads ## 1.4.3 diff --git a/lib/plugins/multipart_parser.js b/lib/plugins/multipart_parser.js index 32145562a..e0d40489d 100644 --- a/lib/plugins/multipart_parser.js +++ b/lib/plugins/multipart_parser.js @@ -54,9 +54,16 @@ function multipartBodyParser(options) { req.params[k] = fields[k]; }); + + if (req.params.files && !options.overrideParams) { + req.log.warn('parameter files was already sent'); + return; + } + req.params.files = files; } req.log.trace('(multipart): fields=%j', fields); + req.log.trace('(multipart): files=%j', files); return next(); }); }; From 96e7d358fc35583179c318b43e3ec6d051026ca7 Mon Sep 17 00:00:00 2001 From: Shaun Berryman Date: Tue, 10 Jul 2012 13:13:23 -0700 Subject: [PATCH 29/42] Content-MD5 did not match this will re-produce the error --- test/client.test.js | 87 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/test/client.test.js b/test/client.test.js index 9daf1dc95..f37223e23 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -156,6 +156,93 @@ test('PUT json', function (t) { }); +test('GH-169 PUT json Content-MD5', function (t) { + var msg = { + "_id": "4ff71172bc148900000010a3", + "userId": "4f711b377579dbf65e000001", + "courseId": "4f69021bff338faffa000001", + "createdByUserId": "4f711b377579dbf65e000001", + "dateFrom": "2012-06-04", + "dateTo": "2012-09-30", + "notes": "Rates do not include tax & are subject to change without notice\\nRental Clubs are available for $30 per set\\nAll major credit cards accepted", + "updatedAt": "2012-07-06T17:59:08.581Z", + "periods": [{ + "name": "morning", + "weekdayWalking": 1500, + "weekdayCart": 3000, + "weekendWalking": 2000, + "weekendCart": 3500, + "timeFrom": 0, + "timeTo": 780, + "_id": "4ff71172bc148900000010a4" + }, + { + "timeFrom": 780, + "name": "twilight", + "timeTo": 900, + "weekdayWalking": 1500, + "weekdayCart": 2500, + "weekendWalking": 1500, + "weekendCart": 3000, + "_id": "4ff7276cbc148900000010f4" + }, + { + "timeFrom": 900, + "name": "super twilight", + "weekdayWalking": 1200, + "weekdayCart": 2000, + "weekendWalking": 1200, + "weekendCart": 2500, + "timeTo": 1439, + "_id": "4ff7276cbc148900000010f3" + }], + "holidays": [{ + "country": "US", + "name": "Flag Day", + "start": 1339657200000, + "end": 1339743600000, + "date": "2012-06-14" + }, + { + "country": "US / MX", + "name": "Father's Day, Día del Padre (Father's Day)", + "start": 1340262000000, + "end": 1340348400000, + "date": "2012-06-21" + }, + { + "country": "US", + "name": "Independence Day", + "start": 1341385200000, + "end": 1341471600000, + "date": "2012-07-04" + }, + { + "country": "US", + "name": "Labor Day", + "start": 1347001200000, + "end": 1347087600000, + "date": "2012-09-07" + }], + "weekdaySunday": false, + "weekdaySaturday": false, + "weekdayFriday": false, + "weekdayThursday": true, + "weekdayWednesday": true, + "weekdayTuesday": true, + "weekdayMonday": true + }; + + client.put('/json/md5', msg, function (err, req, res, obj) { + t.ifError(err); + t.ok(req); + t.ok(res); + t.equivalent(obj, {hello: 'md5'}); + t.end(); + }); +}); + + test('create string client', function (t) { client = restify.createClient({ url: 'http://127.0.0.1:' + PORT, From 49afab4f24e2e0344626321742f5f85036efb29c Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Fri, 20 Jul 2012 21:43:52 +0000 Subject: [PATCH 30/42] GH-169: JSON Body parser not working correctly as a stream --- lib/plugins/json_body_parser.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/plugins/json_body_parser.js b/lib/plugins/json_body_parser.js index 70c883993..d2341a2cc 100644 --- a/lib/plugins/json_body_parser.js +++ b/lib/plugins/json_body_parser.js @@ -48,8 +48,6 @@ function jsonBodyParser(options) { if (maxBodySize && bytesReceived > maxBodySize) return; req.body += chunk.toString('utf8'); - if (hash) - hash.update(chunk); }); req.on('error', function (err) { return next(err); @@ -62,8 +60,13 @@ function jsonBodyParser(options) { if (!req.body) return next(); - if (hash && req.header('content-md5') !== hash.digest('base64')) - return next(new BadDigestError('Content-MD5 did not match')); + if (hash) { + hash.update(req.body); + var digest = hash.digest('base64'); + if (req.header('content-md5') !== digest) { + return next(new BadDigestError('Content-MD5 did not match')); + } + } try { var params = JSON.parse(req.body); From 915cb7422d177aa1624095869a177c65493f40c6 Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Fri, 20 Jul 2012 21:58:51 +0000 Subject: [PATCH 31/42] GH-177 dtrace-provider 0.0.9 --- package.json | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index e11e94315..0321b1844 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,7 @@ { "author": "Mark Cavage ", "contributors": [ + "Luri Aranda", "Dominic Barnes", "Tamas Daniel", "Domenic Denicola", @@ -11,18 +12,20 @@ "Trent Mick", "Falco Nogatz", "Pedro Palazón", + "Petter Rasmussen", "Andrew Robinson", "Isaac Schlueter", "Andrew Sliwinski", "Matt Smillie", "Simon Sturmer", "Diego Torres", + "Jonathan Wiepert", "Mike Williams" ], "name": "restify", "homepage": "http://mcavage.github.com/node-restify", "description": "REST framework", - "version": "1.4.3", + "version": "1.4.4", "repository": { "type": "git", "url": "git://github.com/mcavage/node-restify.git" @@ -36,10 +39,10 @@ }, "dependencies": { "async": "0.1.22", - "bunyan": "0.8.0", + "bunyan": "0.10.0", "byline": "2.0.2", "formidable": "1.0.11", - "dtrace-provider": "0.0.8", + "dtrace-provider": "0.0.9", "http-signature": "0.9.9", "lru-cache": "1.1.0", "mime": "1.2.5", @@ -53,7 +56,7 @@ "tap": "0.2.5" }, "optionalDependencies": { - "dtrace-provider": "0.0.6" + "dtrace-provider": "0.0.9" }, "scripts": { "test": "./node_modules/.bin/tap ./test/*.test.js" From f6354197477460a8246dbd2c74e485dae7723b3d Mon Sep 17 00:00:00 2001 From: Mark Cavage Date: Fri, 20 Jul 2012 22:06:57 +0000 Subject: [PATCH 32/42] Changelog update --- CHANGES.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 40359448e..29dcc143b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,7 +2,15 @@ ## 1.4.4 (not yet released) +- GH-177 move to dtrace-provider 0.0.9 +- GH-172 option `keepExtensions` in body parser (Luri Aranda) +- GH-173 set req.files on multipart (Jonathan Wiepert) +- GH-169 json body parser failing content-md5, sometimes +- GH-166 plugins assume content-length is integer (Petter Rasmussen) +- GH-164 Get rid of ETag quotes and W/ prefix for ETag header (Dominic Denicola) +- GH-163 Don't default content-length to 0 - GH-160 don't rely on npm's directories: "bin". +- GH-159 req.charset not working (Tamas Daniel) - set `req.params.files` on multipart file uploads ## 1.4.3 From e3afc19e8830dab475bf93174487c152360ebd6b Mon Sep 17 00:00:00 2001 From: Kevin Chan Date: Mon, 23 Jul 2012 15:58:55 -0700 Subject: [PATCH 33/42] docs: Error handling section includes usage of once-deprecated, now non-existant restify.ConflictError --- docs/index.restdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.restdown b/docs/index.restdown index 379c059ce..f0e0ef2f3 100644 --- a/docs/index.restdown +++ b/docs/index.restdown @@ -348,7 +348,7 @@ HTTP status codes as a subclass of `HttpError`. So, for example, you can do this: server.get('/hello/:name', function(req, res, next) { - return next(new restify.ConflictError("I just don't like you")); + return next(new restify.InvalidArgumentError("I just don't like you")); }); $ curl -is -H 'accept: text/*' localhost:8080/hello/mark From ce3c945ff4355ec0bb39295ed93234d21cf6188c Mon Sep 17 00:00:00 2001 From: Kevin Chan Date: Tue, 31 Jul 2012 14:42:42 -0700 Subject: [PATCH 34/42] server.use does not throw up when passing in non-functions --- lib/server.js | 3 ++- test/server.test.js | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/server.js b/lib/server.js index 4a164e0c3..b77a13f68 100644 --- a/lib/server.js +++ b/lib/server.js @@ -41,7 +41,8 @@ function argsToChain() { handlers.forEach(function (h) { if (Array.isArray(h)) return process(h); - if (!typeof (h) === 'function') + + if (typeof (h) !== 'function') throw new TypeError('handlers must be Functions'); return chain.push(h); diff --git a/test/server.test.js b/test/server.test.js index ffb141157..805324624 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -250,6 +250,31 @@ test('get (path and version not ok)', function (t) { }); }); +test('use - throws TypeError on non function as argument', function (t) { + + var server = restify.createServer({ dtrace: DTRACE, log: LOGGER }); + + var err = new TypeError('handlers must be Functions'); + + t.throws(function () { + server.use('/nonfn'); + }, err); + + t.throws(function () { + server.use({an:'object'}); + }, err); + + t.throws(function () { + server.use( + function good() { return next() }, + '/bad', + {really:'bad'} + ); + }, err); + + + t.end(); +}); test('use + get (path only)', function (t) { var server = restify.createServer({ dtrace: DTRACE, log: LOGGER }); From 3593840b64b839f1310b2cc8a9b1ff7309cb0a73 Mon Sep 17 00:00:00 2001 From: William Wicks Date: Thu, 9 Aug 2012 19:04:59 -0700 Subject: [PATCH 35/42] passing 'name' to param pre-conditions --- lib/server.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/server.js b/lib/server.js index b77a13f68..0b72f5518 100644 --- a/lib/server.js +++ b/lib/server.js @@ -587,10 +587,10 @@ Server.prototype._findRoute = function _findRoute(req, res) { * @param {Function} The middleware function to execute */ Server.prototype.param = function param(name, fn) { - return this.use(function (req, res, next) { - if (req.params && req.params[name]) - return fn.apply(this, arguments); + return this.use(function (req, res, next) { + if (req.params && req.params[name]) + return fn.call(this, req, res, next, name); - return next(); - }); + return next(); + }); }; From 1dce56a47d979ba360267c1f7a60df4dc5705661 Mon Sep 17 00:00:00 2001 From: William Wicks Date: Mon, 13 Aug 2012 05:09:48 -0700 Subject: [PATCH 36/42] adding 'value' parameter to server.params --- lib/server.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/server.js b/lib/server.js index 0b72f5518..f3ae9ba65 100644 --- a/lib/server.js +++ b/lib/server.js @@ -588,8 +588,9 @@ Server.prototype._findRoute = function _findRoute(req, res) { */ Server.prototype.param = function param(name, fn) { return this.use(function (req, res, next) { - if (req.params && req.params[name]) - return fn.call(this, req, res, next, name); + var value = req.params && req.params[name]; + if (value) + return fn.call(this, req, res, next, value, name); return next(); }); From c2fdfb25cb823c8d443fa2b901db74f9ef367f49 Mon Sep 17 00:00:00 2001 From: Andrew Dunkman Date: Tue, 14 Aug 2012 08:49:11 -0500 Subject: [PATCH 37/42] Allowed param pattern overriding for url routing. --- lib/route.js | 17 +++++++++++------ lib/server.js | 4 +++- test/route.test.js | 28 ++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/route.js b/lib/route.js index cfd76af99..3e0acc4cd 100644 --- a/lib/route.js +++ b/lib/route.js @@ -184,12 +184,17 @@ function Route(options) { self.pattern += '\\/+'; if (fragment.charAt(0) === ':') { - // Previously was gratuitous, but better to just be standard - // self.pattern += '([a-zA-Z0-9-_~%!;@=+\\$\\*\\.]+)'; - // - // See RFC3986, or this handy table: - // http://en.wikipedia.org/wiki/Percent-encoding#Types_of_URI_characters - self.pattern += '([a-zA-Z0-9-_~\\.%]+)'; + if (options.urlParamPattern) { + self.pattern += '(' + options.urlParamPattern + ')'; + } + else { + // Previously was gratuitous, but better to just be standard + // self.pattern += '([a-zA-Z0-9-_~%!;@=+\\$\\*\\.]+)'; + // + // See RFC3986, or this handy table: + // http://en.wikipedia.org/wiki/Percent-encoding#Types_of_URI_characters + self.pattern += '([a-zA-Z0-9-_~\\.%]+)'; + } self.params.push(fragment.slice(1)); } else { self.pattern += fragment; diff --git a/lib/server.js b/lib/server.js index f3ae9ba65..8331dee4b 100644 --- a/lib/server.js +++ b/lib/server.js @@ -118,6 +118,7 @@ function Server(options) { this.name = options.name || 'restify'; this.preChain = []; this.routes = []; + this.urlParamPattern = options.urlParamPattern; this.version = options.version || false; this.responseTimeHeader = options.responseTimeHeader || 'X-Response-Time'; this.responseTimeFormatter = options.responseTimeFormatter; @@ -423,7 +424,8 @@ Server.prototype._addRoute = function _addRoute(method, options, handlers) { handlers: chain, name: options.name, version: options.version || self.version, - dtrace: self.dtrace + dtrace: self.dtrace, + urlParamPattern: self.urlParamPattern }); route.on('error', function (err) { self.emit('error', err); diff --git a/test/route.test.js b/test/route.test.js index e8fcfac06..f7a9542eb 100644 --- a/test/route.test.js +++ b/test/route.test.js @@ -218,6 +218,34 @@ test('test matches params url-encoded', function (t) { }); +test('test matches params with custom regex', function (t) { + var route = new Route({ + log: new Logger({name: 'restify/test/route'}), + url: '/foo/:bar', + method: 'GET', + urlParamPattern: '[a-zA-Z0-9-_~%!;@=+\\$\\*\\.]+' + }); + t.ok(route); + t.equivalent(route.matches({ + method: 'GET', + path: '/foo/a%40b.com' + }), { bar: 'a@b.com' }); + t.ok(route.matches({ + method: 'GET', + path: '/foo/a@b.com' + }), { bar: 'a@b.com' }); + t.ok(route.matches({ + method: 'GET', + path: '/foo/a*b.com' + }), { bar: 'a*b.com' }); + t.notOk(route.matches({ + method: 'GET', + path: '/foo/a%40b.com/bar' + })); + t.end(); +}); + + test('test matches multiple params', function (t) { var route = new Route({ log: new Logger({name: 'restify/test/route'}), From 484adc9984d9fd2fe5c0c7ace5ac635ee4e2c33b Mon Sep 17 00:00:00 2001 From: Daniel Hammond Date: Sun, 2 Sep 2012 19:00:28 -0400 Subject: [PATCH 38/42] Adding "uploadDir" support to multipart file uploads via formidable. --- lib/plugins/multipart_parser.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/plugins/multipart_parser.js b/lib/plugins/multipart_parser.js index e0d40489d..670387255 100644 --- a/lib/plugins/multipart_parser.js +++ b/lib/plugins/multipart_parser.js @@ -37,6 +37,7 @@ function multipartBodyParser(options) { var form = new formidable.IncomingForm(); form.keepExtensions = options.keepExtensions ? true : false; + if (options.uploadDir) form.uploadDir = options.uploadDir; return form.parse(req, function (err, fields, files) { if (err) From 4b83531cae0361f82474ab54907f61ab65c14ac7 Mon Sep 17 00:00:00 2001 From: Peter Potrebic Date: Tue, 4 Sep 2012 11:33:46 -0700 Subject: [PATCH 39/42] GH-203 - fix this by checking for blank/whitespace bodies. Added unit tests, which fail without fix and pass with fix in place. --- lib/clients/json_client.js | 2 +- test/client.test.js | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/lib/clients/json_client.js b/lib/clients/json_client.js index 23bc08591..630105006 100644 --- a/lib/clients/json_client.js +++ b/lib/clients/json_client.js @@ -50,7 +50,7 @@ JsonClient.prototype.parse = function parse(req, callback) { return this._super.parse.call(this, req, function (err, req, res, data) { var obj; try { - if (data) { + if (data && !/^\s*$/.test(data)) { obj = JSON.parse(data); } else { obj = {}; diff --git a/test/client.test.js b/test/client.test.js index f37223e23..7cbb7ae5b 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -39,7 +39,20 @@ function sendText(req, res, next) { return next(); } +function sendWhitespace(req, res, next) { + var body = ' '; + if (req.params.flavor === 'spaces') { + body = ' '; + } else if (req.params.flavor === 'tabs') { + body = ' \t\t '; + } + + // override contentType as otherwise the string is json-ified to include quotes. Don't want that for this test. + res.contentType = 'text/plain'; + res.send(body); + return next(); +} ///--- Tests @@ -70,6 +83,8 @@ test('setup', function (t) { server.put('/str/:name', sendText); server.post('/str/:name', sendText); + server.get('/whitespace/:flavor', sendWhitespace); + server.listen(PORT, '127.0.0.1', function () { t.end(); }); @@ -99,6 +114,25 @@ test('GET json', function (t) { }); }); +test('GH-203 GET json, body is whitespace', function (t) { + client.get('/whitespace/spaces', function (err, req, res, obj) { + t.ifError(err); + t.ok(req); + t.ok(res); + t.equivalent(obj, {}); + t.end(); + }); +}); + +test('GH-203 GET json, body is tabs', function (t) { + client.get('/whitespace/spaces', function (err, req, res, obj) { + t.ifError(err); + t.ok(req); + t.ok(res); + t.equivalent(obj, {}); + t.end(); + }); +}); test('GH-115 GET path with spaces', function (t) { client.get('/json/foo bar', function (err, req, res, obj) { From 79cb034fa7e00048caefdba1122cbe74f1309119 Mon Sep 17 00:00:00 2001 From: Peter Potrebic Date: Tue, 4 Sep 2012 11:38:04 -0700 Subject: [PATCH 40/42] GH-203 - fix type in 2nd unit test. It was previously a dupe of the 1st --- test/client.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/client.test.js b/test/client.test.js index 7cbb7ae5b..6c42440b5 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -125,7 +125,7 @@ test('GH-203 GET json, body is whitespace', function (t) { }); test('GH-203 GET json, body is tabs', function (t) { - client.get('/whitespace/spaces', function (err, req, res, obj) { + client.get('/whitespace/tabs', function (err, req, res, obj) { t.ifError(err); t.ok(req); t.ok(res); From 446cd36b9d3d3fb56efae5cb7d35b6b1f9a5e2ac Mon Sep 17 00:00:00 2001 From: Thiago Caiubi Date: Wed, 12 Sep 2012 17:05:12 -0300 Subject: [PATCH 41/42] Fix log binding when pre chaining --- lib/request.js | 6 ++++-- lib/server.js | 5 ++++- test/server.test.js | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/request.js b/lib/request.js index 709b5360d..c66e5b09d 100644 --- a/lib/request.js +++ b/lib/request.js @@ -88,9 +88,11 @@ function parseContentLength(req) { module.exports = { - extendRequest: function extendRequest(req) { - var _url = url.parse(req.url); + extendRequest: function extendRequest(options) { + var req = options.request, + _url = url.parse(req.url); + req.log = options.log; req.chunked = req.headers['transfer-encoding'] === 'chunked'; req.contentLength = parseContentLength(req); req.contentType = parseContentType(req); diff --git a/lib/server.js b/lib/server.js index 8331dee4b..ac3790f5d 100644 --- a/lib/server.js +++ b/lib/server.js @@ -460,7 +460,10 @@ Server.prototype._addRoute = function _addRoute(method, options, handlers) { Server.prototype._request = function _request(request, response, expect100) { var self = this; - var req = extendRequest(request); + var req = extendRequest({ + log: self.log, + request: request + }); var res = extendResponse({ formatters: self.formatters, log: self.log, diff --git a/test/server.test.js b/test/server.test.js index 805324624..8b678e8b7 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -692,6 +692,7 @@ test('GH-64 prerouting chain', function (t) { var server = restify.createServer({ dtrace: DTRACE, log: LOGGER }); server.pre(function (req, res, next) { + req.log.info('prerouting chain'); req.headers.accept = 'application/json'; return next(); }); From 40a820c66307f847fa4e9e65809fd6128086ba82 Mon Sep 17 00:00:00 2001 From: Geoff Wagstaff Date: Tue, 18 Sep 2012 18:45:03 +0100 Subject: [PATCH 42/42] Ensure body is given to JsonClient.post If body is was not defined, body would be null (which passes typeof test because typeof null == 'object') and JSON.parse(body) would coerce it to string 'null'. This got sent to the server and triggered json_body_parser to send back InvalidContentError (Invalid JSON). JsonClient should throw an error if body is falsy to prevent this. --- lib/clients/json_client.js | 2 +- test/client.test.js | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/clients/json_client.js b/lib/clients/json_client.js index 630105006..eb95247f2 100644 --- a/lib/clients/json_client.js +++ b/lib/clients/json_client.js @@ -31,7 +31,7 @@ module.exports = JsonClient; JsonClient.prototype.write = function write(options, body, callback) { if (typeof (options) !== 'object') throw new TypeError('options (Object) required'); - if (body !== null && typeof (body) !== 'object') + if (!body || typeof (body) !== 'object') throw new TypeError('body (Object) required'); if (typeof (callback) !== 'function') throw new TypeError('callback (Function) required'); diff --git a/test/client.test.js b/test/client.test.js index 6c42440b5..c47fd819a 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -178,6 +178,13 @@ test('POST json', function (t) { }); }); +test('POST nothing', function (t) { + t.throws(function () { + client.post('/json/mcavage', function(){}); + }, new TypeError('body (Object) required')); + t.end(); +}); + test('PUT json', function (t) { client.post('/json/mcavage', { hello: 'foo' }, function (err, req, res, obj) {