Skip to content
This repository has been archived by the owner on Mar 14, 2019. It is now read-only.

Server crashes during image manipulation #550

Open
wursttheke opened this issue Feb 19, 2015 · 7 comments
Open

Server crashes during image manipulation #550

wursttheke opened this issue Feb 19, 2015 · 7 comments

Comments

@wursttheke
Copy link
Contributor

If you define a transformWrite function as described in the docs with some image manipulation like this:

// tranformWrite function
var createThumb = function(fileObj, readStream, writeStream) {
  var size = '96';
  gm(readStream).autoOrient().resize(size, size + '^').gravity('Center').extent(size, size).stream('PNG').pipe(writeStream);
};

When you now upload an erroneous image file, e.g. a simple text file with an image file extension like mytext.png it will result in a server crash, because gm can't identify it as an image:

W20150219-12:29:23.966(1)? (STDERR) 
W20150219-12:29:23.970(1)? (STDERR) events.js:72
W20150219-12:29:23.970(1)? (STDERR)         throw er; // Unhandled 'error' event
W20150219-12:29:23.970(1)? (STDERR)               ^
W20150219-12:29:23.975(1)? (STDERR) Error: Command failed: gm identify: No decode delegate for this image format (/var/folders/2j/bg5bwdr9175c1q4dz132h3gr0000gn/T/gm2cORwu).
W20150219-12:29:23.976(1)? (STDERR) gm identify: Request did not return an image.
W20150219-12:29:23.976(1)? (STDERR) 
W20150219-12:29:23.976(1)? (STDERR)     at ChildProcess._spawn.proc.on.onExit (/Users/phw/.meteor/packages/cfs_graphicsmagick/.0.0.17.vswgdv++os+web.browser+web.cordova/npm/node_modules/gm/lib/command.js:242:17)
W20150219-12:29:23.976(1)? (STDERR)     at ChildProcess.emit (events.js:98:17)
W20150219-12:29:23.976(1)? (STDERR)     at maybeClose (child_process.js:756:16)
W20150219-12:29:23.976(1)? (STDERR)     at Socket.<anonymous> (child_process.js:969:11)
W20150219-12:29:23.977(1)? (STDERR)     at Socket.emit (events.js:95:17)
W20150219-12:29:23.977(1)? (STDERR)     at Pipe.close (net.js:465:12)
=> Exited with code: 8

And somehow (I think because of the automatic retry feature) it will just keep crashing if the server restarts. You would have to delete the temp file to sort of 'fix' the situation.

My current (working) solution is to first do a gm.identify and then catch a possible error:

// tranform write function
var createThumb = function(fileObj, readStream, writeStream) {
  var size = '96';
  var g = gm(readStream);

  g.identify({ bufferStream: true }, Meteor.bindEnvironment(function (err, data) {
    if (err) {
      fileObj.remove(); 
    } else {
      g.autoOrient().resize(size, size + '^').gravity('Center').extent(size, size).stream('PNG').pipe(writeStream);
    }
};

Couple things to notice here.

  • The stream needs to be buffered to be able to do a subsequent gm call.
  • If you want to somehow alter the collection document in case of an error (e.g. just remove it), you have to call Meteor.bindEnvironment for the callback to work the meteor way.

Perhaps there is a more elegant way to this, I'm not seeing.

People upload nasty things, so I think this is something that should be handled by the package in an elegant 'non crashing' way. Deleting the fileObj is certainly not what you want to do. Maybe an error field could be written to it to inform the user something went wrong.

@digz6666
Copy link

Thank you, it helped me to solve my problem.

@evolross
Copy link

I just ran into this same issue a week ago while testing. I think everyone using this library will have this issue if they bothered to test uploading bad files. Something should really be rolled into this package to handle this - that is if it were being maintained, as this is a huge bug. Basically the contentTypes filters don't work.

THANK YOU for this solution! It works good to catch a bad file and then clean things up in your Collections. My only issue is there still doesn't seem to be a way to throw an error back out to the client Images.insert call. Though a work-around is to set a database flag.

Same issue here with a different approach: #598

@evolross
Copy link

evolross commented Sep 4, 2015

Just a heads up, I've been doing more testing and it looks like there's plenty of file types that will NOT cause g.identify to throw an error - but cannot be either transformed or displayed by the browser and will cause problems. For example, rename a PDF to a JPG and upload it. It won't cause g.identify to error, it just hangs in my app. The good news is that the data returned by g.identify has a ton of properties in it including data.format where you can verify that the file equals "JPEG, PNG, GIF.." else throw an error.

List of Graphics Magick compatible file types: http://www.graphicsmagick.org/formats.html

@evolross
Copy link

evolross commented Oct 5, 2015

Update - There's a nice work-around here:

#227

@pcorey
Copy link

pcorey commented Oct 6, 2015

Hey all,

This could potentially be used by malicious users to DOS an application. I've accepted a PR to add an alert to east5th:package-scan for cfs:standard-packages for versions vulnerable to this issue (currently all version).

@huevoncito
Copy link

@wursttheke Having this same issue but can't get the server to stop crashing. What do you mean by delete the temp file? I've cleared all tempstore collections in the db, butt still nothing

@pedroquinteros1969
Copy link

I had the same problem with Windows 10, I switched to Ubuntu 16.04 operating system and works perfectly .. 07 Sep 2016 .. sorry my English ..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants