Skip to content

Commit

Permalink
Fix decoding error with trailing comment extension blocks
Browse files Browse the repository at this point in the history
The GIF files that can be found on the Internet come in a wide variety
of forms. Some strictly adhere to the original specification, others do
not and differ in the actual sequence of blocks or their number.

For this reason, my decoder has a kind of "virtual" FrameBlock, which
can contain all possible blocks in different order that occur in a GIF
animation.

- Graphic Control Extension
- Image Description
- Local Color Table
- Image Data Block
- Plain Text Extension
- Application Extension
- Comment Extension

The TableBasedImage block, which is a chain of ImageDescriptor, (Local
Color Table) and ImageData, is used as a feature for terminating a
FrameBlock.

So far I have only seen GIF files that follow this scheme. However,
there was one example where one (or more) comment extensions were added
before the end. Here the decoding process failed.

This fix also adds "global comments" that are not part of my FrameBlock
and are appended to the GifDataStream afterwards.
  • Loading branch information
olivervogel committed Feb 18, 2024
1 parent 1640943 commit cac9182
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 2 deletions.
13 changes: 12 additions & 1 deletion src/Decoders/GifDataStreamDecoder.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

namespace Intervention\Gif\Decoders;

use Intervention\Gif\AbstractExtension;
use Intervention\Gif\Blocks\ColorTable;
use Intervention\Gif\Blocks\CommentExtension;
use Intervention\Gif\Blocks\FrameBlock;
use Intervention\Gif\Blocks\Header;
use Intervention\Gif\Blocks\LogicalScreenDescriptor;
Expand Down Expand Up @@ -38,7 +40,16 @@ public function decode(): GifDataStream
}

while ($this->viewNextByte() != Trailer::MARKER) {
$gif->addFrame(FrameBlock::decode($this->handle));
match ($this->viewNextBytes(2)) {
// trailing "global" comment blocks which are not part of "FrameBlock"
AbstractExtension::MARKER . CommentExtension::LABEL
=> $gif->addComment(
CommentExtension::decode($this->handle)
),
default => $gif->addFrame(
FrameBlock::decode($this->handle)
),
};
}

return $gif;
Expand Down
13 changes: 13 additions & 0 deletions src/Encoders/GifDataStreamEncoder.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public function encode(): string
$this->source->getLogicalScreenDescriptor()->encode(),
$this->maybeEncodeGlobalColorTable(),
$this->encodeFrames(),
$this->encodeComments(),
$this->source->getTrailer()->encode(),
]);
}
Expand All @@ -54,4 +55,16 @@ protected function encodeFrames(): string
return $frame->encode();
}, $this->source->getFrames()));
}

/**
* Encode comment extension blocks of source
*
* @return string
*/
protected function encodeComments(): string
{
return implode('', array_map(function ($commentExtension) {
return $commentExtension->encode();
}, $this->source->getComments()));
}
}
27 changes: 26 additions & 1 deletion src/GifDataStream.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Intervention\Gif;

use Intervention\Gif\Blocks\ColorTable;
use Intervention\Gif\Blocks\CommentExtension;
use Intervention\Gif\Blocks\FrameBlock;
use Intervention\Gif\Blocks\Header;
use Intervention\Gif\Blocks\LogicalScreenDescriptor;
Expand All @@ -20,7 +21,8 @@ public function __construct(
protected Header $header = new Header(),
protected LogicalScreenDescriptor $logicalScreenDescriptor = new LogicalScreenDescriptor(),
protected ?ColorTable $globalColorTable = null,
protected array $frames = []
protected array $frames = [],
protected array $comments = []
) {
}

Expand Down Expand Up @@ -122,6 +124,16 @@ public function getFrames(): array
return $this->frames;
}

/**
* Return array of "global" comments
*
* @return array
*/
public function getComments(): array
{
return $this->comments;
}

/**
* Return first frame
*
Expand Down Expand Up @@ -149,6 +161,19 @@ public function addFrame(FrameBlock $frame): self
return $this;
}

/**
* Add frame
*
* @param FrameBlock $frame
* @return GifDataStream
*/
public function addComment(CommentExtension $comment): self

Check failure on line 170 in src/GifDataStream.php

View workflow job for this annotation

GitHub Actions / Testing on PHP 8.1

PHPDoc tag @param references unknown parameter: $frame

Check failure on line 170 in src/GifDataStream.php

View workflow job for this annotation

GitHub Actions / Testing on PHP 8.2

PHPDoc tag @param references unknown parameter: $frame

Check failure on line 170 in src/GifDataStream.php

View workflow job for this annotation

GitHub Actions / Testing on PHP 8.3

PHPDoc tag @param references unknown parameter: $frame
{
$this->comments[] = $comment;

return $this;
}

/**
* Set the current data
*
Expand Down
12 changes: 12 additions & 0 deletions tests/GifDataStreamTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,16 @@ public function testDecode()
}, $gif->getFrames()));
$this->assertEquals([0, 0, 0, 0, 0, 0, 0, 0], $sizes);
}

public function testDecodeTrailingComment(): void
{
$gif = GifDataStream::decode(
$this->getTestHandle(
file_get_contents(__DIR__ . '/images/animation_trailing_comment.gif')
),
);

$this->assertInstanceOf(GifDataStream::class, $gif);
$this->assertCount(1, $gif->getComments());
}
}
Binary file added tests/images/animation_trailing_comment.gif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit cac9182

Please sign in to comment.