From 7c0363994d30f8af0331c2fef8ccdf61202f7ca1 Mon Sep 17 00:00:00 2001 From: simov Date: Sat, 8 Nov 2014 11:43:49 +0200 Subject: [PATCH 1/6] Add multipart chunked flag --- request.js | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/request.js b/request.js index ff3cbfb0d..54e56af3a 100644 --- a/request.js +++ b/request.js @@ -685,7 +685,13 @@ Request.prototype.init = function (options) { self._multipart.pipe(self) } if (self.body) { - self.write(self.body) + if (Array.isArray(self.body)) { + self.body.forEach(function (part) { + self.write(part) + }) + } else { + self.write(self.body) + } self.end() } else if (self.requestBodyStream) { console.warn('options.requestBodyStream is deprecated, please pass the request object to stream.pipe.') @@ -1414,7 +1420,14 @@ Request.prototype.form = function (form) { } Request.prototype.multipart = function (multipart) { var self = this - self._multipart = new CombinedStream() + + var chunked = (multipart instanceof Array) + multipart = multipart.data || multipart + + var items = chunked ? new CombinedStream() : [] + function add (part) { + return chunked ? items.append(part) : items.push(new Buffer(part)) + } var headerName = self.hasHeader('content-type') if (!headerName || headerName.indexOf('multipart') === -1) { @@ -1428,7 +1441,7 @@ Request.prototype.multipart = function (multipart) { } if (self.preambleCRLF) { - self._multipart.append('\r\n') + add('\r\n') } multipart.forEach(function (part) { @@ -1442,16 +1455,17 @@ Request.prototype.multipart = function (multipart) { preamble += key + ': ' + part[key] + '\r\n' }) preamble += '\r\n' - self._multipart.append(preamble) - self._multipart.append(body) - self._multipart.append('\r\n') + add(preamble) + add(body) + add('\r\n') }) - self._multipart.append('--' + self.boundary + '--') + add('--' + self.boundary + '--') if (self.postambleCRLF) { - self._multipart.append('\r\n') + add('\r\n') } + self[chunked ? '_multipart' : 'body'] = items return self } Request.prototype.json = function (val) { From 765ae9d1ed3a6db4f2663bb1b0e0b59ca5d82efa Mon Sep 17 00:00:00 2001 From: simov Date: Mon, 10 Nov 2014 19:26:47 +0200 Subject: [PATCH 2/6] Introduce multipart.chunked flag --- request.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/request.js b/request.js index 54e56af3a..a1ef33869 100644 --- a/request.js +++ b/request.js @@ -1421,7 +1421,7 @@ Request.prototype.form = function (form) { Request.prototype.multipart = function (multipart) { var self = this - var chunked = (multipart instanceof Array) + var chunked = (multipart instanceof Array) || multipart.chunked multipart = multipart.data || multipart var items = chunked ? new CombinedStream() : [] From 03bc9998d7b1b0e01d91ccbbb451cf1d1feb47cd Mon Sep 17 00:00:00 2001 From: simov Date: Mon, 10 Nov 2014 19:27:11 +0200 Subject: [PATCH 3/6] Test for all possible multipart configurations --- tests/test-multipart.js | 65 ++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/tests/test-multipart.js b/tests/test-multipart.js index 6673348e6..a8b07d383 100644 --- a/tests/test-multipart.js +++ b/tests/test-multipart.js @@ -6,7 +6,7 @@ var http = require('http') , fs = require('fs') , tape = require('tape') -function runTest(t, json) { +function runTest(t, a) { var remoteFile = path.join(__dirname, 'googledoodle.jpg') , localFile = path.join(__dirname, 'unicycle.jpg') , multipartData = [] @@ -37,42 +37,51 @@ function runTest(t, json) { t.ok( data.indexOf('name: my_buffer') !== -1 ) t.ok( data.indexOf(multipartData[1].body) !== -1 ) - // 3rd field : my_file - t.ok( data.indexOf('name: my_file') !== -1 ) - // check for unicycle.jpg traces - t.ok( data.indexOf('2005:06:21 01:44:12') !== -1 ) + if (a.chunked) { + // 3rd field : my_file + t.ok( data.indexOf('name: my_file') !== -1 ) + // check for unicycle.jpg traces + t.ok( data.indexOf('2005:06:21 01:44:12') !== -1 ) - // 4th field : remote_file - t.ok( data.indexOf('name: remote_file') !== -1 ) - // check for http://localhost:8080/file traces - t.ok( data.indexOf('Photoshop ICC') !== -1 ) + // 4th field : remote_file + t.ok( data.indexOf('name: remote_file') !== -1 ) + // check for http://localhost:8080/file traces + t.ok( data.indexOf('Photoshop ICC') !== -1 ) + } res.writeHead(200) - res.end(json ? JSON.stringify({status: 'done'}) : 'done') + res.end(a.json ? JSON.stringify({status: 'done'}) : 'done') }) }) server.listen(8080, function() { // @NOTE: multipartData properties must be set here so that my_file read stream does not leak in node v0.8 - multipartData = [ - {name: 'my_field', body: 'my_value'}, - {name: 'my_buffer', body: new Buffer([1, 2, 3])}, - {name: 'my_file', body: fs.createReadStream(localFile)}, - {name: 'remote_file', body: request('http://localhost:8080/file')} - ] + multipartData = a.chunked + ? [ + {name: 'my_field', body: 'my_value'}, + {name: 'my_buffer', body: new Buffer([1, 2, 3])}, + {name: 'my_file', body: fs.createReadStream(localFile)}, + {name: 'remote_file', body: request('http://localhost:8080/file')} + ] + : [ + {name: 'my_field', body: 'my_value'}, + {name: 'my_buffer', body: new Buffer([1, 2, 3])} + ] var reqOptions = { url: 'http://localhost:8080/upload', - multipart: multipartData + multipart: a.array + ? multipartData + : {chunked: a.chunked, data: multipartData} } - if (json) { + if (a.json) { reqOptions.json = true } request.post(reqOptions, function (err, res, body) { t.equal(err, null) t.equal(res.statusCode, 200) - t.deepEqual(body, json ? {status: 'done'} : 'done') + t.deepEqual(body, a.json ? {status: 'done'} : 'done') server.close() t.end() }) @@ -80,10 +89,18 @@ function runTest(t, json) { }) } -tape('multipart related', function(t) { - runTest(t, false) -}) +var cases = [ + {name: '-json +array', args: {json: false, array: true, chunked: null}}, + {name: '-json +chunked', args: {json: false, array: false, chunked: true}}, + {name: '-json -chunked', args: {json: false, array: false, chunked: false}}, + + {name: '+json +array', args: {json: true, array: true, chunked: null}}, + {name: '+json +chunked', args: {json: true, array: false, chunked: true}}, + {name: '+json -chunked', args: {json: true, array: false, chunked: false}} +] -tape('multipart related + JSON', function(t) { - runTest(t, true) +cases.forEach(function (test) { + tape('multipart related ' + test.name, function(t) { + runTest.call(null, t, test.args) + }) }) From 5dd3ff163259bfa93faad5118651bb08d5dabe84 Mon Sep 17 00:00:00 2001 From: simov Date: Mon, 10 Nov 2014 22:11:50 +0200 Subject: [PATCH 4/6] Improve multipart docs --- README.md | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 3be9fad19..44c443a36 100644 --- a/README.md +++ b/README.md @@ -295,25 +295,37 @@ See the [form-data README](https://github.com/felixge/node-form-data) for more i Some variations in different HTTP implementations require a newline/CRLF before, after, or both before and after the boundary of a `multipart/related` request (using the multipart option). This has been observed in the .NET WebAPI version 4.0. You can turn on a boundary preambleCRLF or postamble by passing them as `true` to your request options. ```javascript - request( - { method: 'PUT' - , preambleCRLF: true - , postambleCRLF: true - , uri: 'http://service.com/upload' - , multipart: - [ { 'content-type': 'application/json' - , body: JSON.stringify({foo: 'bar', _attachments: {'message.txt': {follows: true, length: 18, 'content_type': 'text/plain' }}}) - } - , { body: 'I am an attachment' } + request({ + method: 'PUT', + preambleCRLF: true, + postambleCRLF: true, + uri: 'http://service.com/upload', + multipart: [ + { + 'content-type': 'application/json' + body: JSON.stringify({foo: 'bar', _attachments: {'message.txt': {follows: true, length: 18, 'content_type': 'text/plain' }}}) + }, + { body: 'I am an attachment' }, + { body: fs.createReadStream('image.png') } + ], + // alternatively pass an object containing additional options + multipart: { + chunked: false, + data: [ + { + 'content-type': 'application/json', + body: JSON.stringify({foo: 'bar', _attachments: {'message.txt': {follows: true, length: 18, 'content_type': 'text/plain' }}}) + }, + { body: 'I am an attachment' } ] } - , function (error, response, body) { - if (error) { - return console.error('upload failed:', error); - } - console.log('Upload successful! Server responded with:', body); + }, + function (error, response, body) { + if (error) { + return console.error('upload failed:', error); } - ) + console.log('Upload successful! Server responded with:', body); + }) ``` @@ -514,10 +526,10 @@ The first argument can be either a `url` or an `options` object. The only requir * `body` - entity body for PATCH, POST and PUT requests. Must be a `Buffer` or `String`, unless `json` is `true`. If `json` is `true`, then `body` must be a JSON-serializable object. * `form` - when passed an object or a querystring, this sets `body` to a querystring representation of value, and adds `Content-type: application/x-www-form-urlencoded` header. When passed no options, a `FormData` instance is returned (and is piped to request). See "Forms" section above. * `formData` - Data to pass for a `multipart/form-data` request. See "Forms" section above. -* `multipart` - (experimental) Data to pass for a `multipart/related` request. See "Forms" section above +* `multipart` - array of objects which contains their own headers and `body` attribute. Sends `multipart/related` request. See _Forms_ section above. + * Alternatively you can pass in an object `{chunked: false, data: []}` where `chunked` is used to specify the `transfer-encoding` header of your request. In non chunked requests body streams are not allowed. * `auth` - A hash containing values `user` || `username`, `pass` || `password`, and `sendImmediately` (optional). See documentation above. * `json` - sets `body` but to JSON representation of value and adds `Content-type: application/json` header. Additionally, parses the response body as JSON. -* `multipart` - (experimental) array of objects which contains their own headers and `body` attribute. Sends `multipart/related` request. See example below. * `preambleCRLF` - append a newline/CRLF before the boundary of your `multipart/form-data` request. * `postambleCRLF` - append a newline/CRLF at the end of the boundary of your `multipart/form-data` request. * `followRedirect` - follow HTTP 3xx responses as redirects (default: `true`). This property can also be implemented as function which gets `response` object as a single argument and should return `true` if redirects should continue or `false` otherwise. From 8c4d81fa90282dbb5409b4804c28b4af65908792 Mon Sep 17 00:00:00 2001 From: James Nylen Date: Mon, 10 Nov 2014 15:31:18 -0600 Subject: [PATCH 5/6] Tweak multipart docs --- README.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 44c443a36..66c4ea3bf 100644 --- a/README.md +++ b/README.md @@ -525,9 +525,16 @@ The first argument can be either a `url` or an `options` object. The only requir * `headers` - http headers (default: `{}`) * `body` - entity body for PATCH, POST and PUT requests. Must be a `Buffer` or `String`, unless `json` is `true`. If `json` is `true`, then `body` must be a JSON-serializable object. * `form` - when passed an object or a querystring, this sets `body` to a querystring representation of value, and adds `Content-type: application/x-www-form-urlencoded` header. When passed no options, a `FormData` instance is returned (and is piped to request). See "Forms" section above. -* `formData` - Data to pass for a `multipart/form-data` request. See "Forms" section above. -* `multipart` - array of objects which contains their own headers and `body` attribute. Sends `multipart/related` request. See _Forms_ section above. - * Alternatively you can pass in an object `{chunked: false, data: []}` where `chunked` is used to specify the `transfer-encoding` header of your request. In non chunked requests body streams are not allowed. +* `formData` - Data to pass for a `multipart/form-data` request. See + [Forms](#forms) section above. +* `multipart` - array of objects which contain their own headers and `body` + attributes. Sends a `multipart/related` request. See [Forms](#forms) section + above. + * Alternatively you can pass in an object `{chunked: false, data: []}` where + `chunked` is used to specify whether the request is sent in + [chunked transfer encoding](https://en.wikipedia.org/wiki/Chunked_transfer_encoding) + (the default is `chunked: true`). In non-chunked requests, data items with + body streams are not allowed. * `auth` - A hash containing values `user` || `username`, `pass` || `password`, and `sendImmediately` (optional). See documentation above. * `json` - sets `body` but to JSON representation of value and adds `Content-type: application/json` header. Additionally, parses the response body as JSON. * `preambleCRLF` - append a newline/CRLF before the boundary of your `multipart/form-data` request. From 33cd9e297a00c5540e55778a24a706effc35434c Mon Sep 17 00:00:00 2001 From: simov Date: Tue, 11 Nov 2014 08:50:13 +0200 Subject: [PATCH 6/6] Defaults multipart chunked to true if not specified --- request.js | 2 +- tests/test-multipart.js | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/request.js b/request.js index a1ef33869..62bd8c14a 100644 --- a/request.js +++ b/request.js @@ -1421,7 +1421,7 @@ Request.prototype.form = function (form) { Request.prototype.multipart = function (multipart) { var self = this - var chunked = (multipart instanceof Array) || multipart.chunked + var chunked = (multipart instanceof Array) || (multipart.chunked === undefined) || multipart.chunked multipart = multipart.data || multipart var items = chunked ? new CombinedStream() : [] diff --git a/tests/test-multipart.js b/tests/test-multipart.js index a8b07d383..5f29b43cd 100644 --- a/tests/test-multipart.js +++ b/tests/test-multipart.js @@ -10,6 +10,7 @@ function runTest(t, a) { var remoteFile = path.join(__dirname, 'googledoodle.jpg') , localFile = path.join(__dirname, 'unicycle.jpg') , multipartData = [] + , chunked = a.array || (a.chunked === undefined) || a.chunked var server = http.createServer(function(req, res) { if (req.url === '/file') { @@ -37,7 +38,7 @@ function runTest(t, a) { t.ok( data.indexOf('name: my_buffer') !== -1 ) t.ok( data.indexOf(multipartData[1].body) !== -1 ) - if (a.chunked) { + if (chunked) { // 3rd field : my_file t.ok( data.indexOf('name: my_file') !== -1 ) // check for unicycle.jpg traces @@ -57,7 +58,7 @@ function runTest(t, a) { server.listen(8080, function() { // @NOTE: multipartData properties must be set here so that my_file read stream does not leak in node v0.8 - multipartData = a.chunked + multipartData = chunked ? [ {name: 'my_field', body: 'my_value'}, {name: 'my_buffer', body: new Buffer([1, 2, 3])}, @@ -90,17 +91,19 @@ function runTest(t, a) { } var cases = [ - {name: '-json +array', args: {json: false, array: true, chunked: null}}, + {name: '-json +array', args: {json: false, array: true}}, + {name: '-json -array', args: {json: false, array: false}}, {name: '-json +chunked', args: {json: false, array: false, chunked: true}}, {name: '-json -chunked', args: {json: false, array: false, chunked: false}}, - {name: '+json +array', args: {json: true, array: true, chunked: null}}, + {name: '+json +array', args: {json: true, array: true}}, + {name: '+json -array', args: {json: true, array: false}}, {name: '+json +chunked', args: {json: true, array: false, chunked: true}}, {name: '+json -chunked', args: {json: true, array: false, chunked: false}} ] cases.forEach(function (test) { tape('multipart related ' + test.name, function(t) { - runTest.call(null, t, test.args) + runTest(t, test.args) }) })