form.parse() not functioning as described in the documentation

[andrewhodel]

form.parse(request, [cb])

Parses an incoming node.js request containing form data. If cb is provided, autoFields and autoFiles are set to true and all fields and files are collected and passed to the callback:

---

The file event will fire for each and every file selected in a multiple file input. This is expected.

However, when using form.parse and autoFiles: true only one of the many files are passed to the callback.

The documentation indicates that when using autoFiles: true all fields and files are collected and passed to the callback but this is not what I am experiencing.

The problem that this causes for me is that I need to ensure a function is called for each file, after all files have been sent. If I call that function in the file event for each one, I have no way of verifying that.

I would like to be able to loop through any array of files passed to the callback of form.parse, and which point I can ensure that the function is called for each.

[andrewrk]

It looks like this behavior is covered by some test cases; specifically https://github.com/superjoe30/node-multiparty/blob/master/test/standalone/test-issue-4.js

Could you provide a test case which fails?

[andrewhodel]

I'm just reading the documentation, it says "all fields and files are collected and passed to the callback".

This must be a documentation bug.

[andrewrk]

I am claiming that the test case I linked above shows an example of all fields and files being collected and passed to the callback. Can you explain why my claim is incorrect?

[andrewhodel]

Here are 3 files from an upload which were selected by a simple input type file with multiple.

These are being logged from the file event, it fires once for each file as expected

form.on('file', function (name, file) { console.log(file); ]);

{ originalFilename: 'ss.png', path: '/tmp/4903-1kom3oe.png', headers: { 'content-disposition': 'form-data; name="Filedata"; filename="ss.png"', 'content-type': 'image/png' }, ws: { path: '/tmp/4903-1kom3oe.png', fd: 14, writable: false, flags: 'w', encoding: 'binary', mode: 438, bytesWritten: 178786, busy: false, _queue: [], _open: [Function], _events: { error: [Object], close: [Object] }, drainable: true }, hash: 'a4aad987d1467dc16a627e32bfd11b0a', size: 178786 } { originalFilename: 'laguna-bay-dslam-router-config.pdf', path: '/tmp/4903-p2m9vh.pdf', headers: { 'content-disposition': 'form-data; name="Filedata"; filename="laguna-bay-dslam-router-config.pdf"', 'content-type': 'application/pdf' }, ws: { path: '/tmp/4903-p2m9vh.pdf', fd: 15, writable: false, flags: 'w', encoding: 'binary', mode: 438, bytesWritten: 39409, busy: false, _queue: [], _open: [Function], _events: { error: [Object], close: [Object] }, drainable: true }, hash: '613c41d0b02feac58235ebc2b77bb4ba', size: 39409 } { originalFilename: 'winbox.exe', path: '/tmp/4903-by6lpz.exe', headers: { 'content-disposition': 'form-data; name="Filedata"; filename="winbox.exe"', 'content-type': 'application/x-msdownload' }, ws: { path: '/tmp/4903-by6lpz.exe', fd: 14, writable: false, flags: 'w', encoding: 'binary', mode: 438, bytesWritten: 114176, busy: false, _queue: [], _open: [Function], _events: { error: [Object], close: [Object] }, drainable: true }, hash: 'a63bb3a7ba06ee55c06a8315c2cfa066', size: 114176 }

This is the same upload with the data from form.parse();

It only contains one of the files, not all of the files as mentioned in the doc.

form.parse(request, function (err, fields, files) { console.log(files); });

{ Filedata: { originalFilename: 'winbox.exe', path: '/tmp/4903-by6lpz.exe', headers: { 'content-disposition': 'form-data; name="Filedata"; filename="winbox.exe"', 'content-type': 'application/x-msdownload' }, ws: { path: '/tmp/4903-by6lpz.exe', fd: 14, writable: false, flags: 'w', encoding: 'binary', mode: 438, bytesWritten: 114176, busy: false, _queue: [], _open: [Function], _events: [Object], drainable: true }, hash: 'a63bb3a7ba06ee55c06a8315c2cfa066', size: 114176 } }

[andrewrk]

If you go one step further and provide some .js code with an assertion failure and a request fixture it would make this straightforward enough that I can deal with it right now. Otherwise it'll go on my "investigate eventually" list.

[andrewhodel]

Sorry I don't have time to put together a proper test, but here's how I worked around it:

Inside: http.createServer(function (request, response) {

                // break out params
                var up = url.parse(request.url, true);
                // check if this is a file upload
                if (up.pathname === '/upload' && request.method === 'POST') {
                    // parse a file upload
                    var form = new multiparty.Form();
                    form.on('error', function (err) {
                        response.writeHead(400, {
                            'content-type': 'text/plain'
                        });
                        response.end("invalid request: " + err);
                    });
                    form.on('close', function () {});
                    var multipartybug = [];
                    form.on('file', function (name, file) {
                        multipartybug.push(file);
                    });
                    form.parse(request, function (err, fields, files) {
                        if ('username' == conf.username && bcrypt.compareSync('password', conf.password)) {
                            // process files
                            function iterate(file, cb) {
                                // runs once for each file
                                processFile(file.path, file.originalFilename, function (err) {
                                    cb(err, null);
                                });
                            }
                            async.map(multipartybug, iterate, function (err, results) {
                                response.statusCode = 200;
                                response.setHeader("Content-Type", "text/html");
                                response.setHeader('Access-Control-Allow-Origin', '*');
                                response.end('success');
                            });
                        } else {
                            response.statusCode = 401;
                            response.setHeader("Content-Type", "text/html");
                            response.setHeader('Access-Control-Allow-Origin', '*');
                            response.end('error');
                        }
                    });
                }

Now on the client side, just a simple input type file.

input type="file" multiple="multiple" id="uploadFiles" name="files[]"

And make a simple ajax uploader, I suppose you could also use form action:

    document.getElementById('uploadFiles').addEventListener('change', function (evt) {
        var files = evt.target.files; // FileList object
        var formdata = new FormData();
        formdata.append('param1', 'value');
        // open xhr
        var xhr = new XMLHttpRequest();
        // progress event
        xhr.upload.addEventListener("progress", function (e) {
            if (e.lengthComputable) {
                var currentState = (e.loaded / e.total) * 100;
                console.log(currentState+'%');
            }
        }, false);
        // error event
        xhr.upload.addEventListener("error", function (e) {
            alert('error uploading file');
        }, false);
        // loadend event
        xhr.upload.addEventListener("loadend", function (e) {
            alert('done');
        }, false);
        // Loop through the FileList
        for (var i = 0, f; f = files[i]; i++) {
            // add file to formdata
            formdata.append('Filedata', files[i]);
        }
        // send xhr
        xhr.open('POST', serverApi + '/upload');
        xhr.send(formdata);
    });

Upload a single file. Everything will work as expected with multiparty, the file event will fire once and the parse event will return the the file in the files array.

Upload again but this time select multiple files, the file event will fire once for every file as expected but the parse event will only return one file in the files array no matter the number uploaded. The way I got around this behavior was by expecting the parse event to finish after the file events no matter how many of them there are (this is true).

Because of this, I was able to push data to an array each iteration of the file event, then use async.map() in the parse event to gain access to the data of all the uploaded files.

[andrewhodel]

The overall reason for needing the data of all files in the files array from the parse event is because without it you cannot respond to the upload (from the perspective of the server) immediately as the request finishes with the list of files they uploaded.

[andrewrk]

I looked into the issue. It looks like a design flaw - we try to collect all the files into an object where the property is the input name and the value is the file object. The problem is that the <input type="file" multiple="multiple"> upload uses the same name for each file, so each successive file overwrites the older ones with the same name.

This is not a problem if you use the event-based API as you suggested in this issue. However I recommend against using a hybrid as you did. If you're going to use the event-based API, then use form.on('end', handler) instead of passing a callback to form.parse().

To resolve this issue we have a couple options:

1. Break API compatibility and change files to be an array instead of an object.
2. Collect files into both an array and an object for convenience and supply both in the callback. This would retain API compatibility.
3. Document the issue and recommend using the event-based API in this use case.

Since the callback-based API is merely a convenience on top of the event-based API, I would suggest we go for option 2.

@jonathanong do you have an opinion on this matter?

[andrewhodel]

I say option 2, thanks for looking into it.

[jonathanong]

2 would be fine. i would just say mirror however node-formidable did it (i don't know how they did).