Webkit unable to show gifs with applcation extension string shorter than 11 bytes
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Mar 2013 18:27:07 +0000 (18:27 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Mar 2013 18:27:07 +0000 (18:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110620

Patch by Viatcheslav Ostapenko <sl.ostapenko@samsung.com> on 2013-03-12
Reviewed by Laszlo Gombos.

Source/WebCore:

Use actual block size for gifs application extension string even if it is below 11 bytes
to be able to decode this kind of gifs.

Test: fast/images/gif-short-app-extension-string.html

* platform/image-decoders/gif/GIFImageReader.cpp:
(GIFImageReader::decodeInternal):

LayoutTests:

Test that webkit is able to decode gifs with short application extension string.

* fast/images/gif-short-app-extension-string-expected.png: Added.
* fast/images/gif-short-app-extension-string-expected.txt: Added.
* fast/images/gif-short-app-extension-string.html: Added.
* fast/images/resources/short-app-extension-string.gif: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@145569 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/images/gif-short-app-extension-string-expected.png [new file with mode: 0644]
LayoutTests/fast/images/gif-short-app-extension-string-expected.txt [new file with mode: 0644]
LayoutTests/fast/images/gif-short-app-extension-string.html [new file with mode: 0644]
LayoutTests/fast/images/resources/short-app-extension-string.gif [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/image-decoders/gif/GIFImageReader.cpp

index 9f34e6f..457621d 100644 (file)
@@ -1,3 +1,17 @@
+2013-03-12  Viatcheslav Ostapenko  <sl.ostapenko@samsung.com>
+
+        Webkit unable to show gifs with applcation extension string shorter than 11 bytes
+        https://bugs.webkit.org/show_bug.cgi?id=110620
+
+        Reviewed by Laszlo Gombos.
+
+        Test that webkit is able to decode gifs with short application extension string.
+
+        * fast/images/gif-short-app-extension-string-expected.png: Added.
+        * fast/images/gif-short-app-extension-string-expected.txt: Added.
+        * fast/images/gif-short-app-extension-string.html: Added.
+        * fast/images/resources/short-app-extension-string.gif: Added.
+
 2013-03-12  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed GTK gardening.
diff --git a/LayoutTests/fast/images/gif-short-app-extension-string-expected.png b/LayoutTests/fast/images/gif-short-app-extension-string-expected.png
new file mode 100644 (file)
index 0000000..3104976
Binary files /dev/null and b/LayoutTests/fast/images/gif-short-app-extension-string-expected.png differ
diff --git a/LayoutTests/fast/images/gif-short-app-extension-string-expected.txt b/LayoutTests/fast/images/gif-short-app-extension-string-expected.txt
new file mode 100644 (file)
index 0000000..d765357
--- /dev/null
@@ -0,0 +1,7 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderImage {IMG} at (0,0) size 353x25
+      RenderText {#text} at (0,0) size 0x0
diff --git a/LayoutTests/fast/images/gif-short-app-extension-string.html b/LayoutTests/fast/images/gif-short-app-extension-string.html
new file mode 100644 (file)
index 0000000..7215c3a
--- /dev/null
@@ -0,0 +1,5 @@
+<html>
+<body>
+<img src="resources/short-app-extension-string.gif">
+</body>
+</html>
diff --git a/LayoutTests/fast/images/resources/short-app-extension-string.gif b/LayoutTests/fast/images/resources/short-app-extension-string.gif
new file mode 100644 (file)
index 0000000..70af95a
Binary files /dev/null and b/LayoutTests/fast/images/resources/short-app-extension-string.gif differ
index 9602468..48c1321 100644 (file)
@@ -1,3 +1,18 @@
+2013-03-12  Viatcheslav Ostapenko  <sl.ostapenko@samsung.com>
+
+        Webkit unable to show gifs with applcation extension string shorter than 11 bytes
+        https://bugs.webkit.org/show_bug.cgi?id=110620
+
+        Reviewed by Laszlo Gombos.
+
+        Use actual block size for gifs application extension string even if it is below 11 bytes 
+        to be able to decode this kind of gifs.
+
+        Test: fast/images/gif-short-app-extension-string.html
+
+        * platform/image-decoders/gif/GIFImageReader.cpp:
+        (GIFImageReader::decodeInternal):
+
 2013-03-12  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r145277.
index b87f6df..56c924b 100644 (file)
@@ -478,35 +478,28 @@ bool GIFImageReader::decodeInternal(size_t dataPosition, size_t len, GIFImageDec
             size_t bytesInBlock = currentComponent[1];
             GIFState es = GIFSkipBlock;
 
-            // The GIF spec mandates lengths for three of the extensions below.
-            // However, it's possible for GIFs in the wild to deviate. For example,
-            // some GIFs that embed ICC color profiles using GIFApplicationExtension
-            // violate the spec and treat this extension block like a sort of
-            // "extension + data" block, giving a size greater than 11 and filling the
-            // remaining bytes with data (then following with more data blocks as
-            // needed), instead of placing a true data block just after the 11 byte
-            // extension block.
-            //
-            // Accordingly, if the specified length is larger than the required value,
-            // we use it. If it's smaller, then we enforce the spec value, because the
-            // parsers for these extensions expect to have the specified number of
-            // bytes available, and if we don't ensure that, they could read off the
-            // end of the heap buffer. (In this case, it's likely the GIF is corrupt
-            // and we'll soon fail to decode anyway.)
             switch (*currentComponent) {
             case 0xf9:
                 es = GIFControlExtension;
+                // The GIF spec mandates that the GIFControlExtension header block length is 4 bytes,
+                // and the parser for this block reads 4 bytes, so we must enforce that the buffer
+                // contains at least this many bytes. If the GIF specifies a different length, we
+                // allow that, so long as it's larger; the additional data will simply be ignored.
                 bytesInBlock = std::max(bytesInBlock, static_cast<size_t>(4));
                 break;
 
+            // The GIF spec also specifies the lengths of the following two extensions' headers
+            // (as 12 and 11 bytes, respectively). Because we ignore the plain text extension entirely
+            // and sanity-check the actual length of the application extension header before reading it,
+            // we allow GIFs to deviate from these values in either direction. This is important for
+            // real-world compatibility, as GIFs in the wild exist with application extension headers
+            // that are both shorter and longer than 11 bytes.
             case 0x01:
                 // ignoring plain text extension
-                bytesInBlock = std::max(bytesInBlock, static_cast<size_t>(12));
                 break;
 
             case 0xff:
                 es = GIFApplicationExtension;
-                bytesInBlock = std::max(bytesInBlock, static_cast<size_t>(11));
                 break;
 
             case 0xfe:
@@ -578,7 +571,8 @@ bool GIFImageReader::decodeInternal(size_t dataPosition, size_t len, GIFImageDec
 
         case GIFApplicationExtension: {
             // Check for netscape application extension.
-            if (!strncmp((char*)currentComponent, "NETSCAPE2.0", 11) || !strncmp((char*)currentComponent, "ANIMEXTS1.0", 11))
+            if (m_bytesToConsume == 11 
+                && (!strncmp((char*)currentComponent, "NETSCAPE2.0", 11) || !strncmp((char*)currentComponent, "ANIMEXTS1.0", 11)))
                 GETN(1, GIFNetscapeExtensionBlock);
             else
                 GETN(1, GIFConsumeBlock);