[canvaskit] Fix GIF decode failure (#161536)

Fixes an error when decoding GIFs to check if they are animated. The
decoder needs to be more resilient in the face of Special Purpose blocks
that are in the stream in places not specified in the GIF89a spec.

Fixes flutter/flutter#161376

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

Author: harryterkelsen

engine/src/flutter/lib/web_ui/lib/src/engine/image_format_detector.dart

@@ -282,6 +282,7 @@ class _GifHeaderReader {
     int framesFound = 0;
     // Read the GIF until we either find 2 frames or reach the end of the GIF.
     while (true) {
+      _maybeSkipSpecialPurposeBlocks();
       final bool isTrailer = _checkForTrailer();
       if (isTrailer) {
         return framesFound > 1;
@@ -290,11 +291,7 @@ class _GifHeaderReader {
       // If we haven't reached the end, then the next block must either be a
       // graphic block or a special-purpose block (comment extension or
       // application extension).
-      final bool isSpecialPurposeBlock = _checkForSpecialPurposeBlock();
-      if (isSpecialPurposeBlock) {
-        _skipSpecialPurposeBlock();
-        continue;
-      }
+      _maybeSkipSpecialPurposeBlocks();
 
       // If the next block isn't a special-purpose block, it must be a graphic
       // block. Increase the frame count, skip the graphic block, and keep
@@ -322,8 +319,15 @@ class _GifHeaderReader {
     return nextByte == 0x3b;
   }
 
-  /// Returns [true] if the next block is a Special-Purpose Block (either a
-  /// Comment Extension or an Application Extension).
+  /// Skip Special Purpose Blocks (they do not effect decoding).
+  void _maybeSkipSpecialPurposeBlocks() {
+    while (_checkForSpecialPurposeBlock()) {
+      _skipSpecialPurposeBlock();
+    }
+  }
+
+  /// Returns [true] if the next block is a Special-Purpose Block (extension
+  /// label between 0xFA and 0xFF).
   bool _checkForSpecialPurposeBlock() {
     final int extensionIntroducer = bytes.getUint8(_position);
     if (extensionIntroducer != 0x21) {
@@ -332,9 +336,8 @@ class _GifHeaderReader {
 
     final int extensionLabel = bytes.getUint8(_position + 1);
 
-    // The Comment Extension label is 0xFE, the Application Extension Label is
-    // 0xFF.
-    return extensionLabel == 0xfe || extensionLabel == 0xff;
+    // A Special Purpose Block has a label between 0xFA and 0xFF.
+    return extensionLabel >= 0xfa && extensionLabel <= 0xff;
   }
 
   /// Skips past the current control block.
@@ -364,17 +367,21 @@ class _GifHeaderReader {
 
   /// Skip past the graphic block.
   void _skipGraphicBlock() {
+    _maybeSkipSpecialPurposeBlocks();
     // Check for the optional Graphic Control Extension.
     if (_checkForGraphicControlExtension()) {
       _skipGraphicControlExtension();
     }
 
+    _maybeSkipSpecialPurposeBlocks();
     // Check if the Graphic Block is a Plain Text Extension.
     if (_checkForPlainTextExtension()) {
       _skipPlainTextExtension();
       return;
     }
 
+    _maybeSkipSpecialPurposeBlocks();
+
     // This is a Table-Based Image block.
     assert(bytes.getUint8(_position) == 0x2c);
 

engine/src/flutter/lib/web_ui/test/engine/image_format_detector_test.dart

@@ -21,7 +21,8 @@ Future<void> testMain() async {
 
   Future<List<String>> createTestFiles() async {
     final HttpFetchResponse listingResponse = await httpFetch('/test_images/');
-    final List<String> testFiles = (await listingResponse.json() as List<dynamic>).cast<String>();
+    List<String> testFiles = (await listingResponse.json() as List<dynamic>).cast<String>();
+    testFiles = testFiles.map((String baseName) => '/test_images/$baseName').toList();
 
     // Sanity-check the test file list. If suddenly test files are moved or
     // deleted, and the test server returns an empty list, or is missing some
@@ -40,7 +41,7 @@ Future<void> testMain() async {
 
   for (final String testFile in testFiles!) {
     test('can detect image type of $testFile', () async {
-      final HttpFetchResponse response = await httpFetch('/test_images/$testFile');
+      final HttpFetchResponse response = await httpFetch(testFile);
 
       if (!response.hasPayload) {
         throw Exception('Unable to fetch() image test file "$testFile"');
@@ -50,23 +51,23 @@ Future<void> testMain() async {
 
       // WebP files which are known to be animated.
       const List<String> animatedWebpFiles = <String>[
-        'blendBG.webp',
-        'required.webp',
-        'stoplight_h.webp',
-        'stoplight.webp',
+        '/test_images/blendBG.webp',
+        '/test_images/required.webp',
+        '/test_images/stoplight_h.webp',
+        '/test_images/stoplight.webp',
       ];
 
       // GIF files which are known to be animated.
       const List<String> animatedGifFiles = <String>[
-        'alphabetAnim.gif',
-        'colorTables.gif',
-        'flightAnim.gif',
-        'gif-transparent-index.gif',
-        'randPixelsAnim.gif',
-        'randPixelsAnim2.gif',
-        'required.gif',
-        'test640x479.gif',
-        'xOffsetTooBig.gif',
+        '/test_images/alphabetAnim.gif',
+        '/test_images/colorTables.gif',
+        '/test_images/flightAnim.gif',
+        '/test_images/gif-transparent-index.gif',
+        '/test_images/randPixelsAnim.gif',
+        '/test_images/randPixelsAnim2.gif',
+        '/test_images/required.gif',
+        '/test_images/test640x479.gif',
+        '/test_images/xOffsetTooBig.gif',
       ];
 
       final String testFileExtension = testFile.substring(testFile.lastIndexOf('.') + 1);
@@ -84,4 +85,94 @@ Future<void> testMain() async {
       expect(detectImageType(responseBytes), expectedImageType);
     });
   }
+
+  test('can decode GIF with many nonstandard Special Purpose Blocks', () async {
+    expect(detectImageType(_createTestGif()), ImageType.animatedGif);
+  });
+}
+
+/// Generates a blank GIF to be used in tests.
+Uint8List _createTestGif({
+  int width = 1,
+  int height = 1,
+  int numFrames = 2,
+  bool includeManyCommentBlocks = true,
+}) {
+  final List<int> bytes = <int>[];
+  // Generate header.
+  bytes.addAll('GIF'.codeUnits);
+  bytes.addAll('89a'.codeUnits);
+
+  // Generate logical screen.
+  List<int> padInt(int x) {
+    assert(x >= 0 && x.bitLength <= 16);
+    if (x.bitLength > 8) {
+      return <int>[x >> 8, x & 0xff];
+    }
+    return <int>[0, x];
+  }
+
+  bytes.addAll(padInt(width));
+  bytes.addAll(padInt(height));
+  // Indicate there is no Global Color Table.
+  bytes.add(0x70);
+  bytes.add(0);
+  bytes.add(0);
+
+  // Generate data.
+  List<int> generateCommentBlock() {
+    final List<int> comment = <int>[];
+    comment.add(0x21);
+    comment.add(0xfe);
+    const String commentString = 'This is a comment';
+    comment.add(commentString.codeUnits.length);
+    comment.addAll(commentString.codeUnits);
+    comment.add(0);
+    return comment;
+  }
+
+  for (int i = 0; i < numFrames; i++) {
+    if (includeManyCommentBlocks) {
+      bytes.addAll(generateCommentBlock());
+    }
+    // Add a Graphic Control Extension block.
+    bytes.add(0x21);
+    bytes.add(0xf9);
+    bytes.add(4);
+    bytes.add(0);
+    // Indicate a delay of 1/10 of a second between frames.
+    bytes.add(0);
+    bytes.add(10);
+    bytes.add(0);
+    bytes.add(0);
+
+    if (includeManyCommentBlocks) {
+      bytes.addAll(generateCommentBlock());
+    }
+
+    // Add a Table-Based Image.
+    bytes.add(0x2c);
+    bytes.add(0);
+    bytes.add(0);
+    bytes.add(0);
+    bytes.add(0);
+    bytes.addAll(padInt(width));
+    bytes.addAll(padInt(height));
+    bytes.add(0);
+
+    bytes.add(0);
+    const String fakeImageData = 'This is an image';
+    bytes.add(fakeImageData.codeUnits.length);
+    bytes.addAll(fakeImageData.codeUnits);
+    bytes.add(0);
+  }
+
+  if (includeManyCommentBlocks) {
+    bytes.addAll(generateCommentBlock());
+  }
+
+  // Generate trailer.
+  bytes.add(0x3b);
+
+  return Uint8List.fromList(bytes);
 }