Skip to content

Commit

Permalink
Fixed incomplete file upload issue. Closes #167
Browse files Browse the repository at this point in the history
Route now waits until all files have been written / closed
until executing
  • Loading branch information
tj committed Feb 3, 2010
1 parent 0186659 commit 2701dfd
Showing 1 changed file with 16 additions and 4 deletions.
20 changes: 16 additions & 4 deletions lib/express/core.js
Expand Up @@ -219,18 +219,28 @@ Server = Class({
request.setBodyEncoding('binary')
if (request.headers['content-type'] &&
request.headers['content-type'].indexOf('multipart/form-data') !== -1) {
var stream = new multipart.Stream(request)
var stream = new multipart.Stream(request),
promise = new process.Promise,
pendingFiles = 0
request.params = { post: {}}
stream.addListener('part', function(part){
if (part.filename) {
var file = new File(part.tempfile = '/tmp/express-' + Number(new Date) + uid(), 'w')
++pendingFiles
part.pos = 0
part.addListener('body', function(chunk){
file.write(chunk, part.pos, 'binary')
file.write(chunk, part.pos, 'binary').addErrback(function(){
promise.emitError.apply(promise, arguments)
})
part.pos += chunk.length
})
part.addListener('complete', function(){
file.close()
file.close().addCallback(function(){
if (!--pendingFiles)
promise.emitSuccess()
}).addErrback(function(){
promise.emitError.apply(promise, arguments)
})
mergeParam(part.name, part, request.params.post)
})
}
Expand All @@ -242,7 +252,9 @@ Server = Class({
if (part.buf.length)
mergeParam(part.name, part.buf, request.params.post)
})
}).addListener('complete', function(){ self.route(request, response) })
}).addListener('complete', function(){
promise.addCallback(function(){ self.route(request, response) })
})
}
else
request
Expand Down

2 comments on commit 2701dfd

@aheckmann
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there are a couple issues here.
First, if this is not a file upload, promise.emitSuccess() is never executed resulting in request timeouts.
Second, (and I might be way off here), on an upload, if the first part fires complete before the next part starts, emitSuccess will fire too early.

@tj
Copy link
Member Author

@tj tj commented on 2701dfd Feb 4, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its a bit of a clusterfuck, lovely verbose async stuff. As for the non-file multi-part body
it should work fine (pretty sure) because of !--pendingFiles, however when no parts (including regular form fields)
are present it might not fire correctly, ill have to try that out.

I think you are right about that actually, I will see what I can do to work around that, might not be any
reasonable way to determine how many files are inbound

Please sign in to comment.