Skip to content

Loading a JPEG with precision set to 12 crashes all processes #1160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
logicbomb421 opened this issue May 8, 2018 · 8 comments
Open

Loading a JPEG with precision set to 12 crashes all processes #1160

logicbomb421 opened this issue May 8, 2018 · 8 comments

Comments

@logicbomb421
Copy link

When attempting to set the src property of a Canvas.Image object with a Buffer, all processes (node-canvas, our user app, etc) crash if the buffer contains a JPEG image with precision set to 12 bits.

Interestingly, wrapping the src set in a try/catch does not catch the error. The only way to catch it is visually by running the process and watching the command line, which will produce the following:

< Unsupported JPEG data precision 12

I have also tried to set the img.onerror handler to get more context, however it seems this is never called before the process(es) crash.

Steps to Reproduce

const fileName = 'myImage.jpg';

require('fs').readFile(require('path').resolve(__dirname, fileName), (err, data) => {
  if (err) {
    console.log(err);
    process.exit(1);
  }

  const Image = require('canvas').Image;
  let img = new Image();
  img.src = data; // <-- this is the call that crashes everything
  
  console.log('success');
});

Your Environment

  • Version of node-canvas: 1.6.9
  • Environment: node 8.9.4 on Mac OSX 10.12.6
@zbjornson
Copy link
Collaborator

Can you post your 12-bit jpeg here please? I'm not finding one to test with by googling.

I think (a) we can prevent the process abort and instead trigger a nice error, but (b) actually handling 12-bit jpegs will require you (or prebuilds, if you're using them) to update your jpeg lib.

@Hakerh400
Copy link
Contributor

Tested on Windows 10 with versions 1.6.9 and 2.0.0 and git-master and I'm unable to reproduce this issue. The error callback is properly called and the error is caught.

The code I used:

var img = new Image();

img.onload = () => {
  console.log('ok');
};

img.onerror = err => {
  console.log('Error occured');
  throw err;
};

img.src = fs.readFileSync('test.jpg');

The 12-bit image I used: test.zip

@logicbomb421
Copy link
Author

@zbjornson here are a few sample images that cause the issue for us:
samples.zip

@Hakerh400 I ran the sample image provided through the example code also provided and am able to reproduce the same < Unsupported JPEG data precision 12 process-crashing behavior.

I believe @zbjornson hit the nail on the head in that we would need to update our jpeg lib to actually handle 12 bit precision, however I'd think this error shouldn't crash the process running it, correct?

@zbjornson
Copy link
Collaborator

@logicbomb421 Thanks. Can you also post the result of require("canvas").jpegVersion please?

@logicbomb421
Copy link
Author

@zbjornson that call returns 9c.

@zbjornson
Copy link
Collaborator

Interesting, that's the latest version. 9a added 12-bit color support.

Will try to look at this in the next few weeks.

@zbjornson zbjornson changed the title Loading a jpeg with precision set to 12 crashes all processes Loading a JPEG with precision set to 12 crashes all processes Aug 31, 2018
@Hakerh400
Copy link
Contributor

Interesting, that's the latest version. 9a added 12-bit color support.

It seems that 12-bit support is a build-time flag.
See comment 5 at chromium#869320

@zbjornson
Copy link
Collaborator

Good find.

But, oof: We do not support run-time selection of data precision, sorry. ... which is why the commenter in that bug said they'd have to bundle two builds of it.

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

No branches or pull requests

3 participants