# juce\_loadJPEGImageFromStream crashes

**URL:** <https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231>\
**Category:** General JUCE discussion\
**Created:** [October 29, 2007, 5:21pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231 "2007-10-29T17:21:37Z")\
**Posts on this page:** 17\
**Page:** 1

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 5:21pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/1 "2007-10-29T17:21:37Z")

</div>

Hi Jules,

juce\_loadJPEGImageFromStream currently ignores all jpeg decoding errors silentlty.

This is very problematic in our code, which can be told to decode jpeg images on possibly very, very bad input streams (read: mp3 picture frames 🙂 ).

When this happens, the jpglib routines continue execution on a very corrupted internal state (read: null pointers 🙂 )

Furthermore, from what I read in the full jpglib source and usage examples, it seems like ERREXIT should not be silently ignored and c client code should use setjmp/longjmp handlers to resume execution in user code. For us C++ folks, this amounts to exception throwing and handling.

The following code is a rather simple fix that does just that.  
It also contains a check that skips decoding of zero-sized images.

```auto
--- juce_JPEGLoaderBug/juce_JPEGLoader.cpp	Wed Oct 10 10:41:32 2007
+++ juce_JPEGLoaderBug/juce_JPEGLoader.patched.cpp	Mon Oct 29 18:09:45 2007
@@ -54,6 +54,20 @@

 //==============================================================================
+
+struct JPEGDecodingFailure {};
+
+void fatalErrorHandler (j_common_ptr cinfo)
+{
+ char errorBuffer [JMSG_LENGTH_MAX];
+ (*cinfo->err->format_message)(cinfo, errorBuffer);
+ jpeg_destroy(cinfo);
+
+ Logger::outputDebugString(String("JPEG decoding error: ") << errorBuffer);
+
+ throw JPEGDecodingFailure();
+}
+
 static void silentErrorCallback1 (j_common_ptr)
 {
 }
@@ -70,7 +84,7 @@
 {
     zerostruct (err);
 
- err.error_exit = silentErrorCallback1;
+ err.error_exit = fatalErrorHandler;
     err.emit_message = silentErrorCallback2;
     err.output_message = silentErrorCallback1;
     err.format_message = silentErrorCallback3;
@@ -124,12 +138,22 @@
         jpegDecompStruct.src->next_input_byte = (const unsigned char*) mb.getData();
         jpegDecompStruct.src->bytes_in_buffer = mb.getSize();
 
+ try
+ {
         jpeg_read_header (&jpegDecompStruct, TRUE);
+ }
+ catch (JPEGDecodingFailure&)
+ {
+ return image;
+ }
 
         jpeg_calc_output_dimensions (&jpegDecompStruct);
 
         const int width = jpegDecompStruct.output_width;
         const int height = jpegDecompStruct.output_height;
+
+ if (width * height == 0)
+ return image;
 
         jpegDecompStruct.out_color_space = JCS_RGB;
```

Would it be a problem to commit something along those lines to the trunk?

Thanks.

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 5:40pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/2 "2007-10-29T17:40:31Z")

</div>

I hit ``post’’ a little too fast ☹

The proper error handler setup should be:

```auto
    jpeg_std_error (&err);
    err.error_exit = fatalErrorHandler;
    err.emit_message = silentErrorCallback2;
    err.output_message = silentErrorCallback1;
    err.reset_error_mgr = silentErrorCallback1;
```

for the error message to be correctly formatted.

Also the catch clause should be pushed after the scope of the big if… since ERREXIT calls are scattered all over jpglib functions, not only the header decoding part.

Sorry for that.

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [October 29, 2007, 6:06pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/3 "2007-10-29T18:06:33Z")

</div>

That’s great - thanks for getting that working!

One thing that seems a pity is to leak the jpegDecompStruct, by not calling jpeg\_destroy\_decompress if it fails… do you know if it’s safe to still call that when it has gone wrong?

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 6:27pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/4 "2007-10-29T18:27:15Z")

</div>

It should not be leaked, indeed, but I’m pretty sure it’s not, since the libjpg code contains this:

```auto
GLOBAL(void)
jpeg_destroy_decompress (j_decompress_ptr cinfo)
{
  jpeg_destroy((j_common_ptr) cinfo); /* use common routine */
}
```

and the fatalErrorHandler function calls jpeg\_destroy.

Apparently, in libjpg, decoder and encoders are treated the same way, regarding dynamic allocations.

```auto
/* Routines that are to be used by both halves of the library are declared
 * to receive a pointer to this structure. There are no actual instances of
 * jpeg_common_struct, only of jpeg_compress_struct and jpeg_decompress_struct.
 */
struct jpeg_common_struct {
  jpeg_common_fields; /* Fields common to both master struct types */
  /* Additional fields follow in an actual jpeg_compress_struct or
   * jpeg_decompress_struct. All three structs must agree on these
   * initial fields! (This would be a lot cleaner in C++.)
   */
};

typedef struct jpeg_common_struct * j_common_ptr;
typedef struct jpeg_compress_struct * j_compress_ptr;
typedef struct jpeg_decompress_struct * j_decompress_ptr;
```

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 6:31pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/5 "2007-10-29T18:31:53Z")

</div>

Sorry, did not answer your other question.

The file ``example.c’’ from the official libjpg distribution contains this error handler:

```auto
METHODDEF(void)
my_error_exit (j_common_ptr cinfo)
{
  /* cinfo->err really points to a my_error_mgr struct, so coerce pointer */
  my_error_ptr myerr = (my_error_ptr) cinfo->err;

  /* Always display the message. */
  /* We could postpone this until after returning, if we chose. */
  (*cinfo->err->output_message) (cinfo);

  /* Return control to the setjmp point */
  longjmp(myerr->setjmp_buffer, 1);
}
```

and jumps to this point on return from longjmp:

```auto
  if (setjmp(jerr.setjmp_buffer)) {
    /* If we get here, the JPEG code has signaled an error.
     * We need to clean up the JPEG object, close the input file, and return.
     */
    jpeg_destroy_decompress(&cinfo);
    fclose(infile);
    return 0;
  }
```

So I guess it is safe to destroy the cinfo struct after decoding failed.

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [October 29, 2007, 6:32pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/6 "2007-10-29T18:32:48Z")

</div>

oh yes, sorry - I didn’t notice that call there.

One other thing - I’m not too bothered about getting the error message, and was trying to avoid code bloat by not linking to jerror.c if possible. How badly do you need to get that error string?

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 6:42pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/7 "2007-10-29T18:42:49Z")

</div>

I won’t care that much about the jpglib error messages, once I’ll be done with this attached picture frame mess 🙂

Anyway, would it harm to have it, at least in debug builds ?

The format\_message is only 40 lines long and does not depend on anything else but sprintf … maybe some copy/paste/reformat/preprocessor\_conditionals could avoid injecting the whole jerror.c code into juce in debug builds, and bypass it entirely in release ones.

Thanks very much for asking, anyway 🙂

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [October 29, 2007, 6:44pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/8 "2007-10-29T18:44:30Z")

</div>

It’s just that if you include code to get the message, your exe gets bloated with every possible error message, formatting functions, etc., which is a waste if it’s never needed.

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 6:50pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/9 "2007-10-29T18:50:53Z")

</div>

Yes, you’re right.

I did not see that JMESSAGE stuff and thought error messages were linked in anyway.

I guess I’ll be able to live without them after my next merge with the trunk 🙂

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [October 29, 2007, 6:55pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/10 "2007-10-29T18:55:05Z")

</div>

Ok, I’ve checked in a version now that should do the trick. I’ve not got any images that are broken enough to test it though!

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 7:01pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/11 "2007-10-29T19:01:24Z")

</div>

Thanks!

I’d be glad to provide 🙂

Do you want one such image?

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 29, 2007, 7:34pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/12 "2007-10-29T19:34:12Z")

</div>

Me again…

While torturing the jpeg decoder, it seems like I found another bug:

The decompStruct-\>src-\>bytes\_in\_buffer unsigned field is sometimes overflown by jpegSkip in juce\_JPEGLoader.cpp, on some of my trashy images.

This also makes libjpg code crash.

This is how I fixed it:

```auto
static void jpegSkip (j_decompress_ptr decompStruct, long num) throw()
{
    decompStruct->src->next_input_byte += num;

    const long clampedNum = jmin(num, long(decompStruct->src->bytes_in_buffer));
    decompStruct->src->bytes_in_buffer -= clampedNum;
}
```

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [October 29, 2007, 7:46pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/13 "2007-10-29T19:46:37Z")

</div>

Cool - thanks again!

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 30, 2007, 2:23pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/14 "2007-10-30T14:23:50Z")

</div>

Hi Jules,

I noticed the error-skipping behavior in the png loader as well…

You disabled png\_error and png\_chunk\_error.

This also makes the libpng decoding code crash on invalid input…

I solved this by modifying pngconf.h,

```auto
#define png_error(a, b) png_err(a)
#define png_chunk_error(a, b) png_err(a)
```

and used png\_set\_error\_fn and the same logic as in the jpeg code for the error handling.

There’s just a little more cleanup to do in the catch block.

And it stopped crashing.

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [October 30, 2007, 2:53pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/15 "2007-10-30T14:53:00Z")

</div>

ok, have a go of the code I’ve just checked in to see if it does the job…

---

<div class="post-metadata">

**Author:** ![grossebete](https://avatars.discourse-cdn.com/v4/letter/g/90db22/32.png) [@grossebete](https://forum.juce.com/u/grossebete)\
**Post date:** [October 30, 2007, 4:03pm UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/16 "2007-10-30T16:03:47Z")

</div>

Yup. Fits the bill 🙂

Thanks !

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [May 12, 2017, 10:06am UTC](https://forum.juce.com/t/juce-loadjpegimagefromstream-crashes/2231/17 "2017-05-12T10:06:13Z")

</div>