Skip to content
This repository
  • 2 commits
  • 3 files changed
  • 2 comments
  • 1 contributor
Jun 29, 2010
Felix Geisendörfer Fix: Handle pause() calls on destroyed uploads
This patch also improves the behavior for resume().

net.Stream currently does not guarantee 'data' to stop emitting after
pausing a stream. This makes the current throttling function
problematic and prone to race conditions. This patch will take of it
for now, but a further patch for node itself will make things even
smoother.
3988f82
Felix Geisendörfer Bump version 857559e
17  lib/formidable/incoming_form.js
@@ -31,7 +31,16 @@ exports.IncomingForm = IncomingForm;
31 31
 
32 32
 IncomingForm.prototype.parse = function(req, cb) {
33 33
   this.pause = function() {
34  
-    req.pause();
  34
+    try {
  35
+      req.pause();
  36
+    } catch (err) {
  37
+      // the stream was destroyed
  38
+      if (!this.ended) {
  39
+        // before it was completed, crash & burn
  40
+        this._error(err);
  41
+      }
  42
+      return false;
  43
+    }
35 44
     return true;
36 45
   };
37 46
 
@@ -39,7 +48,11 @@ IncomingForm.prototype.parse = function(req, cb) {
39 48
     try {
40 49
       req.resume();
41 50
     } catch (err) {
42  
-      this._error(err);
  51
+      // the stream was destroyed
  52
+      if (!this.ended) {
  53
+        // before it was completed, crash & burn
  54
+        this._error(err);
  55
+      }
43 56
       return false;
44 57
     }
45 58
 
2  package.json
... ...
@@ -1,5 +1,5 @@
1 1
 { "name" : "formidable"
2  
-, "version": "0.9.3"
  2
+, "version": "0.9.4"
3 3
 , "dependencies": {"gently": ">=0.7.0"}
4 4
 , "directories" : { "lib" : "./lib/formidable" }
5 5
 , "main" : "./lib/formidable/index"
43  test/simple/test-incoming-form.js
@@ -58,13 +58,41 @@ test(function parse() {
58 58
     gently.expect(REQ, 'pause');
59 59
     assert.strictEqual(form.pause(), true);
60 60
   })();
61  
-  
  61
+
  62
+  (function testPauseCriticalException() {
  63
+    form.ended = false;
  64
+
  65
+    var ERR = new Error('dasdsa');
  66
+    gently.expect(REQ, 'pause', function() {
  67
+      throw ERR;
  68
+    });
  69
+
  70
+    gently.expect(form, '_error', function(err) {
  71
+      assert.strictEqual(err, ERR);
  72
+    });
  73
+
  74
+    assert.strictEqual(form.pause(), false);
  75
+  })();
  76
+
  77
+  (function testPauseHarmlessException() {
  78
+    form.ended = true;
  79
+
  80
+    var ERR = new Error('dasdsa');
  81
+    gently.expect(REQ, 'pause', function() {
  82
+      throw ERR;
  83
+    });
  84
+
  85
+    assert.strictEqual(form.pause(), false);
  86
+  })();
  87
+
62 88
   (function testResume() {
63 89
     gently.expect(REQ, 'resume');
64 90
     assert.strictEqual(form.resume(), true);
65 91
   })();
66 92
 
67  
-  (function testResumeException() {
  93
+  (function testResumeCriticalException() {
  94
+    form.ended = false;
  95
+
68 96
     var ERR = new Error('dasdsa');
69 97
     gently.expect(REQ, 'resume', function() {
70 98
       throw ERR;
@@ -76,6 +104,17 @@ test(function parse() {
76 104
 
77 105
     assert.strictEqual(form.resume(), false);
78 106
   })();
  107
+
  108
+  (function testResumeHarmlessException() {
  109
+    form.ended = true;
  110
+
  111
+    var ERR = new Error('dasdsa');
  112
+    gently.expect(REQ, 'resume', function() {
  113
+      throw ERR;
  114
+    });
  115
+
  116
+    assert.strictEqual(form.resume(), false);
  117
+  })();
79 118
   
80 119
   (function testEmitError() {
81 120
     var ERR = new Error('something bad happened');

Showing you all comments on commits in this comparison.

TJ Holowaychuk

cool that seems to have done the trick :D thanks man, great lib

Felix Geisendörfer
Owner

np, let me know if you find anything else : )

Something went wrong with that request. Please try again.