Control: tags -1 - moreinfo
> Please unblock package node-formidable
> > [...]
> > node-formidable hasn't been updated for years. Testing version isn't
> > compatible with our nodejs but this was not seen since there was no
> > real tests (see https://bugs.debian.org/924589).
> >
> > I upgraded it and added full tests (build and autopkgtest).
> >
> > node-formidable reverse dependencies:
> > - node-superagent:
> > +- node-supertest (no reverse dependencies)
> >
> > node-formidable bug does not affect these packages since the failing
> > functions are not used in it, that's why I decrease bug severity to
> > important. I tested the 2 reverse dependencies with both old and new
> > node-formidable with success (sbuild and autopkgtest).
> >
> > debdiff is big since I embedded some node modules (only for tests,
> > they are not installed). Upstream changes are not so big. I updated
> > also debian/* files and added examples (and tests of course).
> >
> > I think it's low risky to update node-formidable. However I leave
> > you
> > appreciate if it is opportune.
>
>
> Hi all,
>
> when enabling upstream test in 1.2.1-1, it fails only under debci LXC
> environment. After many tries, I fixed this in 1.2.1-2 (was a test
> loop:
> test/run.js tried to test itself). So all tests are OK now. If you
> think
> it is opportune to unblock node-formidable,
Hi all,
here is a partial diff that shows changes on installed files.
Cheers,
Xavier
diff --git a/lib/file.js b/lib/file.js
index 1e7184d..50d34c0 100644
--- a/lib/file.js
+++ b/lib/file.js
@@ -1,7 +1,7 @@
if (global.GENTLY) require = GENTLY.hijack(require);
var util = require('util'),
- WriteStream = require('fs').WriteStream,
+ fs = require('fs'),
EventEmitter = require('events').EventEmitter,
crypto = require('crypto');
@@ -23,17 +23,19 @@ function File(properties) {
if(typeof this.hash === 'string') {
this.hash = crypto.createHash(properties.hash);
+ } else {
+ this.hash = null;
}
}
module.exports = File;
util.inherits(File, EventEmitter);
File.prototype.open = function() {
- this._writeStream = new WriteStream(this.path);
+ this._writeStream = new fs.WriteStream(this.path);
};
File.prototype.toJSON = function() {
- return {
+ var json = {
size: this.size,
path: this.path,
name: this.name,
@@ -43,16 +45,23 @@ File.prototype.toJSON = function() {
filename: this.filename,
mime: this.mime
};
+ if (this.hash && this.hash != "") {
+ json.hash = this.hash;
+ }
+ return json;
};
File.prototype.write = function(buffer, cb) {
var self = this;
+ if (self.hash) {
+ self.hash.update(buffer);
+ }
+
+ if (this._writeStream.closed) {
+ return cb();
+ }
+
this._writeStream.write(buffer, function() {
- if (self.hash) {
- if (self.hash.hasOwnProperty('update')) {
- self.hash.update(buffer);
- }
- }
self.lastModifiedDate = new Date();
self.size += buffer.length;
self.emit('progress', self.size);
@@ -62,10 +71,10 @@ File.prototype.write = function(buffer, cb) {
File.prototype.end = function(cb) {
var self = this;
+ if (self.hash) {
+ self.hash = self.hash.digest('hex');
+ }
this._writeStream.end(function() {
- if(self.hash) {
- self.hash = self.hash.digest('hex');
- }
self.emit('end');
cb();
});
diff --git a/lib/incoming_form.js b/lib/incoming_form.js
index 291236d..dbd920b 100644
--- a/lib/incoming_form.js
+++ b/lib/incoming_form.js
@@ -1,5 +1,6 @@
if (global.GENTLY) require = GENTLY.hijack(require);
+var crypto = require('crypto');
var fs = require('fs');
var util = require('util'),
path = require('path'),
@@ -23,13 +24,15 @@ function IncomingForm(opts) {
this.ended = false;
this.maxFields = opts.maxFields || 1000;
- this.maxFieldsSize = opts.maxFieldsSize || 2 * 1024 * 1024;
+ this.maxFieldsSize = opts.maxFieldsSize || 20 * 1024 * 1024;
+ this.maxFileSize = opts.maxFileSize || 200 * 1024 * 1024;
this.keepExtensions = opts.keepExtensions || false;
- this.uploadDir = opts.uploadDir || os.tmpDir();
+ this.uploadDir = opts.uploadDir || (os.tmpdir && os.tmpdir()) || os.tmpDir();
this.encoding = opts.encoding || 'utf-8';
this.headers = null;
this.type = null;
- this.hash = false;
+ this.hash = opts.hash || false;
+ this.multiples = opts.multiples || false;
this.bytesReceived = null;
this.bytesExpected = null;
@@ -37,10 +40,11 @@ function IncomingForm(opts) {
this._parser = null;
this._flushing = 0;
this._fieldsSize = 0;
+ this._fileSize = 0;
this.openedFiles = [];
return this;
-};
+}
util.inherits(IncomingForm, EventEmitter);
exports.IncomingForm = IncomingForm;
@@ -74,6 +78,40 @@ IncomingForm.prototype.parse = function(req, cb) {
return true;
};
+ // Setup callback first, so we don't miss anything from data events emitted
+ // immediately.
+ if (cb) {
+ var fields = {}, files = {};
+ this
+ .on('field', function(name, value) {
+ fields[name] = value;
+ })
+ .on('file', function(name, file) {
+ if (this.multiples) {
+ if (files[name]) {
+ if (!Array.isArray(files[name])) {
+ files[name] = [files[name]];
+ }
+ files[name].push(file);
+ } else {
+ files[name] = file;
+ }
+ } else {
+ files[name] = file;
+ }
+ })
+ .on('error', function(err) {
+ cb(err, fields, files);
+ })
+ .on('end', function() {
+ cb(null, fields, files);
+ });
+ }
+
+ // Parse headers and setup the parser, ready to start listening for data.
+ this.writeHeaders(req.headers);
+
+ // Start listening for data.
var self = this;
req
.on('error', function(err) {
@@ -97,25 +135,6 @@ IncomingForm.prototype.parse = function(req, cb) {
}
});
- if (cb) {
- var fields = {}, files = {};
- this
- .on('field', function(name, value) {
- fields[name] = value;
- })
- .on('file', function(name, file) {
- files[name] = file;
- })
- .on('error', function(err) {
- cb(err, fields, files);
- })
- .on('end', function() {
- cb(null, fields, files);
- });
- }
-
- this.writeHeaders(req.headers);
-
return this;
};
@@ -126,8 +145,11 @@ IncomingForm.prototype.writeHeaders = function(headers) {
};
IncomingForm.prototype.write = function(buffer) {
+ if (this.error) {
+ return;
+ }
if (!this._parser) {
- this._error(new Error('unintialized parser'));
+ this._error(new Error('uninitialized parser'));
return;
}
@@ -160,6 +182,7 @@ IncomingForm.prototype.onPart = function(part) {
IncomingForm.prototype.handlePart = function(part) {
var self = this;
+ // This MUST check exactly for undefined. You can not change it to !part.filename.
if (part.filename === undefined) {
var value = ''
, decoder = new StringDecoder(this.encoding);
@@ -194,6 +217,14 @@ IncomingForm.prototype.handlePart = function(part) {
this.openedFiles.push(file);
part.on('data', function(buffer) {
+ self._fileSize += buffer.length;
+ if (self._fileSize > self.maxFileSize) {
+ self._error(new Error('maxFileSize exceeded, received '+self._fileSize+' bytes of file data'));
+ return;
+ }
+ if (buffer.length == 0) {
+ return;
+ }
self.pause();
file.write(buffer, function() {
self.resume();
@@ -241,8 +272,8 @@ IncomingForm.prototype._parseContentType = function() {
}
if (this.headers['content-type'].match(/multipart/i)) {
- var m;
- if (m = this.headers['content-type'].match(/boundary=(?:"([^"]+)"|([^;]+))/i)) {
+ var m = this.headers['content-type'].match(/boundary=(?:"([^"]+)"|([^;]+))/i);
+ if (m) {
this._initMultipart(m[1] || m[2]);
} else {
this._error(new Error('bad content-type header, no multipart boundary'));
@@ -264,21 +295,25 @@ IncomingForm.prototype._error = function(err) {
}
this.error = err;
- this.pause();
this.emit('error', err);
if (Array.isArray(this.openedFiles)) {
this.openedFiles.forEach(function(file) {
file._writeStream.destroy();
- setTimeout(fs.unlink, 0, file.path);
+ setTimeout(fs.unlink, 0, file.path, function(error) { });
});
}
};
IncomingForm.prototype._parseContentLength = function() {
+ this.bytesReceived = 0;
if (this.headers['content-length']) {
- this.bytesReceived = 0;
this.bytesExpected = parseInt(this.headers['content-length'], 10);
+ } else if (this.headers['transfer-encoding'] === undefined) {
+ this.bytesExpected = 0;
+ }
+
+ if (this.bytesExpected !== null) {
this.emit('progress', this.bytesReceived, this.bytesExpected);
}
};
@@ -325,10 +360,11 @@ IncomingForm.prototype._initMultipart = function(boundary) {
headerField = headerField.toLowerCase();
part.headers[headerField] = headerValue;
- var m;
+ // matches either a quoted-string or a token (RFC 2616 section 19.5.1)
+ var m = headerValue.match(/\bname=("([^"]*)"|([^\(\)<>@,;:\\"\/\[\]\?=\{\}\s\t/]+))/i);
if (headerField == 'content-disposition') {
- if (m = headerValue.match(/\bname="([^"]+)"/i)) {
- part.name = m[1];
+ if (m) {
+ part.name = m[2] || m[3] || '';
}
part.filename = self._fileName(headerValue);
@@ -366,13 +402,13 @@ IncomingForm.prototype._initMultipart = function(boundary) {
can be divided by 4, it will result in a number of buytes that
can be divided vy 3.
*/
- var offset = parseInt(part.transferBuffer.length / 4) * 4;
- part.emit('data', new Buffer(part.transferBuffer.substring(0, offset), 'base64'))
+ var offset = parseInt(part.transferBuffer.length / 4, 10) * 4;
+ part.emit('data', new Buffer(part.transferBuffer.substring(0, offset), 'base64'));
part.transferBuffer = part.transferBuffer.substring(offset);
};
parser.onPartEnd = function() {
- part.emit('data', new Buffer(part.transferBuffer, 'base64'))
+ part.emit('data', new Buffer(part.transferBuffer, 'base64'));
part.emit('end');
};
break;
@@ -394,10 +430,12 @@ IncomingForm.prototype._initMultipart = function(boundary) {
};
IncomingForm.prototype._fileName = function(headerValue) {
- var m = headerValue.match(/\bfilename="(.*?)"($|; )/i);
+ // matches either a quoted-string or a token (RFC 2616 section 19.5.1)
+ var m = headerValue.match(/\bfilename=("(.*?)"|([^\(\)<>@,;:\\"\/\[\]\?=\{\}\s\t/]+))($|;\s)/i);
if (!m) return;
- var filename = m[1].substr(m[1].lastIndexOf('\\') + 1);
+ var match = m[2] || m[3] || '';
+ var filename = match.substr(match.lastIndexOf('\\') + 1);
filename = filename.replace(/%22/g, '"');
filename = filename.replace(/&#([\d]{4});/g, function(m, code) {
return String.fromCharCode(code);
@@ -434,10 +472,9 @@ IncomingForm.prototype._initOctetStream = function() {
type: mime
});
- file.open();
-
this.emit('fileBegin', filename, file);
-
+ file.open();
+ this.openedFiles.push(file);
this._flushing++;
var self = this;
@@ -466,8 +503,10 @@ IncomingForm.prototype._initOctetStream = function() {
self.ended = true;
var done = function(){
- self.emit('file', 'file', file);
- self._maybeEnd();
+ file.end(function() {
+ self.emit('file', 'file', file);
+ self._maybeEnd();
+ });
};
if(outstandingWrites === 0){
@@ -481,16 +520,12 @@ IncomingForm.prototype._initOctetStream = function() {
IncomingForm.prototype._initJSONencoded = function() {
this.type = 'json';
- var parser = new JSONParser()
+ var parser = new JSONParser(this)
, self = this;
- if (this.bytesExpected) {
- parser.initWithLength(this.bytesExpected);
- }
-
parser.onField = function(key, val) {
self.emit('field', key, val);
- }
+ };
parser.onEnd = function() {
self.ended = true;
@@ -501,14 +536,12 @@ IncomingForm.prototype._initJSONencoded = function() {
};
IncomingForm.prototype._uploadPath = function(filename) {
- var name = '';
- for (var i = 0; i < 32; i++) {
- name += Math.floor(Math.random() * 16).toString(16);
- }
+ var buf = crypto.randomBytes(16);
+ var name = 'upload_' + buf.toString('hex');
if (this.keepExtensions) {
var ext = path.extname(filename);
- ext = ext.replace(/(\.[a-z0-9]+).*/, '$1');
+ ext = ext.replace(/(\.[a-z0-9]+).*/i, '$1');
name += ext;
}
@@ -523,4 +556,3 @@ IncomingForm.prototype._maybeEnd = function() {
this.emit('end');
};
-
diff --git a/lib/json_parser.js b/lib/json_parser.js
index 6ce966b..28a23ba 100644
--- a/lib/json_parser.js
+++ b/lib/json_parser.js
@@ -1,35 +1,30 @@
if (global.GENTLY) require = GENTLY.hijack(require);
-var Buffer = require('buffer').Buffer
+var Buffer = require('buffer').Buffer;
-function JSONParser() {
- this.data = new Buffer('');
+function JSONParser(parent) {
+ this.parent = parent;
+ this.chunks = [];
this.bytesWritten = 0;
-};
-exports.JSONParser = JSONParser;
-
-JSONParser.prototype.initWithLength = function(length) {
- this.data = new Buffer(length);
}
+exports.JSONParser = JSONParser;
JSONParser.prototype.write = function(buffer) {
- if (this.data.length >= this.bytesWritten + buffer.length) {
- buffer.copy(this.data, this.bytesWritten);
- } else {
- this.data = Buffer.concat([this.data, buffer]);
- }
this.bytesWritten += buffer.length;
+ this.chunks.push(buffer);
return buffer.length;
-}
+};
JSONParser.prototype.end = function() {
try {
- var fields = JSON.parse(this.data.toString('utf8'))
+ var fields = JSON.parse(Buffer.concat(this.chunks));
for (var field in fields) {
this.onField(field, fields[field]);
}
- } catch (e) {}
+ } catch (e) {
+ this.parent.emit('error', e);
+ }
this.data = null;
this.onEnd();
-}
\ No newline at end of file
+};
diff --git a/lib/multipart_parser.js b/lib/multipart_parser.js
index 98a6856..36de2b0 100644
--- a/lib/multipart_parser.js
+++ b/lib/multipart_parser.js
@@ -46,7 +46,7 @@ function MultipartParser() {
this.index = null;
this.flags = 0;
-};
+}
exports.MultipartParser = MultipartParser;
MultipartParser.stateToString = function(stateNumber) {
@@ -58,8 +58,8 @@ MultipartParser.stateToString = function(stateNumber) {
MultipartParser.prototype.initWithBoundary = function(str) {
this.boundary = new Buffer(str.length+4);
- this.boundary.write('\r\n--', 'ascii', 0);
- this.boundary.write(str, 'ascii', 4);
+ this.boundary.write('\r\n--', 0);
+ this.boundary.write(str, 4);
this.lookbehind = new Buffer(this.boundary.length+8);
this.state = S.START;
@@ -127,18 +127,25 @@ MultipartParser.prototype.write = function(buffer) {
state = S.START_BOUNDARY;
case S.START_BOUNDARY:
if (index == boundary.length - 2) {
- if (c != CR) {
+ if (c == HYPHEN) {
+ flags |= F.LAST_BOUNDARY;
+ } else if (c != CR) {
return i;
}
index++;
break;
} else if (index - 1 == boundary.length - 2) {
- if (c != LF) {
+ if (flags & F.LAST_BOUNDARY && c == HYPHEN){
+ callback('end');
+ state = S.END;
+ flags = 0;
+ } else if (!(flags & F.LAST_BOUNDARY) && c == LF) {
+ index = 0;
+ callback('partBegin');
+ state = S.HEADER_FIELD_START;
+ } else {
return i;
}
- index = 0;
- callback('partBegin');
- state = S.HEADER_FIELD_START;
break;
}
@@ -214,7 +221,7 @@ MultipartParser.prototype.write = function(buffer) {
case S.PART_DATA:
prevIndex = index;
- if (index == 0) {
+ if (index === 0) {
// boyer-moore derrived algorithm to safely skip non-boundary data
i += boundaryEnd;
while (i < bufferLength && !(buffer[i] in boundaryChars)) {
@@ -226,7 +233,7 @@ MultipartParser.prototype.write = function(buffer) {
if (index < boundary.length) {
if (boundary[index] == c) {
- if (index == 0) {
+ if (index === 0) {
dataCallback('partData', true);
}
index++;
@@ -260,6 +267,7 @@ MultipartParser.prototype.write = function(buffer) {
callback('partEnd');
callback('end');
state = S.END;
+ flags = 0;
} else {
index = 0;
}
@@ -310,7 +318,7 @@ MultipartParser.prototype.end = function() {
self[callbackSymbol]();
}
};
- if ((this.state == S.HEADER_FIELD_START && this.index == 0) ||
+ if ((this.state == S.HEADER_FIELD_START && this.index === 0) ||
(this.state == S.PART_DATA && this.index == this.boundary.length)) {
callback(this, 'partEnd');
callback(this, 'end');
