Bug 1315554 - Part 1. Enforce the parent decoder size (ICO) for child decoders (BMP, PNG). r=tnikkel
☠☠ backed out by 8b52b9a7c5f8 ☠ ☠
authorAndrew Osmond <aosmond@mozilla.com>
Sat, 22 Jul 2017 00:14:58 -0400
changeset 419086 fd310390a64affc366cc85759a7624a5e3f9f239
parent 419085 8d7673f2fd2ffcc7abb23a8959f431381b9d6386
child 419087 e1eec63b920fe003676d38aca1303dc158646ebb
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1315554
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1315554 - Part 1. Enforce the parent decoder size (ICO) for child decoders (BMP, PNG). r=tnikkel
image/Decoder.cpp
image/Decoder.h
image/DecoderFactory.cpp
image/DecoderFactory.h
image/decoders/nsBMPDecoder.cpp
image/decoders/nsICODecoder.cpp
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -400,16 +400,24 @@ Decoder::PostSize(int32_t aWidth,
 {
   // Validate.
   MOZ_ASSERT(aWidth >= 0, "Width can't be negative!");
   MOZ_ASSERT(aHeight >= 0, "Height can't be negative!");
 
   // Set our intrinsic size.
   mImageMetadata.SetSize(aWidth, aHeight, aOrientation);
 
+  // Verify it is the expected size, if given. Note that this is only used by
+  // the ICO decoder for embedded image types, so only its subdecoders are
+  // required to handle failures in PostSize.
+  if (!IsExpectedSize()) {
+    PostError();
+    return;
+  }
+
   // Set our output size if it's not already set.
   if (!mOutputSize) {
     mOutputSize = Some(IntSize(aWidth, aHeight));
   }
 
   MOZ_ASSERT(mOutputSize->width <= aWidth && mOutputSize->height <= aHeight,
              "Output size will result in upscaling");
 
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -211,16 +211,33 @@ public:
 
   /**
    * @return either the size passed to SetOutputSize() or Nothing(), indicating
    * that SetOutputSize() was not explicitly called.
    */
   Maybe<gfx::IntSize> ExplicitOutputSize() const;
 
   /**
+   * Sets the expected image size of this decoder. Decoding will fail if this
+   * does not match.
+   */
+  void SetExpectedSize(const gfx::IntSize& aSize)
+  {
+    mExpectedSize.emplace(aSize);
+  }
+
+  /**
+   * Is the image size what was expected, if specified?
+   */
+  bool IsExpectedSize() const
+  {
+    return mExpectedSize.isNothing() || *mExpectedSize == Size();
+  }
+
+  /**
    * Set an iterator to the SourceBuffer which will feed data to this decoder.
    * This must always be called before calling Init(). (And only before Init().)
    *
    * XXX(seth): We should eliminate this method and pass a SourceBufferIterator
    * to the various decoder constructors instead.
    */
   void SetIterator(SourceBufferIterator&& aIterator)
   {
@@ -522,16 +539,17 @@ protected:
 
 private:
   RefPtr<RasterImage> mImage;
   Maybe<SourceBufferIterator> mIterator;
   RawAccessFrameRef mCurrentFrame;
   ImageMetadata mImageMetadata;
   gfx::IntRect mInvalidRect; // Tracks an invalidation region in the current frame.
   Maybe<gfx::IntSize> mOutputSize;  // The size of our output surface.
+  Maybe<gfx::IntSize> mExpectedSize; // The expected size of the image.
   Progress mProgress;
 
   uint32_t mFrameCount; // Number of frames, including anything in-progress
   FrameTimeout mLoopLength;  // Length of a single loop of this image.
   gfx::IntRect mFirstFrameRefreshArea;  // The area of the image that needs to
                                         // be invalidated when the animation loops.
 
   // Telemetry data for this decoder.
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -231,16 +231,17 @@ DecoderFactory::CreateMetadataDecoder(De
   RefPtr<IDecodingTask> task = new MetadataDecodingTask(WrapNotNull(decoder));
   return task.forget();
 }
 
 /* static */ already_AddRefed<Decoder>
 DecoderFactory::CreateDecoderForICOResource(DecoderType aType,
                                             NotNull<SourceBuffer*> aSourceBuffer,
                                             NotNull<nsICODecoder*> aICODecoder,
+                                            const Maybe<IntSize>& aExpectedSize,
                                             const Maybe<uint32_t>& aDataOffset
                                               /* = Nothing() */)
 {
   // Create the decoder.
   RefPtr<Decoder> decoder;
   switch (aType) {
     case DecoderType::BMP:
       MOZ_ASSERT(aDataOffset);
@@ -259,16 +260,19 @@ DecoderFactory::CreateDecoderForICOResou
 
   MOZ_ASSERT(decoder);
 
   // Initialize the decoder, copying settings from @aICODecoder.
   MOZ_ASSERT(!aICODecoder->IsMetadataDecode());
   decoder->SetMetadataDecode(aICODecoder->IsMetadataDecode());
   decoder->SetIterator(aSourceBuffer->Iterator());
   decoder->SetOutputSize(aICODecoder->OutputSize());
+  if (aExpectedSize) {
+    decoder->SetExpectedSize(*aExpectedSize);
+  }
   decoder->SetDecoderFlags(aICODecoder->GetDecoderFlags());
   decoder->SetSurfaceFlags(aICODecoder->GetSurfaceFlags());
   decoder->SetFinalizeFrames(false);
 
   if (NS_FAILED(decoder->Init())) {
     return nullptr;
   }
 
--- a/image/DecoderFactory.h
+++ b/image/DecoderFactory.h
@@ -121,24 +121,26 @@ public:
    *              PNG.
    * @param aSourceBuffer The SourceBuffer which the decoder will read its data
    *                      from.
    * @param aICODecoder The ICO decoder which is controlling this resource
    *                    decoder. @aICODecoder's settings will be copied to the
    *                    resource decoder, so the two decoders will have the
    *                    same decoder flags, surface flags, target size, and
    *                    other parameters.
+   * @param aExpectedSize The expected size of the resource from the ICO header.
    * @param aDataOffset If @aType is BMP, specifies the offset at which data
    *                    begins in the BMP resource. Must be Some() if and only
    *                    if @aType is BMP.
    */
   static already_AddRefed<Decoder>
   CreateDecoderForICOResource(DecoderType aType,
                               NotNull<SourceBuffer*> aSourceBuffer,
                               NotNull<nsICODecoder*> aICODecoder,
+                              const Maybe<gfx::IntSize>& aExpectedSize,
                               const Maybe<uint32_t>& aDataOffset = Nothing());
 
   /**
    * Creates and initializes an anonymous decoder (one which isn't associated
    * with an Image object). Only the first frame of the image will be decoded.
    *
    * @param aType Which type of decoder to create - JPEG, PNG, etc.
    * @param aSourceBuffer The SourceBuffer which the decoder will read its data
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -645,16 +645,19 @@ nsBMPDecoder::ReadBitfields(const char* 
     (mH.mCompression == Compression::BITFIELDS &&
      mBitFields.mAlpha.IsPresent());
   if (mMayHaveTransparency) {
     PostHasTransparency();
   }
 
   // Post our size to the superclass.
   PostSize(mH.mWidth, AbsoluteHeight());
+  if (HasError()) {
+    return Transition::TerminateFailure();
+  }
 
   // We've now read all the headers. If we're doing a metadata decode, we're
   // done.
   if (IsMetadataDecode()) {
     return Transition::TerminateSuccess();
   }
 
   // Set up the color table, if present; it'll be filled in by ReadColorTable().
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -276,16 +276,20 @@ nsICODecoder::ReadDirEntry(const char* a
       mImageMetadata.SetHotspot(mBiggestResourceHotSpot.width,
                                 mBiggestResourceHotSpot.height);
     }
 
     // We always report the biggest resource's size as the intrinsic size; this
     // is necessary for downscale-during-decode to work since we won't even
     // attempt to *upscale* while decoding.
     PostSize(mBiggestResourceSize.width, mBiggestResourceSize.height);
+    if (HasError()) {
+      return Transition::TerminateFailure();
+    }
+
     if (IsMetadataDecode()) {
       return Transition::TerminateSuccess();
     }
 
     // If the resource we selected matches the output size perfectly, we don't
     // need to do any downscaling.
     if (GetRealSize() == OutputSize()) {
       MOZ_ASSERT_IF(desiredSize, GetRealSize() == *desiredSize);
@@ -311,17 +315,18 @@ nsICODecoder::SniffResource(const char* 
                        PNGSIGNATURESIZE);
   if (isPNG) {
     // Create a PNG decoder which will do the rest of the work for us.
     mContainedSourceBuffer = new SourceBuffer();
     mContainedSourceBuffer->ExpectLength(mDirEntry.mBytesInRes);
     mContainedDecoder =
       DecoderFactory::CreateDecoderForICOResource(DecoderType::PNG,
                                                   WrapNotNull(mContainedSourceBuffer),
-                                                  WrapNotNull(this));
+                                                  WrapNotNull(this),
+                                                  Some(GetRealSize()));
 
     if (!WriteToContainedDecoder(aData, PNGSIGNATURESIZE)) {
       return Transition::TerminateFailure();
     }
 
     if (mDirEntry.mBytesInRes <= PNGSIGNATURESIZE) {
       return Transition::TerminateFailure();
     }
@@ -390,16 +395,17 @@ nsICODecoder::ReadBIH(const char* aData)
   // Create a BMP decoder which will do most of the work for us; the exception
   // is the AND mask, which isn't present in standalone BMPs.
   mContainedSourceBuffer = new SourceBuffer();
   mContainedSourceBuffer->ExpectLength(mDirEntry.mBytesInRes);
   mContainedDecoder =
     DecoderFactory::CreateDecoderForICOResource(DecoderType::BMP,
                                                 WrapNotNull(mContainedSourceBuffer),
                                                 WrapNotNull(this),
+                                                Some(GetRealSize()),
                                                 Some(dataOffset));
   RefPtr<nsBMPDecoder> bmpDecoder =
     static_cast<nsBMPDecoder*>(mContainedDecoder.get());
 
   // Verify that the BIH width and height values match the ICO directory entry,
   // and fix the BIH height value to compensate for the fact that the underlying
   // BMP decoder doesn't know about AND masks.
   if (!CheckAndFixBitmapSize(reinterpret_cast<int8_t*>(mBIHraw))) {