Fix bmp RLE "bug"

Chromium's test suite contains an RLE image that reports a certain
file size in the header, but then contains additional encoded data.
Our bmp decoder would only decode half of the image, before stopping.

With this fix, we check for additional data before returning
kIncompleteInput.

If this lands, I will upload the test image to the bots.

Also adding an invalid image test to CodexTest.

BUG=skia:

Review URL: https://codereview.chromium.org/1273853004

3 files changed, 75 insertions, 17 deletions

diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 5eb53544b5..fa4608ffee 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -478,6 +478,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
             case kBitMask_BmpInputFormat:
                 // Bmp-in-Ico must be standard mode
                 if (inIco) {
+                    SkCodecPrintf("Error: Icos may not use bit mask format.\n");
                     return false;
                 }
                 // Skip to the start of the pixel array.
@@ -493,9 +494,10 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
                 return true;
             case kRLE_BmpInputFormat:
                 // Bmp-in-Ico must be standard mode
-                if (inIco) {
-                    return false;
-                }
+                // When inIco is true, this line cannot be reached, since we
+                // require that RLE Bmps have a valid number of totalBytes, and
+                // Icos skip the header that contains totalBytes.
+                SkASSERT(!inIco);
                 *codecOut = SkNEW_ARGS(SkBmpRLECodec, (
                         imageInfo, stream, bitsPerPixel, numColors,
                         bytesPerColor, offset - bytesRead, rowOrder, RLEBytes));
@@ -553,7 +555,7 @@ void* SkBmpCodec::getDstStartRow(void* dst, size_t dstRowBytes, int32_t y) const
  */
 uint32_t SkBmpCodec::computeNumColors(uint32_t numColors) {
     // Zero is a default for maxColors
-    // Also set fNumColors to maxColors when it is too large
+    // Also set numColors to maxColors when it is too large
     uint32_t maxColors = 1 << fBitsPerPixel;
     if (numColors == 0 || numColors >= maxColors) {
         return maxColors;
diff --git a/src/codec/SkBmpRLECodec.cpp b/src/codec/SkBmpRLECodec.cpp
index 6be7449ed8..c71a5409d2 100644
--- a/src/codec/SkBmpRLECodec.cpp
+++ b/src/codec/SkBmpRLECodec.cpp
@@ -183,6 +183,41 @@ bool SkBmpRLECodec::initializeStreamBuffer() {
 }
 
 /*
+ * Before signalling kIncompleteInput, we should attempt to load the
+ * stream buffer with additional data.
+ *
+ * @return the number of bytes remaining in the stream buffer after
+ *         attempting to read more bytes from the stream
+ */
+size_t SkBmpRLECodec::checkForMoreData() {
+    const size_t remainingBytes = fRLEBytes - fCurrRLEByte;
+    uint8_t* buffer = fStreamBuffer.get();
+
+    // We will be reusing the same buffer, starting over from the beginning.
+    // Move any remaining bytes to the start of the buffer.
+    // We use memmove() instead of memcpy() because there is risk that the dst
+    // and src memory will overlap in corrupt images.
+    memmove(buffer, SkTAddOffset<uint8_t>(buffer, fCurrRLEByte), remainingBytes);
+
+    // Adjust the buffer ptr to the start of the unfilled data.
+    buffer += remainingBytes;
+
+    // Try to read additional bytes from the stream.  There are fCurrRLEByte
+    // bytes of additional space remaining in the buffer, assuming that we
+    // have already copied remainingBytes to the start of the buffer.
+    size_t additionalBytes = this->stream()->read(buffer, fCurrRLEByte);
+
+    // Update counters and return the number of bytes we currently have
+    // available.  We are at the start of the buffer again.
+    fCurrRLEByte = 0;
+    // If we were unable to fill the buffer, fRLEBytes is no longer equal to
+    // the size of the buffer.  There will be unused space at the end.  This
+    // should be fine, given that there are no more bytes in the stream.
+    fRLEBytes = remainingBytes + additionalBytes;
+    return fRLEBytes;
+}
+
+/*
  * Set an RLE pixel using the color table
  */
 void SkBmpRLECodec::setPixel(void* dst, size_t dstRowBytes,
@@ -287,8 +322,10 @@ SkCodec::Result SkBmpRLECodec::decode(const SkImageInfo& dstInfo,
 
         // Every entry takes at least two bytes
         if ((int) fRLEBytes - fCurrRLEByte < 2) {
-            SkCodecPrintf("Warning: incomplete RLE input.\n");
-            return kIncompleteInput;
+            SkCodecPrintf("Warning: might be incomplete RLE input.\n");
+            if (this->checkForMoreData() < 2) {
+                return kIncompleteInput;
+            }
         }
 
         // Read the next two bytes.  These bytes have different meanings
@@ -310,8 +347,10 @@ SkCodec::Result SkBmpRLECodec::decode(const SkImageInfo& dstInfo,
                 case RLE_DELTA: {
                     // Two bytes are needed to specify delta
                     if ((int) fRLEBytes - fCurrRLEByte < 2) {
-                        SkCodecPrintf("Warning: incomplete RLE input\n");
-                        return kIncompleteInput;
+                        SkCodecPrintf("Warning: might be incomplete RLE input.\n");
+                        if (this->checkForMoreData() < 2) {
+                            return kIncompleteInput;
+                        }
                     }
                     // Modify x and y
                     const uint8_t dx = fStreamBuffer.get()[fCurrRLEByte++];
@@ -319,8 +358,8 @@ SkCodec::Result SkBmpRLECodec::decode(const SkImageInfo& dstInfo,
                     x += dx;
                     y += dy;
                     if (x > width || y > height) {
-                        SkCodecPrintf("Warning: invalid RLE input 1.\n");
-                        return kIncompleteInput;
+                        SkCodecPrintf("Warning: invalid RLE input.\n");
+                        return kInvalidInput;
                     }
                     break;
                 }
@@ -333,12 +372,18 @@ SkCodec::Result SkBmpRLECodec::decode(const SkImageInfo& dstInfo,
                     const size_t rowBytes = compute_row_bytes(numPixels,
                             this->bitsPerPixel());
                     // Abort if setting numPixels moves us off the edge of the
-                    // image.  Also abort if there are not enough bytes
+                    // image.
+                    if (x + numPixels > width) {
+                        SkCodecPrintf("Warning: invalid RLE input.\n");
+                        return kInvalidInput;
+                    }
+                    // Also abort if there are not enough bytes
                     // remaining in the stream to set numPixels.
-                    if (x + numPixels > width ||
-                            (int) fRLEBytes - fCurrRLEByte < SkAlign2(rowBytes)) {
-                        SkCodecPrintf("Warning: invalid RLE input 2.\n");
-                        return kIncompleteInput;
+                    if ((int) fRLEBytes - fCurrRLEByte < SkAlign2(rowBytes)) {
+                        SkCodecPrintf("Warning: might be incomplete RLE input.\n");
+                        if (this->checkForMoreData() < SkAlign2(rowBytes)) {
+                            return kIncompleteInput;
+                        }
                     }
                     // Set numPixels number of pixels
                     while (numPixels > 0) {
@@ -394,8 +439,10 @@ SkCodec::Result SkBmpRLECodec::decode(const SkImageInfo& dstInfo,
                 // There are two more required bytes to finish encoding the
                 // color.
                 if ((int) fRLEBytes - fCurrRLEByte < 2) {
-                    SkCodecPrintf("Warning: incomplete RLE input\n");
-                    return kIncompleteInput;
+                    SkCodecPrintf("Warning: might be incomplete RLE input.\n");
+                    if (this->checkForMoreData() < 2) {
+                        return kIncompleteInput;
+                    }
                 }
 
                 // Fill the pixels up to endX with the specified color
diff --git a/src/codec/SkBmpRLECodec.h b/src/codec/SkBmpRLECodec.h
index f1f1ae97d8..ee8989be64 100644
--- a/src/codec/SkBmpRLECodec.h
+++ b/src/codec/SkBmpRLECodec.h
@@ -56,6 +56,15 @@ private:
     bool initializeStreamBuffer();
 
     /*
+     * Before signalling kIncompleteInput, we should attempt to load the
+     * stream buffer with additional data.
+     *
+     * @return the number of bytes remaining in the stream buffer after
+     *         attempting to read more bytes from the stream
+     */
+    size_t checkForMoreData();
+
+    /*
      * Set an RLE pixel using the color table
      */
     void setPixel(void* dst, size_t dstRowBytes,