Bug 1520656 - BMPs from the clipboard may include extra padding. r=tnikkel a=lizzard
authorAndrew Osmond <aosmond@mozilla.com>
Thu, 14 Feb 2019 14:37:06 -0500
changeset 516055 109d0b170241595e2805fa209d4a6ff491d0ed93
parent 516054 690a467b10e1e36ddc6d234907122088de35e04e
child 516056 6ecdb201bbdb462f3255fd50bb9c6d4b5f1fe78d
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel, lizzard
bugs1520656
milestone66.0
Bug 1520656 - BMPs from the clipboard may include extra padding. r=tnikkel a=lizzard In the original Windows clipboard BMP decoder implementation in nsImageFromClipboard::ConvertColorBitMap, if the bitmap used bitfields compression, it always adjusted the offset to the RGB data by 12 bytes. It did this even for newer BMP header formats which explicitly include space for the bitfields in their header sizes. This patch updates our BMP decoder to do the same for clipboard BMPs, since we have observed pasted BMPs using bitfield compression appearing incorrectly. To the user this appears as if we read a color mask; completely red, blue, green pixels at the start of the last row, causing all of the other rows to start with the last three pixels of the previous row. Differential Revision: https://phabricator.services.mozilla.com/D19955
image/decoders/nsBMPDecoder.cpp
image/decoders/nsBMPDecoder.h
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -61,16 +61,26 @@
 //   another optional gap).
 //
 // WinBMPv3-ICO. This is a variant of WinBMPv3.
 // - It's the BMP format used for BMP images within ICO files.
 // - The only difference with WinBMPv3 is that if an image is 32bpp and has no
 //   compression, then instead of treating the pixel data as 0RGB it is treated
 //   as ARGB, but only if one or more of the A values are non-zero.
 //
+// Clipboard variants.
+// - It's the BMP format used for BMP images captured from the clipboard.
+// - It is missing the file header, containing the BM signature and the data
+//   offset. Instead the data begins after the header.
+// - If it uses BITFIELDS compression, then there is always an additional 12
+//   bytes of data after the header that must be read. In WinBMPv4+, the masks
+//   are supposed to be included in the header size, which are the values we use
+//   for decoding purposes, but there is additional three masks following the
+//   header which must be skipped to get to the pixel data.
+//
 // OS/2 VERSIONS OF THE BMP FORMAT
 // -------------------------------
 // OS2-BMPv1.
 // - Almost identical to WinBMPv2; the differences are basically ignorable.
 //
 // OS2-BMPv2.
 // - Similar to WinBMPv3.
 // - The info header is 64 bytes but can be reduced to as little as 16; any
@@ -152,40 +162,44 @@ static void Set4BitPixel(uint32_t*& aDec
   }
 }
 
 static mozilla::LazyLogModule sBMPLog("BMPDecoder");
 
 // The length of the mBIHSize field in the info header.
 static const uint32_t BIHSIZE_FIELD_LENGTH = 4;
 
-nsBMPDecoder::nsBMPDecoder(RasterImage* aImage, State aState, size_t aLength)
+nsBMPDecoder::nsBMPDecoder(RasterImage* aImage, State aState, size_t aLength,
+                           bool aForClipboard)
     : Decoder(aImage),
       mLexer(Transition::To(aState, aLength), Transition::TerminateSuccess()),
       mIsWithinICO(false),
+      mIsForClipboard(aForClipboard),
       mMayHaveTransparency(false),
       mDoesHaveTransparency(false),
       mNumColors(0),
       mColors(nullptr),
       mBytesPerColor(0),
       mPreGapLength(0),
       mPixelRowSize(0),
       mCurrentRow(0),
       mCurrentPos(0),
       mAbsoluteModeNumPixels(0) {}
 
 // Constructor for normal BMP files or from the clipboard.
 nsBMPDecoder::nsBMPDecoder(RasterImage* aImage, bool aForClipboard)
     : nsBMPDecoder(aImage,
-                   aForClipboard ? State::CLIPBOARD_HEADER : State::FILE_HEADER,
-                   aForClipboard ? BIHSIZE_FIELD_LENGTH : FILE_HEADER_LENGTH) {}
+                   aForClipboard ? State::INFO_HEADER_SIZE : State::FILE_HEADER,
+                   aForClipboard ? BIHSIZE_FIELD_LENGTH : FILE_HEADER_LENGTH,
+                   aForClipboard) {}
 
 // Constructor used for WinBMPv3-ICO files, which lack a file header.
 nsBMPDecoder::nsBMPDecoder(RasterImage* aImage, uint32_t aDataOffset)
-    : nsBMPDecoder(aImage, State::INFO_HEADER_SIZE, BIHSIZE_FIELD_LENGTH) {
+    : nsBMPDecoder(aImage, State::INFO_HEADER_SIZE, BIHSIZE_FIELD_LENGTH,
+                   /* aForClipboard */ false) {
   SetIsWithinICO();
 
   // Even though the file header isn't present in this case, the dataOffset
   // field is set as if it is, and so we must increment mPreGapLength
   // accordingly.
   mPreGapLength += FILE_HEADER_LENGTH;
 
   // This is the one piece of data we normally get from a BMP file header, so
@@ -399,18 +413,16 @@ LexerResult nsBMPDecoder::DoDecode(Sourc
                                    IResumable* aOnResume) {
   MOZ_ASSERT(!HasError(), "Shouldn't call DoDecode after error!");
 
   return mLexer.Lex(aIterator, aOnResume,
                     [=](State aState, const char* aData, size_t aLength) {
                       switch (aState) {
                         case State::FILE_HEADER:
                           return ReadFileHeader(aData, aLength);
-                        case State::CLIPBOARD_HEADER:
-                          return ReadClipboardHeader(aData, aLength);
                         case State::INFO_HEADER_SIZE:
                           return ReadInfoHeaderSize(aData, aLength);
                         case State::INFO_HEADER_REST:
                           return ReadInfoHeaderRest(aData, aLength);
                         case State::BITFIELDS:
                           return ReadBitfields(aData, aLength);
                         case State::COLOR_TABLE:
                           return ReadColorTable(aData, aLength);
@@ -443,23 +455,16 @@ LexerTransition<nsBMPDecoder::State> nsB
 
   // We ignore the filesize (aData + 2) and reserved (aData + 6) fields.
 
   mH.mDataOffset = LittleEndian::readUint32(aData + 10);
 
   return Transition::To(State::INFO_HEADER_SIZE, BIHSIZE_FIELD_LENGTH);
 }
 
-LexerTransition<nsBMPDecoder::State> nsBMPDecoder::ReadClipboardHeader(
-    const char* aData, size_t aLength) {
-  // With the clipboard, the data offset is the header length.
-  mH.mDataOffset = LittleEndian::readUint32(aData);
-  return ReadInfoHeaderSize(aData, aLength);
-}
-
 // We read the info header in two steps: (a) read the mBIHSize field to
 // determine how long the header is; (b) read the rest of the header.
 LexerTransition<nsBMPDecoder::State> nsBMPDecoder::ReadInfoHeaderSize(
     const char* aData, size_t aLength) {
   mPreGapLength += aLength;
 
   mH.mBIHSize = LittleEndian::readUint32(aData);
 
@@ -566,16 +571,23 @@ LexerTransition<nsBMPDecoder::State> nsB
 
   size_t bitFieldsLengthStillToRead = 0;
   if (mH.mCompression == Compression::BITFIELDS) {
     // Need to read bitfields.
     if (mH.mBIHSize >= InfoHeaderLength::WIN_V4) {
       // Bitfields are present in the info header, so we can read them
       // immediately.
       mBitFields.ReadFromHeader(aData + 36, /* aReadAlpha = */ true);
+
+      // If this came from the clipboard, then we know that even if the header
+      // explicitly includes the bitfield masks, we need to add an additional
+      // offset for the start of the RGB data.
+      if (mIsForClipboard) {
+        mH.mDataOffset += BitFields::LENGTH;
+      }
     } else {
       // Bitfields are present after the info header, so we will read them in
       // ReadBitfields().
       bitFieldsLengthStillToRead = BitFields::LENGTH;
     }
   } else if (mH.mBpp == 16) {
     // No bitfields specified; use the default 5-5-5 values.
     mBitFields.SetR5G5B5();
@@ -677,16 +689,22 @@ LexerTransition<nsBMPDecoder::State> nsB
   for (uint32_t i = 0; i < mNumColors; i++) {
     // The format is BGR or BGR0.
     mColors[i].mBlue = uint8_t(aData[0]);
     mColors[i].mGreen = uint8_t(aData[1]);
     mColors[i].mRed = uint8_t(aData[2]);
     aData += mBytesPerColor;
   }
 
+  // If we are decoding a BMP from the clipboard, we did not know the data
+  // offset in advance. It is just defined as after the header and color table.
+  if (mIsForClipboard) {
+    mH.mDataOffset += mPreGapLength;
+  }
+
   // We know how many bytes we've read so far (mPreGapLength) and we know the
   // offset of the pixel data (mH.mDataOffset), so we can determine the length
   // of the gap (possibly zero) between the color table and the pixel data.
   //
   // If the gap is negative the file must be malformed (e.g. mH.mDataOffset
   // points into the middle of the color palette instead of past the end) and
   // we give up.
   if (mPreGapLength > mH.mDataOffset) {
--- a/image/decoders/nsBMPDecoder.h
+++ b/image/decoders/nsBMPDecoder.h
@@ -149,17 +149,16 @@ class nsBMPDecoder : public Decoder {
   nsresult BeforeFinishInternal() override;
   nsresult FinishInternal() override;
 
  private:
   friend class DecoderFactory;
 
   enum class State {
     FILE_HEADER,
-    CLIPBOARD_HEADER,
     INFO_HEADER_SIZE,
     INFO_HEADER_REST,
     BITFIELDS,
     COLOR_TABLE,
     GAP,
     AFTER_GAP,
     PIXEL_ROW,
     RLE_SEGMENT,
@@ -169,26 +168,26 @@ class nsBMPDecoder : public Decoder {
 
   // This is the constructor used for normal and clipboard BMP images.
   explicit nsBMPDecoder(RasterImage* aImage, bool aForClipboard = false);
 
   // This is the constructor used for BMP resources in ICO images.
   nsBMPDecoder(RasterImage* aImage, uint32_t aDataOffset);
 
   // Helper constructor called by the other two.
-  nsBMPDecoder(RasterImage* aImage, State aState, size_t aLength);
+  nsBMPDecoder(RasterImage* aImage, State aState, size_t aLength,
+               bool aForClipboard);
 
   int32_t AbsoluteHeight() const { return abs(mH.mHeight); }
 
   uint32_t* RowBuffer();
 
   void FinishRow();
 
   LexerTransition<State> ReadFileHeader(const char* aData, size_t aLength);
-  LexerTransition<State> ReadClipboardHeader(const char* aData, size_t aLength);
   LexerTransition<State> ReadInfoHeaderSize(const char* aData, size_t aLength);
   LexerTransition<State> ReadInfoHeaderRest(const char* aData, size_t aLength);
   LexerTransition<State> ReadBitfields(const char* aData, size_t aLength);
   LexerTransition<State> ReadColorTable(const char* aData, size_t aLength);
   LexerTransition<State> SkipGap();
   LexerTransition<State> AfterGap();
   LexerTransition<State> ReadPixelRow(const char* aData);
   LexerTransition<State> ReadRLESegment(const char* aData);
@@ -197,16 +196,19 @@ class nsBMPDecoder : public Decoder {
 
   StreamingLexer<State> mLexer;
 
   bmp::Header mH;
 
   // If the BMP is within an ICO file our treatment of it differs slightly.
   bool mIsWithinICO;
 
+  // If the BMP decoded from the clipboard, we don't start with a data offset.
+  bool mIsForClipboard;
+
   bmp::BitFields mBitFields;
 
   // Might the image have transparency? Determined from the headers during
   // metadata decode. (Does not guarantee the image actually has transparency.)
   bool mMayHaveTransparency;
 
   // Does the image have transparency? Determined during full decoding, so only
   // use this after that has been completed.