Skip to content

Commit

Permalink
Fix handling invalid orientation in AutoRotateFilterLoader
Browse files Browse the repository at this point in the history
AutoRotateFilterLoader tried to throw an exception that didn't exist
when it had received an image with invalid EXIF orientation tag.

It also used to fall back to exif_read_data when an old Imagine was used
and it wasn't tested properly which caused breaking a build.

The test is somewhat tricky. I've added an asset with a tiny JPEG that
has an EXIF orientation tag. When an old imagine is used, test reads an
image into memory, resets the orientation byte and makes ImageInterface
mock respond with the modified image when raw JPEG is requested.
  • Loading branch information
kamazee committed Oct 31, 2015
1 parent 888b6c5 commit de1bf92
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 24 deletions.
12 changes: 7 additions & 5 deletions Imagine/Filter/Loader/AutoRotateFilterLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Liip\ImagineBundle\Imagine\Filter\Loader;

use Imagine\Image\ImageInterface;
use Imagine\Exception\InvalidArgumentException;

/**
* AutoRotateFilterLoader - rotates an Image based on its EXIF Data.
Expand All @@ -22,6 +23,12 @@ class AutoRotateFilterLoader implements LoaderInterface
public function load(ImageInterface $image, array $options = array())
{
if ($orientation = $this->getOrientation($image)) {
if ($orientation < 1 || $orientation > 8) {
throw new InvalidArgumentException(
sprintf('The image has wrong EXIF orientation tag (%d)', $orientation)
);
}

// Rotates if necessary.
$degree = $this->calculateRotation($orientation);
if ($degree !== 0) {
Expand Down Expand Up @@ -59,8 +66,6 @@ private function calculateRotation($orientation)
case 7:
case 8:
return -90;
default:
throw new Exception('Unhandled orientation');
}
}

Expand Down Expand Up @@ -112,9 +117,6 @@ private function isFlipped($orientation)
case 5:
case 7:
return true;

default:
throw new Exception('Unhandled orientation');
}
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
63 changes: 44 additions & 19 deletions Tests/Imagine/Filter/Loader/AutoRotateFilterLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Liip\ImagineBundle\Tests\Filter;

use Imagine\Image\ImageInterface;
use Liip\ImagineBundle\Imagine\Filter\Loader\AutoRotateFilterLoader;
use Liip\ImagineBundle\Tests\AbstractTest;

Expand All @@ -26,30 +27,43 @@ private function loadExif($exifValue, $expectCallRotateValue, $expectCallFlip)
{
$loader = new AutoRotateFilterLoader();

// Mocks the metadata and makes it return the expected exif value for the rotation.
// If $exifValue is null, it means the image doesn't contain any metadata.
$metaData = $this->getMockMetaData();
// Mocks the image and makes it use the fake meta data.
$image = $this->getMockImage();

$metaData
->expects($this->atLeastOnce())
->method('offsetGet')
->willReturn($exifValue);

if ($exifValue && $exifValue !== '1') {
if (method_exists(ImageInterface::class, 'metadata')) {
// Mocks the metadata and makes it return the expected exif value for the rotation.
// If $exifValue is null, it means the image doesn't contain any metadata.
$metaData = $this->getMockMetaData();

$metaData
->expects($this->atLeastOnce())
->method('offsetGet')
->willReturn($exifValue);

if ($exifValue && $exifValue > '1' && $exifValue <= 8) {
$metaData
->expects($this->once())
->method('offsetSet')
->with($this->orientationKey, '1');
}

$image
->expects($this->atLeastOnce())
->method('metadata')
->willReturn($metaData);
} else {
$jpg = file_get_contents(__DIR__.'/../../../Fixtures/assets/pixel_1x1_orientation_at_0x30.jpg');
// The byte with orientation is at offset 0x30 for this image
$jpg[0x30] = chr((int) $exifValue);

$image
->expects($this->once())
->method('offsetSet')
->with($this->orientationKey, '1');
->method('get')
->with('jpg')
->will($this->returnValue($jpg));
}

// Mocks the image and makes it use the fake meta data.
$image = $this->getMockImage();

$image
->expects($this->atLeastOnce())
->method('metadata')
->willReturn($metaData);

// Checks that rotate is called with $expectCallRotateValue, or not called at all if $expectCallRotateValue is null.
$image
->expects($expectCallRotateValue !== null ? $this->once() : $this->never())
Expand Down Expand Up @@ -138,7 +152,18 @@ public function testLoadExif7()
*/
public function testLoadExif8()
{
$this->loadExif('8', null - 90, false);
$this->loadExif('8', -90, false);
}

/**
* Theoretically Orientation is `short` (uint16), so it could be anything
* from [0; 65535].
*
* @expectedException \Imagine\Exception\InvalidArgumentException
*/
public function testLoadExifInvalid()
{
$this->loadExif('10', null, false);
}

/**
Expand Down

0 comments on commit de1bf92

Please sign in to comment.