Fix bmp bug in ReadHeader()

src/codec/SkBmpCodec.cpp

Index: src/codec/SkBmpCodec.cpp
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 8d21e1dd8ee62820382f11f2c8e17c2dfc8ed54a..406a603dceefe3916714cb9cf394baefac094d00 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -457,10 +457,23 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
     // Calculate the number of bytes read so far
     const uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes;
     if (!inIco && offset < bytesRead) {
+        // TODO (msarett): Do we really want to fail if the offset in the header is invalid?
+        //                 Seems like we can just assume that the offset is zero and try to decode?
+        //                 Maybe we don't want to try to decode corrupt images?

[scroggo, 2015/09/01 21:31:25]

All good questions. I think we should support an implicit zero offset if this is
a common use case.

If the rest of our code is robust, getting some false positives here (i.e. we
assumed the offset was zero and the rest of the file was not valid) is probably
okay.

         SkCodecPrintf("Error: pixel data offset less than header size.\n");
         return false;
     }
 
+    // Skip to the start of the pixel array.
+    // We can do this here because there is no color table to read
+    // in bit mask mode.
+    if (!inIco && kBitMask_BmpInputFormat == inputFormat) {
+        if (stream->skip(offset - bytesRead) != offset - bytesRead) {
+            SkCodecPrintf("Error: unable to skip to image data.\n");
+            return false;
+        }
+    }
+
     if (codecOut) {
         // Set the image info
         const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
@@ -469,9 +482,8 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
         // Return the codec
         switch (inputFormat) {
             case kStandard_BmpInputFormat:
-                *codecOut =
-                        new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
-                                               bytesPerColor, offset - bytesRead, rowOrder, inIco);
+                *codecOut = new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
+                        bytesPerColor, offset - bytesRead, rowOrder, inIco);
                 return true;
             case kBitMask_BmpInputFormat:
                 // Bmp-in-Ico must be standard mode
@@ -479,16 +491,9 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
                     SkCodecPrintf("Error: Icos may not use bit mask format.\n");
                     return false;
                 }
-                // Skip to the start of the pixel array.
-                // We can do this here because there is no color table to read
-                // in bit mask mode.
-                if (stream->skip(offset - bytesRead) != offset - bytesRead) {
-                    SkCodecPrintf("Error: unable to skip to image data.\n");
-                    return false;
-                }
 
                 *codecOut = new SkBmpMaskCodec(imageInfo, stream, bitsPerPixel, masks.detach(),
-                                               rowOrder);
+                        rowOrder);
                 return true;
             case kRLE_BmpInputFormat:
                 // Bmp-in-Ico must be standard mode
@@ -496,9 +501,8 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
                 // require that RLE Bmps have a valid number of totalBytes, and
                 // Icos skip the header that contains totalBytes.
                 SkASSERT(!inIco);
-                *codecOut =
-                        new SkBmpRLECodec(imageInfo, stream, bitsPerPixel, numColors, bytesPerColor,
-                                          offset - bytesRead, rowOrder, RLEBytes);
+                *codecOut = new SkBmpRLECodec(imageInfo, stream, bitsPerPixel, numColors,
+                        bytesPerColor, offset - bytesRead, rowOrder, RLEBytes);
                 return true;
             default:
                 SkASSERT(false);