search

bolt / thumbs

Thumbnail handler for Bolt Images
12 stars 16 forks source link

Check the id instead to make sure exif rotation works #41

Closed SvanteRichter closed 7 years ago

SvanteRichter commented 7 years ago

Oh, and thanks to wd-freelancer on slack for finding this bug :)

CarsonF commented 7 years ago

Hmmm Travis says no but only for PHP 5.5/5.6...

SvanteRichter commented 7 years ago

Yeah, not sure why... Doesn't seem like there is any php7 specific about this to me.

GwendolenLynch commented 7 years ago

That PR branch passes locally here under PHP 5.5, 5.6, 7.0 & 7.1 … Travis had a meltdown with some core tests for a few hours recently, so I've restarted them as it'd be good to have green :wink:

GwendolenLynch commented 7 years ago

One thing I hadn't done, was a composer update on that repo … local fails in the same way here now too.

Package operations: 0 installs, 14 updates, 0 removals
  - Updating bolt/filesystem (2.1.1 => 2.2.1): Loading from cache
  - …
CarsonF commented 7 years ago

It's not using the project's phpunit

GwendolenLynch commented 7 years ago

It's not using the project's phpunit

I am

GwendolenLynch commented 7 years ago

On master now:

$ for b in php55 php56 php70 php ; do \
    PV=$($b -v) ; \
    echo "Running $PV" ; \
    echo "Command: $b vendor/bin/phpunit"; \
    echo; \
    $b vendor/bin/phpunit ; \
done 
Running PHP 5.5.38 (cli) (built: Feb 18 2017 07:37:49) 
Copyright (c) 1997-2015 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2015 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2015, by Zend Technologies
    with Xdebug v2.5.1, Copyright (c) 2002-2017, by Derick Rethans
    with blackfire v1.15.0~linux-x64-non_zts55, https://blackfire.io, by Blackfireio Inc.
Command: php55 vendor/bin/phpunit

PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

.......EE....................

Time: 2.3 seconds, Memory: 18.25MB

There were 2 errors:

1) Bolt\Thumbs\Tests\ControllerTest::testIsRestricted
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:58

2) Bolt\Thumbs\Tests\ControllerTest::testNotIsRestrictedWhenLoggedIn
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:103

FAILURES!
Tests: 29, Assertions: 31, Errors: 2.

Generating code coverage report in HTML format ... done
Running PHP 5.6.30 (cli) (built: Jan 19 2017 06:38:13) 
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies
    with Xdebug v2.5.1, Copyright (c) 2002-2017, by Derick Rethans
    with blackfire v1.15.0~linux-x64-non_zts56, https://blackfire.io, by Blackfireio Inc.
Command: php56 vendor/bin/phpunit

PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

.......EE....................

Time: 2.29 seconds, Memory: 18.25MB

There were 2 errors:

1) Bolt\Thumbs\Tests\ControllerTest::testIsRestricted
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:58

2) Bolt\Thumbs\Tests\ControllerTest::testNotIsRestrictedWhenLoggedIn
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:103

FAILURES!
Tests: 29, Assertions: 31, Errors: 2.

Generating code coverage report in HTML format ... done
Running PHP 7.0.17 (cli) (built: Mar 14 2017 13:33:16) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.0.17, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.5.1, Copyright (c) 2002-2017, by Derick Rethans
    with blackfire v1.15.0~linux-x64-non_zts70, https://blackfire.io, by Blackfireio Inc.
Command: php70 vendor/bin/phpunit

PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

.......EE....................

Time: 1.37 seconds, Memory: 12.00MB

There were 2 errors:

1) Bolt\Thumbs\Tests\ControllerTest::testIsRestricted
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:58

2) Bolt\Thumbs\Tests\ControllerTest::testNotIsRestrictedWhenLoggedIn
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:103

FAILURES!
Tests: 29, Assertions: 31, Errors: 2.

Generating code coverage report in HTML format ... done
Running PHP 7.1.3 (cli) (built: Mar 14 2017 17:02:59) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.1.3, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.5.1, Copyright (c) 2002-2017, by Derick Rethans
    with blackfire v1.15.0~linux-x64-non_zts71, https://blackfire.io, by Blackfireio Inc.
Command: php vendor/bin/phpunit

PHPUnit 4.8.35 by Sebastian Bergmann and contributors.

.......EE....................

Time: 1.39 seconds, Memory: 8.00MB

There were 2 errors:

1) Bolt\Thumbs\Tests\ControllerTest::testIsRestricted
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:58

2) Bolt\Thumbs\Tests\ControllerTest::testNotIsRestrictedWhenLoggedIn
Bolt\Filesystem\Exception\IOException: Failed to get image data from file

[snip]/vendor/bolt/filesystem/src/Handler/Image/Info.php:81
[snip]/vendor/bolt/filesystem/src/Adapter/Local.php:116
[snip]/vendor/bolt/filesystem/src/Filesystem.php:669
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:26
[snip]/vendor/bolt/filesystem/src/Handler/Image.php:39
[snip]/src/Response.php:91
[snip]/src/Response.php:35
[snip]/src/Controller.php:136
[snip]/src/Controller.php:81
[snip]/tests/ControllerTest.php:103

FAILURES!
Tests: 29, Assertions: 31, Errors: 2.

Generating code coverage report in HTML format ... done
bobdenotter commented 7 years ago

@GawainLynch Is this still something we need to fix (properly) for 3.3?

SvanteRichter commented 7 years ago

@bobdenotter I think this can go in as-is and will fix the issue, @GawainLynch said this on slack a few days after I opened it:

@sahassar @carson: There is no problem with that thumbs PR and FWIW can go in. I had a look yesterday and the test fails in master due to updates in bolt/filesystem, and had left that PR there as; a) the 3 of us were already focused on it; b) said discussion was there; c) I half expected Casrson to :facepalm: at something obvious for me :laughing:

Sorry for not commenting here, I almost forgot about it

GwendolenLynch commented 7 years ago

Yeah, the test failure exists in master too … I've spent a bit of time on it, and it looks to be coming from filesystem changes (test failure), my concern was that it was a sign of a bigger problem and have been trying to find time with @CarsonF to check both.

Basically, this PR is GTG, I (personally) hadn't merged it, so above wouldn't get forgotten.

bobdenotter commented 7 years ago

Opened #43, so we don't forget about this. Meanwhile: Merging in this issue! :shipit: