15

I have been working on this problem for two days now, and I am stuck. I am using Node.js with Express, and I am trying to implement an upload form. Basically, I want the form to do the following:

  • Check the size of the file and cancel the upload if it is too large (when I say cancel, I mean prevent any further data from being written to disk and remove the temp file)

  • Check the file type and confirm that it is the correct type (.jpg, .png, etc.), if it is not, then stop any further writing to disk and remove the temp file.

Currently, I have the upload working, and I emit errors when a file is too large or it does not match the correct type, and then I remove the file with fs.unlink() after the entire file has been written to disk. But I see a potential problem with this approach: what if a user uploads a huge file (GB's in size)? With my approach right now, it will eventually be removed from my machine but not after wasting a ton of resources. So basically, I am looking to use the minimal amount of resources in order to confirm that a file is okay for uploading. This is the code I have so far:

    var path = absolutePath + '/public/images/users/' + req.session.userId + '/';
    var maxSize = 3146000; // 3MB
    var form = new formidable.IncomingForm();
    form.uploadDir = path;
    form.keepExtensions = true;

    form.on('error', function(message) {
        if(message)
        {
            res.json({err: message});
        }
        else
        {
            res.json({err: 'Upload error, please try again'});
        }
    });

    form.on('fileBegin', function(name, file){
        if(form.bytesExpected > maxSize)
        {
            this.emit('error', 'Size must not be over 3MB');
        }
    });

    form.on('file', function(name, file) {
        var type = file.type;
        type = type.split('/');
        type = type[1];

        if(type != 'jpeg' && type != 'png' && type != 'gif')
        {
            this.emit('error', "JPG's, PNG's, GIF's only");
            fs.unlink(file.path);
        }
        else
        {
            fs.rename(file.path, path + 'profile.' + type);
        }
    });

    form.on('progress', function(bytesReceived, bytesExpected) {
            console.log(bytesReceived); //This is just to view progress
    });

    form.parse(req);

I am also confused because according to the documents at https://github.com/felixge/node-formidable, it says:

A request that experiences an error is automatically paused, you will have to manually call request.resume() if you want the request to continue firing 'data' events.

This would be great, but I can't seem to get it to work. Whenever I emit an 'error', the 'data' events keep firing until completion.

Attempts

I have tried canceling the request when an error is occurred, but to no avail. req.pause() did nothing for me, req.end() and req.abort() gave me an error saying that it wasn't a method, and req.connection.destroy() and req.connection.end() just sent a loop of POST requests.

Final Thoughts

So what I am looking for seems like it should be common-place, but I have spent the last two days searching the internet on a thorough implementation, and I can't seem to find anything. I mean, it is simple to check the size and type of the file AFTER the entire thing has been uploaded, but who wants to waste all of those resources? Not to mention the things that malicious users could do.

I will continue working until I get exactly what I am looking for but I thought that this issue may be relevant for some other users, and hopefully I can get some help!

Thanks for your time.

jfizz
  • 465
  • 5
  • 17

8 Answers8

8

I will attempt to answer my own question...

So after some trial and error with Formidable, I simply gave up on it and switched over to Multiparty. Multiparty ACTUALLY cancels the upload when an error is emitted which was huge.

My Solution

So my solution utilizes client-side size and type checking (not shown in code). The request is then sent to the server. At the server, I check again if the file size and type are correct BEFORE WRITING TO DISK. I am able to do this by using Multiparty's part event. If they are not correct then I simply send a response with a 413 error. (Thanks to josh3736 for clarifying what is supposed to happen in the browser.) After sending back a 413, the behavior of the browser is a little sporadic. For the browser that I was testing in, it just showed a pending post request. I believe this behavior is due to the fact that the entirety of the form has not be handled, therefore, it will not accept any responses. This may not seem like the most elegant way to deal with it since no error code is displayed, but this behavior will only be encountered by malicious users who bypass the client-side checking (my site is dependent on Javascript so all users will have it enabled if they want to use my site). So that is my solution in words, now for some code...

app.post('/profile/profile_pic', urlencoded, function (req, res) {

    var path = absolutePath + '/public/images/users/' + req.session.userId + '/';
    var maxSize = 3146000; // 3MB

    var options = {uploadDir: path};
    var form = new multiparty.Form();

    form.on('error', function(message) {
        res.status = 413;
        res.send(413, 'Upload too large');
        res.end();
    });

    form.on('file', function(name, file) {
        var type = file.headers['content-type'];
        type = type.split('/');
        type = type[1];
        fs.rename(file.path, path + 'profile.' + type);
        path = '/images/users/' + req.session.userId + '/profile.' + type;
    });

    form.on('part', function(part) {
        var type = part.headers['content-type'];
        var size = part.byteCount - part.byteOffset;

        if(type != 'image/jpeg' && type != 'image/png' && type != 'image/gif' != 'application/pdf' || size > maxSize)
        {
            this.emit('error');
        }
    });

    form.on('close', function(){
        res.json({err: 0, path: path});
    });

    form.parse(req);

});
jfizz
  • 465
  • 5
  • 17
  • Switched to multiparty with no luck but found a [front end solution](https://stackoverflow.com/questions/37620165/xhr-abort-doesnt-stop-file-uploading/50767305#50767305). – cage rattler Jun 08 '18 at 19:43
6

To abort the upload, the proper thing to do is close the socket.

req.socket.end();

Unfortunately, the situation where a server wants to abort an in-progress HTTP upload is a mess.

The proper, spec-compliant thing to do here is simply send a HTTP 413 response early – that is, as soon as you detect that the client has sent more bytes than you want to handle. It is up to you whether or not you terminate the socket after sending the error response. This is in line with RFC 2616. [...] What happens next is not ideal.

  • If you leave the socket open, all browsers (Chrome 30, IE 10, Firefox 21) will keep sending data until the entire file is uploaded. Then and only then, the browser will display your error message. This really sucks since the user must wait for the entire file to complete the upload, only to find out the server rejected it. It also wastes your bandwidth.

    The browsers' current behavior is in violation of RFC 2616 § 8.2.2:

    An HTTP/1.1 (or later) client sending a message-body SHOULD monitor the network connection for an error status while it is transmitting the request. If the client sees an error status, it SHOULD immediately cease transmitting the body. If the body is being sent using a "chunked" encoding (section 3.6), a zero length chunk and empty trailer MAY be used to prematurely mark the end of the message. If the body was preceded by a Content-Length header, the client MUST close the connection.

    There are open Chrome and Firefox issues, but don't expect a fix any time soon.

  • If you close the socket immediately after sending the HTTP 413 response, all browsers will obviously stop uploading immediately, but they currently show a "connection reset" error (or similar), not any HTML you might send in the response.

    Again, this is probably a violation of the spec (which allows the server to send a response early and close the connection), but I wouldn't expect browser fixes any time soon here either.

The fact you're seeing a loop of POST requests is suspect. Are you using some kind of AJAX uploader? It may be automatically retrying the upload after you close the socket early.

Community
  • 1
  • 1
josh3736
  • 139,160
  • 33
  • 216
  • 263
  • Thanks for the answer. I tried using `req.socket.end()` but to no avail. However, you did bring up a very good point that I did not think about... I am using the jQuery Form plugin to submit my form via AJAX, so that maybe why I am getting a POST loop. The only thing is.. when I send the POST request to upload an image, in my Chrome dev tools, it says that only one POST was sent. However, in my console for my Node server, it shows seven have been sent. So I am not sure where to look to solve this problem. Any thoughts? – jfizz Dec 13 '13 at 15:57
  • Actually, I think I found the reason behind my looping problem: http://stackoverflow.com/questions/14302512/what-happens-when-no-response-is-received-for-a-request-im-seeing-retries. So basically, the browser is going to retry several times when nothing is sent back because of `req.connection.destroy()`. Now I am trying to look into a way where I can send out the appropriate headers before I destroy the connection. – jfizz Dec 13 '13 at 23:10
  • 2
    Don't `destroy`, send a HTTP 413 and `end`. – josh3736 Dec 14 '13 at 00:06
  • That will send a response back at the time when the amount uploaded is larger than my limit, but it does not stop the entire file from being downloaded. – jfizz Dec 14 '13 at 01:34
  • 3
    Sorry, forgot to mention that you have to set the `Connection` header to `close`. `res.header('Connection', 'close'); res.send(413, 'Upload too large');` node will automatically close the socket, terminating the upload. – josh3736 Dec 14 '13 at 01:59
  • When I tried that, I got an error saying _Error: Can't set headers after they are sent._ and I got the loop of POST requests. – jfizz Dec 14 '13 at 03:19
  • Actually, I fixed the Error, but the loop of POSTs are still there. – jfizz Dec 14 '13 at 03:39
  • Sorry for all of the comments, but I also noticed, with your implementation, that my Chrome Dev tools never show a response from the server, it says it is pending... Not sure if this would shine some light on what is going on.. – jfizz Dec 14 '13 at 03:48
  • All the above failed for me. Node did not close the socket for me using @josh3736's solution. [This](https://stackoverflow.com/questions/37620165/xhr-abort-doesnt-stop-file-uploading/50767305#50767305) worked though. – cage rattler Jun 08 '18 at 19:45
1

Some time has passed and now you can use multer instead of formidable or multiparty. Multer has desired functionality built in.

Eugene Mala
  • 1,081
  • 12
  • 25
  • This is not an answer! Indeed `multer` is far better than `formidable`. `formidable` principles are bad, they are making the library lightweight but what a developer wants from a library is that it should hide the complexity of the file uploads and provide a simple interface for handling it. `formidable` going way simpler that it is not handling the complexity whilst letting developers add other plugins for different tasks. For most developers `multer` is far more good choice. **Lets keep these kind of opinionated answers in the comments and not as answers.** :) – Rohit Kumar Jan 24 '22 at 13:17
1

Form.pause actually will prevent browser sending more data. Call below function did it for me:

var closeConnection = function(res, form){
    try{
    if (!res.ended){
        form.onPart=null;
        form.pause();
        res.end();
        res.ended = true;
    }
    }catch(e){console.log(e);}
}
rq4t
  • 43
  • 6
0

I tried with your solution, it seemed it didn't work on my machine. Client couldn't get the response after I put this line this.emit('error'); within form.on section. It seemed my program was blocked. it meant you couldn't call your upload method twice. the second call wasn't executed since your program was blocked by this line: this.emit('Error'); already.

upload:function(req, res) {
            var form = new multiparty.Form();
            form.on('error', function(message) {
                res.send('Not Okay');
            });
            form.on('part', function(part) {
                console.log('enter form on');
                this.emit('Error');
                res.send('Not Okay');
            });
            form.on('close', function(){
                res.send('Not Okay');
            });
            form.parse(req);
    }

The only solution I found was call part.resume(); within form.on section. it can consume your upload file on your server without touching your disk. but it will consume your network and server resources. Anyway, it's far better than program was blocked situation. That is why I'm keeping looking for another better way to resolve it.

user3034559
  • 1,259
  • 2
  • 15
  • 31
  • I have not encountered any problems like this. When my server gets a request that is too big or wrong file size, it sends back a 413, and I can confirm this by viewing the output to my console. The program wasn't blocked at all. We must have something different with our setup. – jfizz Dec 17 '13 at 00:58
0

Here is my solution:

var maxSize = 30 * 1024 * 1024;    //30MB
app.post('/upload', function(req, res) {

    var size = req.headers['content-length'];
    if (size <= maxSize) {
        form.parse(req, function(err, fields, files) {
            console.log("File uploading");
            if (files && files.upload) {
                res.status(200).json({fields: fields, files: files});
                fs.renameSync(files.upload[0].path, uploadDir + files.upload[0].originalFilename);
            }
            else {
              res.send("Not uploading");
            }
        });
    }
    else {
        res.send(413, "File to large");
    }

And in case of wasting client's uploading time before get response, control it in the client javascript.

if (fileElement.files[0].size > maxSize) {
    ....
}
Soul Clinic
  • 1,189
  • 12
  • 18
  • 3
    I do not think you can trust that the `Content-Length` header will match the size of the file being uploaded. See [here](http://stackoverflow.com/questions/11300641/get-file-size-from-multipart-form-upload-content-length-header) – jfizz May 08 '14 at 05:16
0

Here is a solution that works for me using multiparty.

The answer given by jfizz does not work on all browsers.

VERY IMPORTANT: As josh3736 mentioned on a comment above, you MUST set the Connection header to close!

form.on('part', function(part) {
  if (![some condition I want to be met]) {
    form.emit('error', new Error('File upload canceled by the server.'));
    return;
  }
  ... code to handle the uploading process normally ...
});

form.on('error', function(err) {
  console.log('ERROR uploading the file:',err.message);
  res.header('Connection', 'close');
  res.status(400).send({ status:'error' });
  setTimeout(function() { res.end(); }, 500);
  next(err);
});
user3621841
  • 815
  • 2
  • 10
  • 18
0

I don't know if the thread is still waiting for a answer but in case that is, you can prevent file upload in formidable by setting form.maxFileSize to zero. If the condition are meet, he will send a error event saying that the maxFileSize exceeded.

fileUpload = false
form = new require("formidable").IncomingForm()

if (fileUpload)
    // Do stuff here
else
    form.maxFileSize = 0
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459