Skip to content

Commit

Permalink
Merge pull request #1423 from simov/fix-multipart-header
Browse files Browse the repository at this point in the history 
Allow fully qualified multipart content-type header
  • Loading branch information
@simov
simov committed Feb 18, 2015
2 parents a822e64 + d94cc54 commit 7ef212f
Show file tree
Hide file tree
Showing 3 changed files with 210 additions and 103 deletions.
21 changes: 13 additions & 8 deletions lib/multipart.js
View file
Expand Up @@ -21,14 +21,14 @@ Multipart.prototype.isChunked = function (options) {
throw new Error('Argument error, options.multipart.')
}

if (self.request.getHeader('transfer-encoding') === 'chunked') {
chunked = true
}

if (options.chunked !== undefined) {
chunked = options.chunked
}

if (self.request.getHeader('transfer-encoding') === 'chunked') {
chunked = true
}

if (!chunked) {
parts.forEach(function (part) {
if(typeof part.body === 'undefined') {
Expand All @@ -51,11 +51,16 @@ Multipart.prototype.setHeaders = function (chunked) {
}

var header = self.request.getHeader('content-type')
var contentType = (!header || header.indexOf('multipart') === -1)
? 'multipart/related'
: header.split(';')[0]

self.request.setHeader('content-type', contentType + '; boundary=' + self.boundary)
if (!header || header.indexOf('multipart') === -1) {
self.request.setHeader('content-type', 'multipart/related; boundary=' + self.boundary)
} else {
if (header.indexOf('boundary') !== -1) {
self.boundary = header.replace(/.*boundary=([^\s;])+.*/, '$1')
} else {
self.request.setHeader('content-type', header + '; boundary=' + self.boundary)
}
}
}

Multipart.prototype.build = function (parts, chunked) {
Expand Down
149 changes: 149 additions & 0 deletions tests/test-multipart-encoding.js
View file
@@ -0,0 +1,149 @@
'use strict'

var http = require('http')
, path = require('path')
, request = require('../index')
, fs = require('fs')
, tape = require('tape')


var localFile = path.join(__dirname, 'unicycle.jpg')
var cases = {
// based on body type
'+array -stream': {
options: {
multipart: [{name: 'field', body: 'value'}]
},
expected: {chunked: false}
},
'+array +stream': {
options: {
multipart: [{name: 'file', body: null}]
},
expected: {chunked: true}
},
// encoding overrides body value
'+array +encoding': {
options: {
headers: {'transfer-encoding': 'chunked'},
multipart: [{name: 'field', body: 'value'}]
},
expected: {chunked: true}
},

// based on body type
'+object -stream': {
options: {
multipart: {data: [{name: 'field', body: 'value'}]}
},
expected: {chunked: false}
},
'+object +stream': {
options: {
multipart: {data: [{name: 'file', body: null}]}
},
expected: {chunked: true}
},
// encoding overrides body value
'+object +encoding': {
options: {
headers: {'transfer-encoding': 'chunked'},
multipart: {data: [{name: 'field', body: 'value'}]}
},
expected: {chunked: true}
},

// based on body type
'+object -chunked -stream': {
options: {
multipart: {chunked: false, data: [{name: 'field', body: 'value'}]}
},
expected: {chunked: false}
},
'+object -chunked +stream': {
options: {
multipart: {chunked: false, data: [{name: 'file', body: null}]}
},
expected: {chunked: true}
},
// chunked overrides body value
'+object +chunked -stream': {
options: {
multipart: {chunked: true, data: [{name: 'field', body: 'value'}]}
},
expected: {chunked: true}
},
// encoding overrides chunked
'+object +encoding -chunked': {
options: {
headers: {'transfer-encoding': 'chunked'},
multipart: {chunked: false, data: [{name: 'field', body: 'value'}]}
},
expected: {chunked: true}
}
}

function runTest(t, test) {

var server = http.createServer(function(req, res) {

t.ok(req.headers['content-type'].match(/^multipart\/related; boundary=[^\s;]+$/))

if (test.expected.chunked) {
t.ok(req.headers['transfer-encoding'] === 'chunked')
t.notOk(req.headers['content-length'])
} else {
t.ok(req.headers['content-length'])
t.notOk(req.headers['transfer-encoding'])
}

// temp workaround
var data = ''
req.setEncoding('utf8')

req.on('data', function(d) {
data += d
})

req.on('end', function() {
// check for the fields traces
if (test.expected.chunked && data.indexOf('name: file') !== -1) {
// file
t.ok(data.indexOf('name: file') !== -1)
// check for unicycle.jpg traces
t.ok(data.indexOf('2005:06:21 01:44:12') !== -1)
} else {
// field
t.ok(data.indexOf('name: field') !== -1)
var parts = test.options.multipart.data || test.options.multipart
t.ok(data.indexOf(parts[0].body) !== -1)
}

res.writeHead(200)
res.end()
})
})

server.listen(6767, function() {
// @NOTE: multipartData properties must be set here
// so that file read stream does not leak in node v0.8
var parts = test.options.multipart.data || test.options.multipart
if (parts[0].name === 'file') {
parts[0].body = fs.createReadStream(localFile)
}

request.post('http://localhost:6767', test.options, function (err, res, body) {
t.equal(err, null)
t.equal(res.statusCode, 200)
server.close(function () {
t.end()
})
})
})
}

Object.keys(cases).forEach(function (name) {
tape('multipart-encoding ' + name, function(t) {
runTest(t, cases[name])
})
})
143 changes: 48 additions & 95 deletions tests/test-multipart.js
View file
Expand Up @@ -6,11 +6,11 @@ var http = require('http')
, fs = require('fs')
, tape = require('tape')


function runTest(t, a) {
var remoteFile = path.join(__dirname, 'googledoodle.jpg')
, localFile = path.join(__dirname, 'unicycle.jpg')
, multipartData = []
, chunked = a.stream || a.chunked || a.encoding

var server = http.createServer(function(req, res) {
if (req.url === '/file') {
Expand All @@ -19,18 +19,15 @@ function runTest(t, a) {
return
}

if (a.mixed) {
t.ok(req.headers['content-type'].match(/multipart\/mixed/))
} else {
t.ok(req.headers['content-type'].match(/multipart\/related/))
}

if (chunked) {
t.ok(req.headers['transfer-encoding'] === 'chunked')
t.notOk(req.headers['content-length'])
if (a.header) {
if (a.header.indexOf('mixed') !== -1) {
t.ok(req.headers['content-type'].match(/^multipart\/mixed; boundary=[^\s;]+$/))
} else {
t.ok(req.headers['content-type']
.match(/^multipart\/related; boundary=XXX; type=text\/xml; start="<root>"$/))
}
} else {
t.ok(req.headers['content-length'])
t.notOk(req.headers['transfer-encoding'])
t.ok(req.headers['content-type'].match(/^multipart\/related; boundary=[^\s;]+$/))
}

// temp workaround
Expand All @@ -42,26 +39,28 @@ function runTest(t, a) {
})

req.on('end', function() {
// check for the fields' traces
// check for the fields traces

// 1st field : my_field
t.ok( data.indexOf('name: my_field') !== -1 )
t.ok( data.indexOf(multipartData[0].body) !== -1 )
t.ok(data.indexOf('name: my_field') !== -1)
t.ok(data.indexOf(multipartData[0].body) !== -1)

// 2nd field : my_buffer
t.ok( data.indexOf('name: my_buffer') !== -1 )
t.ok( data.indexOf(multipartData[1].body) !== -1 )

if (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:6767/file traces
t.ok( data.indexOf('Photoshop ICC') !== -1 )
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)

// 4th field : remote_file
t.ok(data.indexOf('name: remote_file') !== -1)
// check for http://localhost:6767/file traces
t.ok(data.indexOf('Photoshop ICC') !== -1)

if (a.header && a.header.indexOf('mixed') !== -1) {
t.ok(data.indexOf('boundary=XXX'))
}

res.writeHead(200)
Expand All @@ -72,33 +71,21 @@ function runTest(t, a) {
server.listen(6767, function() {

// @NOTE: multipartData properties must be set here so that my_file read stream does not leak in node v0.8
multipartData = chunked
? [
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:6767/file')}
]
: [
{name: 'my_field', body: 'my_value'},
{name: 'my_buffer', body: new Buffer([1, 2, 3])}
]

var reqOptions = {
url: 'http://localhost:6767/upload',
headers: (function () {
var headers = {}
if (a.mixed) {
headers['content-type'] = 'multipart/mixed'
}
if (a.encoding) {
headers['transfer-encoding'] = 'chunked'
}
return headers
}()),
multipart: a.array
? multipartData
: {chunked: a.chunked, data: multipartData}
multipart: multipartData
}
if (a.header) {
reqOptions.headers = {
'content-type': a.header
}
}
if (a.json) {
reqOptions.json = true
Expand All @@ -111,60 +98,26 @@ function runTest(t, a) {
t.end()
})
})

})
}

// array - multipart option is array
// object - multipart option is object
// encoding - headers option have transfer-encoding set to chunked
// mixed - headers option have content-type set to something different than multipart/related
// json - json option
// stream - body contains streams or not
// chunked - chunked is set when multipart is object

// var methods = ['post', 'get']
var cases = [
// based on body type
{name: '+array -stream', args: {array: true, encoding: false, stream: false}},
{name: '+array +stream', args: {array: true, encoding: false, stream: true}},
// encoding overrides stream
{name: '+array +encoding', args: {array: true, encoding: true, stream: false}},

// based on body type
{name: '+object -stream', args: {object: true, encoding: false, stream: false}},
{name: '+object +stream', args: {object: true, encoding: false, stream: true}},
// encoding overrides stream
{name: '+object +encoding', args: {object: true, encoding: true, stream: false}},

// based on body type
{name: '+object -chunked -stream', args: {object: true, encoding: false, chunked: false, stream: false}},
{name: '+object -chunked +stream', args: {object: true, encoding: false, chunked: false, stream: true}},
// chunked overrides stream
{name: '+object +chunked -stream', args: {object: true, encoding: false, chunked: true, stream: false}},
// chunked overrides encoding
{name: '+object +encoding -chunked', args: {object: true, encoding: true, chunked: false, stream: false}},
// stream overrides chunked
{name: '+object +encoding -chunked +stream', args: {object: true, encoding: true, chunked: false, stream: true}}
var testHeaders = [
null,
'multipart/mixed',
'multipart/related; boundary=XXX; type=text/xml; start="<root>"'
]

var suite = ['post', 'get'].forEach(function(method) {
[true, false].forEach(function(json) {
[true, false].forEach(function(mixed) {
cases.forEach(function (test) {
var name = [
'multipart related', method,
(json ? '+' : '-') + 'json',
(mixed ? '+' : '-') + 'mixed',
test.name
].join(' ')

tape(name, function(t) {
test.args.method = method
test.args.json = json
test.args.mixed = mixed
runTest(t, test.args)
})
testHeaders.forEach(function(header) {
[true, false].forEach(function(json) {
var name = [
'multipart-related', method.toUpperCase(),
(header || 'default'),
(json ? '+' : '-') + 'json'
].join(' ')

tape(name, function(t) {
runTest(t, {method: method, header: header, json: json})
})
})
})
Expand Down

0 comments on commit 7ef212f

Please sign in to comment.