https://commons.wikimedia.org/wiki/File:Winterville_Aerial_View_HRoe_2016.jpg does not thumbnail, doesn't have extracted metadata, has reported zero width/height.
Description
Description
Details
Details
Related Objects
Related Objects
Event Timeline
Comment Actions
The file contains lots of garbage null bytes between JPEG sections. This technically makes it an invalid JPEG file, but it seems that the normal behavior of all tools that handle JPEG files is to just ignore these bytes, or possibly display a warning; we shouldn't be failing on them.
Comment Actions
Change 316737 had a related patch set uploaded (by Bartosz Dziewoński):
JpegMetadataExtractor: Don't fail when garbage bytes are present between JPEG sections
Comment Actions
The patch above fixes out metadata extractor, but the same bug is present in PHP's function getimagesize(), which we also use. I think we'd have to fix it somewhere around here… https://github.com/php/php-src/blob/c8fe175c508e635ccf04b5c90913e1933e60280f/ext/standard/image.c#L475
Comment Actions
Change 316737 merged by jenkins-bot:
JpegMetadataExtractor: Don't fail when garbage bytes are present between JPEG sections
Comment Actions
Thumbnail generation is only failing because we can't extract the metadata (in particular, the width/height from getimagesize()).
(Note that a new version of the file was uploaded without the issue, you can still see the old one in file history.)
Comment Actions
A nice surprise, this is already fixed in PHP 5.6.26:
- https://bugs.php.net/bug.php?id=72278
- https://github.com/php/php-src/commit/82df4e263886a0da21a00c98189649287666ba5c
I guess I just need to port that fix to HHVM.
Comment Actions
My HHVM patch was merged. Now looking into getting it backported, and deployed in WMF production.
Comment Actions
No, @MoritzMuehlenhoff said we regularly upgrade to HHVM 3.12 point releases. Upstream said on the pull request that this'll be in the next one.
Comment Actions
Oh, 3.12.11 was just released 5 days ago, and it's indeed there (https://github.com/facebook/hhvm/commits/HHVM-3.12.11). So I guess it's a question for Moritz now, when are we planning to upgrade?
Comment Actions
@MoritzMuehlenhoff Do you know when we'll have HHVM 3.12.11 (or the patch https://github.com/facebook/hhvm/pull/7435) in production?
Comment Actions
At the moment we're running 3.12.10 plus security fixes from 3.12.11, but your the patch from https://github.com/facebook/hhvm/pull/7435 isn't present yet. We can add it the next time we upgrade HHVM.
Comment Actions
@matmarex : Our latest HHVM package (based on 3.12.11 plus additional patches) is now fully deployed across the production cluster.
Comment Actions
Thank you @MoritzMuehlenhoff, I can confirm that the original versions of the two problematic files now thumbnail correctly and have the right metadata shown. I have restored them both. For any other currently affected files, this will be resolved after we do T32961: Run refreshImageMetadata.php --force.