Skip to content

Commit 4da496e

Browse files
alexindigoalexindigo
committed
Added error handling. Streamlined edge cases behavior.
#38 Added check for array data type. #40 Added check for attached streams in getLengthSync. - Emits error in both cases.
1 parent c3b43e0 commit 4da496e

5 files changed

Lines changed: 189 additions & 24 deletions

File tree

‎Readme.md‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
# Form-Data [![Build Status](https://travis-ci.org/felixge/node-form-data.png?branch=master)](https://travis-ci.org/felixge/node-form-data) [![Dependency Status](https://gemnasium.com/felixge/node-form-data.png)](https://gemnasium.com/felixge/node-form-data)
22

3-
A module to create readable `"multipart/form-data"` streams. Can be used to
4-
submit forms and file uploads to other web applications.
3+
A module to create readable ```"multipart/form-data"``` streams. Can be used to submit forms and file uploads to other web applications.
54

6-
The API of this module is inspired by the
7-
[XMLHttpRequest-2 FormData Interface][xhr2-fd].
5+
The API of this module is inspired by the [XMLHttpRequest-2 FormData Interface][xhr2-fd].
86

97
[xhr2-fd]: http://dev.w3.org/2006/webapi/XMLHttpRequest-2/Overview.html#the-formdata-interface
108

‎lib/form_data.js‎

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ FormData.prototype.append = function(field, value, options) {
2828
// all that streamy business can't handle numbers
2929
if (typeof value == 'number') value = ''+value;
3030

31+
// https://github.com/felixge/node-form-data/issues/38
32+
if (util.isArray(value))
33+
{
34+
// Please convert your array into string
35+
// the way web server expects it
36+
this._error(new Error('Arrays are not supported.'));
37+
return;
38+
}
39+
3140
var header = this._multiPartHeader(field, value, options);
3241
var footer = this._multiPartFooter(field, value, options);
3342

@@ -42,7 +51,7 @@ FormData.prototype.append = function(field, value, options) {
4251
FormData.prototype._trackLength = function(header, value, options) {
4352
var valueLength = 0;
4453

45-
// used w/ trackLengthSync(), when length is known.
54+
// used w/ getLengthSync(), when length is known.
4655
// e.g. for streaming directly from a remote server,
4756
// w/ a known file a size, and not wanting to wait for
4857
// incoming file to finish to get its size.
@@ -66,15 +75,11 @@ FormData.prototype._trackLength = function(header, value, options) {
6675
return;
6776
}
6877

78+
// no need to bother with the length
79+
if (!options.knownLength)
6980
this._lengthRetrievers.push(function(next) {
7081

71-
// do we already know the size?
72-
// 0 additional leaves value from getSyncLength()
73-
if (options.knownLength != null) {
74-
next(null, 0);
75-
76-
// check if it's local file
77-
} else if (value.hasOwnProperty('fd')) {
82+
if (value.hasOwnProperty('fd')) {
7883
fs.stat(value.path, function(err, stat) {
7984
if (err) {
8085
next(err);
@@ -197,14 +202,27 @@ FormData.prototype._generateBoundary = function() {
197202
this._boundary = boundary;
198203
};
199204

200-
FormData.prototype.getLengthSync = function() {
201-
var knownLength = this._overheadLength + this._valueLength;
205+
// Note: getLengthSync DOESN'T calculate streams length
206+
// As workaround one can calculate file size manually
207+
// and add it as knownLength option
208+
FormData.prototype.getLengthSync = function(debug) {
209+
var knownLength = this._overheadLength + this._valueLength;
202210

203-
if (this._streams.length) {
204-
knownLength += this._lastBoundary().length;
205-
}
211+
// Don't get confused, there are 3 "internal" streams for each keyval pair
212+
// so it basically checks if there is any value added to the form
213+
if (this._streams.length) {
214+
knownLength += this._lastBoundary().length;
215+
}
216+
217+
// https://github.com/felixge/node-form-data/issues/40
218+
if (this._lengthRetrievers.length) {
219+
// Some async length retrivers are present
220+
// therefore synchronous length calculation is false.
221+
// Please use getLength(callback) to get proper length
222+
this._error(new Error('Cannot calculate proper length in synchronous way.'));
223+
}
206224

207-
return knownLength;
225+
return knownLength;
208226
};
209227

210228
FormData.prototype.getLength = function(cb) {
@@ -279,6 +297,14 @@ FormData.prototype.submit = function(params, cb) {
279297
}.bind(this));
280298
};
281299

300+
FormData.prototype._error = function(err) {
301+
if (this.error) return;
302+
303+
this.error = err;
304+
this.pause();
305+
this.emit('error', err);
306+
};
307+
282308
/*
283309
* Santa's little helpers
284310
*/

‎package.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
22
"author": "Felix Geisendörfer <[email protected]> (http://debuggable.com/)",
33
"name": "form-data",
4-
"description": "A module to create readable `\"multipart/form-data\"` streams. Can be used to submit forms and file uploads to other web applications.",
5-
"version": "0.0.10",
4+
"description": "A module to create readable \"multipart/form-data\" streams. Can be used to submit forms and file uploads to other web applications.",
5+
"version": "0.1.0",
66
"repository": {
77
"type": "git",
88
"url": "git://github.com/felixge/node-form-data.git"
@@ -16,13 +16,13 @@
1616
},
1717
"dependencies": {
1818
"combined-stream": "~0.0.4",
19-
"mime": "~1.2.2",
20-
"async": "~0.2.7"
19+
"mime": "~1.2.9",
20+
"async": "~0.2.9"
2121
},
2222
"devDependencies": {
2323
"fake": "~0.2.2",
2424
"far": "~0.0.7",
25-
"formidable": "~1.0.13",
26-
"request": "~2.16.6"
25+
"formidable": "~1.0.14",
26+
"request": "~2.22.0"
2727
}
2828
}

‎test/integration/test-errors.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
var common = require('../common');
2+
var assert = common.assert;
3+
var FormData = require(common.dir.lib + '/form_data');
4+
var fake = require('fake').create();
5+
var fs = require('fs');
6+
7+
// https://github.com/felixge/node-form-data/issues/38
8+
(function testAppendArray() {
9+
10+
var form = new FormData();
11+
12+
var callback = fake.callback(arguments.callee.name + '-onError-append');
13+
fake.expectAnytime(callback, ['Arrays are not supported.']);
14+
15+
form.on('error', function(err)
16+
{
17+
// workaroud for expectAnytime handling objects
18+
callback(err.message);
19+
});
20+
21+
form.append('my_array', ['bird', 'cute']);
22+
})();
23+
24+
25+
(function testGetLengthSync() {
26+
var fields = [
27+
{
28+
name: 'my_string',
29+
value: 'Test 123'
30+
},
31+
{
32+
name: 'my_image',
33+
value: fs.createReadStream(common.dir.fixture + '/unicycle.jpg')
34+
}
35+
];
36+
37+
var form = new FormData();
38+
var expectedLength = 0;
39+
40+
fields.forEach(function(field) {
41+
form.append(field.name, field.value);
42+
if (field.value.path) {
43+
var stat = fs.statSync(field.value.path);
44+
expectedLength += stat.size;
45+
} else {
46+
expectedLength += field.value.length;
47+
}
48+
});
49+
expectedLength += form._overheadLength + form._lastBoundary().length;
50+
51+
52+
var callback = fake.callback(arguments.callee.name + '-onError-getLengthSync');
53+
fake.expectAnytime(callback, ['Cannot calculate proper length in synchronous way.']);
54+
55+
form.on('error', function(err)
56+
{
57+
// workaroud for expectAnytime handling objects
58+
callback(err.message);
59+
});
60+
61+
var calculatedLength = form.getLengthSync();
62+
63+
// getLengthSync DOESN'T calculate streams length
64+
assert.ok(expectedLength > calculatedLength);
65+
})();

‎test/integration/test-form-get-length-sync.js‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
var common = require('../common');
2+
var assert = common.assert;
3+
var FormData = require(common.dir.lib + '/form_data');
4+
var fake = require('fake').create();
5+
var fs = require('fs');
6+
7+
(function testGetLengthSync() {
8+
var fields = [
9+
{
10+
name: 'my_number',
11+
value: 123
12+
},
13+
{
14+
name: 'my_string',
15+
value: 'Test 123'
16+
},
17+
{
18+
name: 'my_buffer',
19+
value: new Buffer('123')
20+
}
21+
];
22+
23+
var form = new FormData();
24+
var expectedLength = 0;
25+
26+
fields.forEach(function(field) {
27+
form.append(field.name, field.value);
28+
expectedLength += (''+field.value).length;
29+
});
30+
31+
expectedLength += form._overheadLength + form._lastBoundary().length;
32+
var calculatedLength = form.getLengthSync();
33+
34+
assert.equal(expectedLength, calculatedLength);
35+
})();
36+
37+
38+
(function testGetLengthSyncWithKnownLength() {
39+
var fields = [
40+
{
41+
name: 'my_number',
42+
value: 123
43+
},
44+
{
45+
name: 'my_string',
46+
value: 'Test 123'
47+
},
48+
{
49+
name: 'my_buffer',
50+
value: new Buffer('123')
51+
},
52+
{
53+
name: 'my_image',
54+
value: fs.createReadStream(common.dir.fixture + '/unicycle.jpg'),
55+
options: { knownLength: fs.statSync(common.dir.fixture + '/unicycle.jpg').size }
56+
}
57+
];
58+
59+
var form = new FormData();
60+
var expectedLength = 0;
61+
62+
fields.forEach(function(field) {
63+
form.append(field.name, field.value, field.options);
64+
if (field.value.path) {
65+
var stat = fs.statSync(field.value.path);
66+
expectedLength += stat.size;
67+
} else {
68+
expectedLength += (''+field.value).length;
69+
}
70+
});
71+
expectedLength += form._overheadLength + form._lastBoundary().length;
72+
73+
var calculatedLength = form.getLengthSync();
74+
75+
assert.equal(expectedLength, calculatedLength);
76+
})();

0 commit comments

Comments
 (0)