3

I'm writing a Sails-js based application where user can upload an image file to server. I'd like to check if the file is truly an image file before saving it to disk.

After struggling with skipper disk adapter I noticed that apparently I couldn't check file properties before finishing the upload. If I checked the file type and tried to respond with res.json("error, wrong file type") or something similar, the client never got response unless I let the upload to finish and responded only after that. This also means I have to do all validations after upload. Which is especially nasty since I have Sails policies which secure the endpoint via middlewares. Unfinished upload somehow manages to block even the authentication middleware which should run before controller.

Below is my best attempt which simply saves the file on disk and in case of validation error, deletes it. Very unsatisfactory. Also, it will never respond if unauthenticated user tries to upload a file:

 create: function (req, res) {

    var valid = true;
    if (!req.body.headline || !req.body.content) {
        valid = false;
    }

    var settings = {
        maxBytes: 10000000,
        dirname: path.join(sails.config.appPath, '/assets/images')
    };

    req.file('image').upload(settings, function whenDone(err, uploadedFiles) {

        if (err) {
            return res.negotiate(err);
        }

        if (uploadedFiles[0]) {
            var upload = req.file('image')._files[0].stream,
                headers = upload.headers,
                allowedTypes = ['image/jpeg', 'image/png'];

            if (_.indexOf(allowedTypes, headers['content-type']) === -1 || !valid) {
                //file type is not supported, abort and delete file
                /** TODO: Ideally we would check the file type and other validations before upload and saving. However,
                 *  skipper didn't seem to support any obvious ways to abort requests at the moment of writing:
                 *  https://github.com/balderdashy/skipper/issues/80
                 *  https://stackoverflow.com/questions/31061719/sails-js-with-skipper-check-if-file-input-were-empty-before-starting-upload
                 *
                 *  Therefore returning from this request before finishing upload results hanging response. Investigate
                 *  alternatives when more time.
                 *
                 *  NOTE: If unauthenticated user with file upload tries to use this endpoint, they will not get response, since
                 *  authentication middleware rejects request but skipper prevents server from responding!
                 */
                var fileAdapter = SkipperDisk();
                return fileAdapter.rm(uploadedFiles[0].fd, function () {
                    res.status(400);
                    return res.json({message: 'Wrong fileformat or missing attributes. Please provide .png or .jpg file and ' +
                    'ensure you have content and headline fields defined.'});
                });
            }

        }

        if (valid) {
            Announcement.create({
                headline: req.body.headline,
                content: req.body.content,
                author: req.session.user})
                .then(function(announcement) {

                    if (uploadedFiles[0]) {
                        announcement.imagePath = uploadedFiles[0].fd;
                        announcement.imageUrl = '/announcements/' + announcement.id + '/image';
                    } else {
                        announcement.imagePath = null;
                        announcement.imageUrl = null;
                    }
                    announcement.save(function(err, saved) {
                        return res.json(saved);
                    });
                });
        } else {
            res.status(400);
            return res.json({message: 'Missing attributes content and/or headline.'});
        }




    });
}

After getting frustrated into Skipper's disk-adapter I browsed its documentation and found docs about writing my own receiver. Armed with that information and similar issue in Stackoverflow I created the following code:

create: function (req, res) {

    var allowedTypes = ['image/jpeg', 'image/png'];

    //output stream to disk
    var output = require('fs').createWriteStream('./storedImage.png');

    //custom receiver for Skipper
    var receiver = new stream.Writable({objectMode: true});
    receiver._write = function(file, enc, done) {

        file.pipe(output);

        output.once('finish', function () {
            console.log("file transfer finished");
            receiver.end();
            return done();
        });

        file.once('readable', function() {
            //tiedoston luku aloitetaan.
            var headers = req.file('image')._files[0].stream.headers;
            console.log("reading file...");

                req.validate({
                    headline: 'string',
                    content: 'string'
                });

            if (_.indexOf(allowedTypes, headers['content-type']) === -1) {
                console.log("forbidden img type!");
                file.end();
            }
        });

        file.once('data', function(d) {
           console.log(d)
        });

        file.once('end', function() {
            console.log("input end");
            output.end();
        });


        output.on('error', function (err) {
            console.log("error in output stream ", err);
            return done({
                incoming: file,
                outgoing: output,
                code: 'E_WRITE',
                stack: typeof err === 'object' ? err.stack : new Error(err),
                name: typeof err === 'object' ? err.name : err,
                message: typeof err === 'object' ? err.message : err
            });
        });

    };

    req.file('image').upload(receiver, function(err, files) {

        if (err) {
            return res.json("There was a problem :(");
        }

        console.log(files);
        return res.json({success: files });
    });

This way I have a bit more control over the streams and I think I figured out the basic idea of getting stream from request, then piping it to filestream. In readable event I try to check the file's type. However, aborting the stream is still a problem. After some trial and error I managed to call end() functions for both file and output streams. If I understood streams correctly, they should close them off. I get correct looking return values from my req.file('image).upload function immediately after receiver detects wrong file type. However, I still can return response! After trying with really big file, it looks like

file.on('data', function(d) {
           console.log(d)
        });

keeps logging new chunks which means that file-stream didn't close off as I expected.

So in the end my final question is, how I can abort the incoming http-request stream properly in Sails.js/Skipper?

Community
  • 1
  • 1
Tumetsu
  • 1,661
  • 2
  • 17
  • 29

0 Answers0