Make Jpeg decoding more fault resistant.

src/images/SkImageDecoder_libjpeg.cpp

 /*
  * Copyright 2007 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #include "SkImageDecoder.h"
 #include "SkImageEncoder.h"
@@ skipping 266 matching lines @@
         if (1 != row_count) {
             return false;
         }
     }
     return true;
 }
 #endif
 
 // This guy exists just to aid in debugging, as it allows debuggers to just
 // set a break-point in one place to see all error exists.
-static bool return_false(const jpeg_decompress_struct& cinfo,
-                         const SkBitmap& bm, const char msg[]) {
-#ifdef SK_DEBUG
+static void error_message(const jpeg_decompress_struct& cinfo,
+                          const SkBitmap& bm, const char msg[]) {
+#if defined(SK_DEBUG) && defined(SK_DEBUG_JPEG_ERRORS)

[scroggo, 2013/09/24 23:01:07]

Like I said in the bug, I think this is a bad idea. This will result in hiding
errors.

     SkDebugf("libjpeg error %d <%s> from %s [%d %d]\n", cinfo.err->msg_code,
              cinfo.err->jpeg_message_table[cinfo.err->msg_code], msg,
              bm.width(), bm.height());
 #endif
+}
+
+
+static bool return_false(const jpeg_decompress_struct& cinfo,
+                         const SkBitmap& bm, const char msg[]) {
+    error_message(cinfo, bm, msg);
     return false;   // must always return false
 }
 
 // Convert a scanline of CMYK samples to RGBX in place. Note that this
 // method moves the "scanline" pointer in its processing
 static void convert_CMYK_to_RGB(uint8_t* scanline, unsigned int width) {
     // At this point we've received CMYK pixels from libjpeg. We
     // perform a crude conversion to RGB (based on the formulae
     // from easyrgb.com):
     //  CMYK -> CMY
@@ skipping 244 matching lines @@
                 cinfo.out_color_space == JCS_RGB_565)))
     {
         JSAMPLE* rowptr = (JSAMPLE*)bm->getPixels();
         INT32 const bpr =  bm->rowBytes();
 
         while (cinfo.output_scanline < cinfo.output_height) {
             int row_count = jpeg_read_scanlines(&cinfo, &rowptr, 1);
             // if row_count == 0, then we didn't get a scanline, so abort.
             // if we supported partial images, we might return true in this case
             if (0 == row_count) {
-                return return_false(cinfo, *bm, "read_scanlines");
+                error_message(cinfo, *bm, "read_scanlines");

[scroggo, 2013/09/24 23:01:07]

I think this should not print an error message at all, since we're going to
consider this a successful decode.

+                return true;

[scroggo, 2013/09/24 23:01:07]

Since we never initialized the memory, won't this result in garbage on all the
following scanlines?

             }
             if (this->shouldCancelDecode()) {
                 return return_false(cinfo, *bm, "shouldCancelDecode");
             }
             rowptr += bpr;
         }
         jpeg_finish_decompress(&cinfo);
         return true;
     }
 #endif
@@ skipping 29 matching lines @@
     //  Possibly skip initial rows [sampler.srcY0]
     if (!skip_src_rows(&cinfo, srcRow, sampler.srcY0())) {
         return return_false(cinfo, *bm, "skip rows");
     }
 
     // now loop through scanlines until y == bm->height() - 1
     for (int y = 0;; y++) {
         JSAMPLE* rowptr = (JSAMPLE*)srcRow;
         int row_count = jpeg_read_scanlines(&cinfo, &rowptr, 1);
         if (0 == row_count) {
-            return return_false(cinfo, *bm, "read_scanlines");
+            error_message(cinfo, *bm, "read_scanlines");
+            return true;
         }
         if (this->shouldCancelDecode()) {
             return return_false(cinfo, *bm, "shouldCancelDecode");
         }
 
         if (JCS_CMYK == cinfo.out_color_space) {
             convert_CMYK_to_RGB(srcRow, cinfo.output_width);
         }
 
         sampler.next(srcRow);
@@ skipping 161 matching lines @@
         INT32 const bpr = bitmap.rowBytes();
         int rowTotalCount = 0;
 
         while (rowTotalCount < height) {
             int rowCount = jpeg_read_tile_scanline(cinfo,
                                                    fImageIndex->huffmanIndex(),
                                                    &rowptr);
             // if row_count == 0, then we didn't get a scanline, so abort.
             // if we supported partial images, we might return true in this case
             if (0 == rowCount) {
-                return return_false(*cinfo, bitmap, "read_scanlines");
+                error_message(*cinfo, bitmap, "read_scanlines");
+                return true;
             }
             if (this->shouldCancelDecode()) {
                 return return_false(*cinfo, bitmap, "shouldCancelDecode");
             }
             rowTotalCount += rowCount;
             rowptr += bpr;
         }
 
         if (swapOnly) {
             bm->swap(bitmap);
@@ skipping 36 matching lines @@
     //  Possibly skip initial rows [sampler.srcY0]
     if (!skip_src_rows_tile(cinfo, fImageIndex->huffmanIndex(), srcRow, sampler.srcY0())) {
         return return_false(*cinfo, bitmap, "skip rows");
     }
 
     // now loop through scanlines until y == bitmap->height() - 1
     for (int y = 0;; y++) {
         JSAMPLE* rowptr = (JSAMPLE*)srcRow;
         int row_count = jpeg_read_tile_scanline(cinfo, fImageIndex->huffmanIndex(), &rowptr);
         if (0 == row_count) {
-            return return_false(*cinfo, bitmap, "read_scanlines");
+            error_message(*cinfo, bitmap, "read_scanlines");
+            return true;
         }
         if (this->shouldCancelDecode()) {
             return return_false(*cinfo, bitmap, "shouldCancelDecode");
         }
 
         if (JCS_CMYK == cinfo->out_color_space) {
             convert_CMYK_to_RGB(srcRow, width);
         }
 
         sampler.next(srcRow);
@@ skipping 293 matching lines @@
 }
 
 static SkImageEncoder* sk_libjpeg_efactory(SkImageEncoder::Type t) {
     return (SkImageEncoder::kJPEG_Type == t) ? SkNEW(SkJPEGImageEncoder) : NULL;
 }
 
 
 static SkImageDecoder_DecodeReg gDReg(sk_libjpeg_dfactory);
 static SkImageDecoder_FormatReg gFormatReg(get_format_jpeg);
 static SkImageEncoder_EncodeReg gEReg(sk_libjpeg_efactory);