Skip to content
This repository
  • 6 commits
  • 10 files changed
  • 0 comments
  • 1 contributor
Jul 16, 2010
Felix Geisendörfer Revert "Feature: IncomingForm.timeout"
This reverts commit 5e2612c.

There seems to be problems with this, causing non-stalled uploads to
report timeouts as well. Need to investigate.
15f0079
Jul 17, 2010
Felix Geisendörfer Fix: Handling of empty header values and field data
This patch introduces two new events to the multipart parser:
'onHeaderEnd' and 'onHeadersEnd'. These make it much easier to properly
handle the output of the parser.

Another nice addition is a system test that verifies a complete upload
from start to finish.
99c1354
Felix Geisendörfer Add .npmignore file 3b67d93
Felix Geisendörfer Make clean for test/tmp bf21f0c
Felix Geisendörfer Feature: IncomingForm.maxFieldSize
The parser is now considered safe. A malicious client can no longer
allocate huge amounts of memory by sending a big field.
d99eea0
Felix Geisendörfer Bump version 18de5c6
2  .npmignore
... ...
@@ -0,0 +1,2 @@
  1
+test/tmp/
  2
+*.upload
7  Makefile
... ...
@@ -1,4 +1,7 @@
1 1
 test:
2  
-	@find test/{simple,integration}/test-*.js | xargs -n 1 -t node
  2
+	@find test/{simple,integration,system}/test-*.js | xargs -n 1 -t node
3 3
 
4  
-.PHONY: test
  4
+clean:
  5
+	rm test/tmp/*
  6
+
  7
+.PHONY: test clean
13  Readme.md
Source Rendered
@@ -14,7 +14,6 @@ A node.js module for dealing with web forms.
14 14
 
15 15
 ### Todo
16 16
 
17  
-* Limit buffer size for fields
18 17
 * Implement QuerystringParser the same way as MultipartParser
19 18
 
20 19
 ## Installation
@@ -74,12 +73,6 @@ Creates a new incoming form.
74 73
 
75 74
 The encoding to use for incoming form fields.
76 75
 
77  
-#### incomingForm.timeout = 30000
78  
-
79  
-The time in ms after which a stalled upload causes an error. Can be set to
80  
-`false` to disable this feature. The resulting error object will have a
81  
-property `timeout` that is set to `true`.
82  
-
83 76
 #### incomingForm.uploadDir = '/tmp'
84 77
 
85 78
 The directory for placing file uploads in. You can later on move them using `fs.rename()`.
@@ -92,6 +85,12 @@ If you want the files written to `incomingForm.uploadDir` to include the extensi
92 85
 
93 86
 Either 'multipart' or 'urlencoded' depending on the incoming request.
94 87
 
  88
+#### incomingForm.maxFieldSize = 2 * 1024 * 1024
  89
+
  90
+Limits the amount of memory a field (not file) can allocate in bytes.
  91
+I this value is exceeded, an `'error'` event is emitted. The default
  92
+size is 2MB.
  93
+
95 94
 #### incomingForm.bytesReceived
96 95
 
97 96
 The amount of bytes received for this form so far.
80  lib/formidable/incoming_form.js
@@ -14,10 +14,10 @@ function IncomingForm() {
14 14
   this.error = null;
15 15
   this.ended = false;
16 16
 
  17
+  this.maxFieldSize = 2 * 1024 * 1024;
17 18
   this.keepExtensions = false;
18 19
   this.uploadDir = '/tmp';
19 20
   this.encoding = 'utf-8';
20  
-  this.timeout = 30000;
21 21
   this.headers = null;
22 22
   this.type = null;
23 23
 
@@ -26,6 +26,7 @@ function IncomingForm() {
26 26
 
27 27
   this._parser = null;
28 28
   this._flushing = 0;
  29
+  this._fieldsSize = 0;
29 30
 };
30 31
 sys.inherits(IncomingForm, EventEmitter);
31 32
 exports.IncomingForm = IncomingForm;
@@ -63,15 +64,6 @@ IncomingForm.prototype.parse = function(req, cb) {
63 64
   this.writeHeaders(req.headers);
64 65
 
65 66
   var self = this;
66  
-  if (this.timeout) {
67  
-    req.connection.setTimeout(this.timeout);
68  
-    req.connection.addListener('timeout', function() {
69  
-      var err = new Error('timeout, no data received for '+self.timeout+'ms')
70  
-      err.timeout = true;
71  
-      self._error(err);
72  
-    });
73  
-  }
74  
-
75 67
   req
76 68
     .addListener('error', function(err) {
77 69
       self._error(err);
@@ -156,13 +148,16 @@ IncomingForm.prototype.handlePart = function(part) {
156 148
       , decoder = new StringDecoder(this.encoding);
157 149
 
158 150
     part.addListener('data', function(buffer) {
  151
+      self._fieldsSize += buffer.length;
  152
+      if (self._fieldsSize > self.maxFieldSize) {
  153
+        self._error(new Error('maxFieldSize exceeded, received '+self._fieldsSize+' bytes of field data'));
  154
+        return;
  155
+      }
159 156
       value += decoder.write(buffer);
160 157
     });
161 158
 
162 159
     part.addListener('end', function() {
163  
-      if (value) {
164  
-        self.emit('field', part.name, value);
165  
-      }
  160
+      self.emit('field', part.name, value);
166 161
     });
167 162
     return;
168 163
   }
@@ -243,29 +238,9 @@ IncomingForm.prototype._initMultipart = function(boundary) {
243 238
 
244 239
   var parser = new MultipartParser()
245 240
     , self = this
246  
-    , headerField = ''
247  
-    , headerValue = ''
248  
-    , part
249  
-    , addHeader = function() {
250  
-        headerField = headerField.toLowerCase();
251  
-        part.headers[headerField] = headerValue;
252  
-
253  
-        var m;
254  
-        if (headerField == 'content-disposition') {
255  
-          if (m = headerValue.match(/name="([^"]+)"/i)) {
256  
-            part.name = m[1];
257  
-          }
258  
-
259  
-          if (m = headerValue.match(/filename="([^"]+)"/i)) {
260  
-            part.filename = m[1].substr(m[1].lastIndexOf('\\') + 1);
261  
-          }
262  
-        } else if (headerField == 'content-type') {
263  
-          part.mime = headerValue;
264  
-        }
265  
-
266  
-        headerField = '';
267  
-        headerValue = '';
268  
-      };
  241
+    , headerField
  242
+    , headerValue
  243
+    , part;
269 244
 
270 245
   parser.initWithBoundary(boundary);
271 246
 
@@ -275,12 +250,11 @@ IncomingForm.prototype._initMultipart = function(boundary) {
275 250
     part.name = null;
276 251
     part.filename = null;
277 252
     part.mime = null;
  253
+    headerField = '';
  254
+    headerValue = '';
278 255
   };
279 256
 
280 257
   parser.onHeaderField = function(b, start, end) {
281  
-    if (headerValue) {
282  
-      addHeader();
283  
-    }
284 258
     headerField += b.toString(self.encoding, start, end);
285 259
   };
286 260
 
@@ -288,12 +262,32 @@ IncomingForm.prototype._initMultipart = function(boundary) {
288 262
     headerValue += b.toString(self.encoding, start, end);
289 263
   };
290 264
 
291  
-  parser.onPartData = function(b, start, end) {
292  
-    if (headerValue) {
293  
-      addHeader();
294  
-      self.onPart(part);
  265
+  parser.onHeaderEnd = function() {
  266
+    headerField = headerField.toLowerCase();
  267
+    part.headers[headerField] = headerValue;
  268
+
  269
+    var m;
  270
+    if (headerField == 'content-disposition') {
  271
+      if (m = headerValue.match(/name="([^"]+)"/i)) {
  272
+        part.name = m[1];
  273
+      }
  274
+
  275
+      if (m = headerValue.match(/filename="([^"]+)"/i)) {
  276
+        part.filename = m[1].substr(m[1].lastIndexOf('\\') + 1);
  277
+      }
  278
+    } else if (headerField == 'content-type') {
  279
+      part.mime = headerValue;
295 280
     }
296 281
 
  282
+    headerField = '';
  283
+    headerValue = '';
  284
+  };
  285
+
  286
+  parser.onHeadersEnd = function() {
  287
+    self.onPart(part);
  288
+  };
  289
+
  290
+  parser.onPartData = function(b, start, end) {
297 291
     part.emit('data', b.slice(start, end));
298 292
   };
299 293
 
2  lib/formidable/multipart_parser.js
@@ -181,6 +181,7 @@ MultipartParser.prototype.write = function(buffer) {
181 181
       case S.HEADER_VALUE:
182 182
         if (c == CR) {
183 183
           dataCallback('headerValue', true);
  184
+          callback('headerEnd');
184 185
           state = S.HEADER_VALUE_ALMOST_DONE;
185 186
         }
186 187
         break;
@@ -195,6 +196,7 @@ MultipartParser.prototype.write = function(buffer) {
195 196
           return i;
196 197
         }
197 198
 
  199
+        callback('headersEnd');
198 200
         state = S.PART_DATA_START;
199 201
         break;
200 202
       case S.PART_DATA_START:
2  package.json
... ...
@@ -1,5 +1,5 @@
1 1
 { "name" : "formidable"
2  
-, "version": "0.9.5"
  2
+, "version": "0.9.6"
3 3
 , "dependencies": {"gently": ">=0.7.0"}
4 4
 , "directories" : { "lib" : "./lib/formidable" }
5 5
 , "main" : "./lib/formidable/index"
BIN  test/fixture/multi_video.upload
Binary file not shown
28  test/integration/test-multipart-parser.js
@@ -15,9 +15,9 @@ Object.keys(fixtures).forEach(function(name) {
15 15
 
16 16
     , parts = []
17 17
     , part = null
18  
-    , headerField = null
19  
-    , headerValue = null
20  
-    , endCalled = false;
  18
+    , headerField
  19
+    , headerValue
  20
+    , endCalled = '';
21 21
 
22 22
   parser.initWithBoundary(fixture.boundary);
23 23
   parser.onPartBegin = function() {
@@ -28,27 +28,21 @@ Object.keys(fixtures).forEach(function(name) {
28 28
   };
29 29
 
30 30
   parser.onHeaderField = function(b, start, end) {
31  
-    var str = b.toString('ascii', start, end);
32  
-    if (headerValue) {
33  
-      part.headers[headerField] = headerValue;
34  
-      headerField = '';
35  
-      headerValue = '';      
36  
-    }
37  
-    headerField += str;
  31
+    headerField += b.toString('ascii', start, end);
38 32
   };
39 33
 
40 34
   parser.onHeaderValue = function(b, start, end) {
41  
-    var str = b.toString('ascii', start, end);
42  
-    headerValue += str;
  35
+    headerValue += b.toString('ascii', start, end);
43 36
   }
44 37
 
  38
+  parser.onHeaderEnd = function() {
  39
+    part.headers[headerField] = headerValue;
  40
+    headerField = '';
  41
+    headerValue = '';
  42
+  };
  43
+
45 44
   parser.onPartData = function(b, start, end) {
46 45
     var str = b.toString('ascii', start, end);
47  
-    if (headerField) {
48  
-      part.headers[headerField] = headerValue;
49  
-      headerValue = '';
50  
-      headerField = '';
51  
-    }
52 46
     part.data += b.binarySlice(start, end);
53 47
   }
54 48
 
49  test/simple/test-incoming-form.js
@@ -29,33 +29,24 @@ test(function constructor() {
29 29
   assert.strictEqual(form.keepExtensions, false);
30 30
   assert.strictEqual(form.uploadDir, '/tmp');
31 31
   assert.strictEqual(form.encoding, 'utf-8');
32  
-  assert.strictEqual(form.timeout, 30000);
33 32
   assert.strictEqual(form.bytesReceived, null);
34 33
   assert.strictEqual(form.bytesExpected, null);
  34
+  assert.strictEqual(form.maxFieldSize, 2 * 1024 * 1024);
35 35
   assert.strictEqual(form._parser, null);
36 36
   assert.strictEqual(form._flushing, 0);
  37
+  assert.strictEqual(form._fieldsSize, 0);
37 38
   assert.ok(form instanceof EventEmitterStub);
38 39
   assert.equal(form.constructor.name, 'IncomingForm');
39 40
 });
40 41
 
41 42
 test(function parse() {
42  
-  var REQ = {headers: {}, connection: {}}
43  
-    , emit = {}
44  
-    , reqEmit = {};
  43
+  var REQ = {headers: {}}
  44
+    , emit = {};
45 45
 
46 46
   gently.expect(form, 'writeHeaders', function(headers) {
47 47
     assert.strictEqual(headers, REQ.headers);
48 48
   });
49 49
 
50  
-  gently.expect(REQ.connection, 'setTimeout', function(timeout) {
51  
-    assert.equal(timeout, form.timeout);
52  
-  });
53  
-
54  
-  gently.expect(REQ.connection, 'addListener', function(event, fn) {
55  
-    assert.equal(event, 'timeout');
56  
-    reqEmit[event] = fn;
57  
-  });
58  
-
59 50
   var events = ['error', 'data', 'end'];
60 51
   gently.expect(REQ, 'addListener', events.length, function(event, fn) {
61 52
     assert.equal(event, events.shift());
@@ -126,15 +117,6 @@ test(function parse() {
126 117
 
127 118
     assert.strictEqual(form.resume(), false);
128 119
   })();
129  
-
130  
-  (function testReqEmitTimeout() {
131  
-    gently.expect(form, '_error', function(err) {
132  
-      assert.equal(err.message, 'timeout, no data received for '+form.timeout+'ms');
133  
-      assert.equal(err.timeout, true);
134  
-    });
135  
-
136  
-    reqEmit.timeout();
137  
-  })();
138 120
   
139 121
   (function testEmitError() {
140 122
     var ERR = new Error('something bad happened');
@@ -186,8 +168,6 @@ test(function parse() {
186 168
       , REQ = {headers: {}}
187 169
       , parseCalled = 0;
188 170
   
189  
-    form.timeout = null;
190  
-
191 171
     gently.expect(form, 'writeHeaders');
192 172
     gently.expect(REQ, 'addListener', 3, function() {
193 173
       return this;
@@ -423,8 +403,11 @@ test(function _initMultipart() {
423 403
     PARSER.onHeaderField(new Buffer('content-disposition'), 10, 19);
424 404
     PARSER.onHeaderValue(new Buffer('form-data; name="field1"'), 0, 14);
425 405
     PARSER.onHeaderValue(new Buffer('form-data; name="field1"'), 14, 24);
  406
+    PARSER.onHeaderEnd();
426 407
     PARSER.onHeaderField(new Buffer('foo'), 0, 3);
427 408
     PARSER.onHeaderValue(new Buffer('bar'), 0, 3);
  409
+    PARSER.onHeaderEnd();
  410
+    PARSER.onHeadersEnd();
428 411
     PARSER.onPartData(new Buffer('hello world'), 0, 5);
429 412
     PARSER.onPartData(new Buffer('hello world'), 5, 11);
430 413
     PARSER.onPartEnd();
@@ -460,8 +443,11 @@ test(function _initMultipart() {
460 443
     PARSER.onPartBegin();
461 444
     PARSER.onHeaderField(new Buffer('content-disposition'), 0, 19);
462 445
     PARSER.onHeaderValue(new Buffer('form-data; name="field2"; filename="C:\\Documents and Settings\\IE\\Must\\Die\\Sunset.jpg"'), 0, 85);
  446
+    PARSER.onHeaderEnd();
463 447
     PARSER.onHeaderField(new Buffer('Content-Type'), 0, 12);
464 448
     PARSER.onHeaderValue(new Buffer('text/plain'), 0, 10);
  449
+    PARSER.onHeaderEnd();
  450
+    PARSER.onHeadersEnd();
465 451
     PARSER.onPartData(new Buffer('... contents of file1.txt ...'), 0, 29);
466 452
     PARSER.onPartEnd();
467 453
   })();
@@ -566,6 +552,21 @@ test(function handlePart() {
566 552
     PART.emit('end');
567 553
   })();
568 554
 
  555
+  (function testFieldSize() {
  556
+    form.maxFieldSize = 8;
  557
+    var PART = new events.EventEmitter();
  558
+    PART.name = 'my_field';
  559
+
  560
+    gently.expect(form, '_error', function(err) {
  561
+      assert.equal(err.message, 'maxFieldSize exceeded, received 9 bytes of field data');
  562
+    });
  563
+
  564
+    form.handlePart(PART);
  565
+    form._fieldsSize = 1;
  566
+    PART.emit('data', new Buffer(7));
  567
+    PART.emit('data', new Buffer(1));
  568
+  })();
  569
+
569 570
   (function testFilePart() {
570 571
     var PART = new events.EventEmitter()
571 572
       , FILE = new events.EventEmitter()
52  test/system/test-multi-video-upload.js
... ...
@@ -0,0 +1,52 @@
  1
+require('../common');
  2
+var BOUNDARY = '---------------------------10102754414578508781458777923'
  3
+  , FIXTURE = TEST_FIXTURES+'/multi_video.upload'
  4
+  , fs = require('fs')
  5
+  , sys = require('sys')
  6
+  , http = require('http')
  7
+  , formidable = require('formidable')
  8
+  , server = http.createServer();
  9
+
  10
+server.addListener('request', function(req, res) {
  11
+  var form = new formidable.IncomingForm()
  12
+    , files = {};
  13
+
  14
+  form.uploadDir = TEST_TMP;
  15
+  form.parse(req);
  16
+
  17
+  form
  18
+    .addListener('field', function(field, value) {
  19
+      assert.equal(field, 'title');
  20
+      assert.equal(value, '');
  21
+    })
  22
+    .addListener('file', function(field, file) {
  23
+      assert.equal(field, 'upload');
  24
+      files[file.filename] = true;
  25
+    })
  26
+    .addListener('end', function() {
  27
+      assert.deepEqual
  28
+        ( files
  29
+        , { 'shortest_video.flv': true
  30
+          , 'shortest_video.mp4' :true
  31
+          }
  32
+        );
  33
+      server.close();
  34
+      res.writeHead(200);
  35
+      res.end('good');
  36
+    });
  37
+});
  38
+
  39
+server.listen(TEST_PORT, function() {
  40
+  var client = http.createClient(TEST_PORT)
  41
+    , headers = {'content-type': 'multipart/form-data; boundary='+BOUNDARY}
  42
+    , request = client.request('POST', '/', headers)
  43
+    , fixture = new fs.ReadStream(FIXTURE);
  44
+
  45
+  fixture
  46
+    .addListener('data', function(b) {;
  47
+      request.write(b);
  48
+    })
  49
+    .addListener('end', function() {
  50
+      request.end();
  51
+    });
  52
+});

No commit comments for this range

Something went wrong with that request. Please try again.