Orientation for shared images broken #20484

Closed
sffax opened this Issue Nov 13, 2015 · 14 comments

Projects

None yet
@sffax
sffax commented Nov 13, 2015 edited

After upgrading ownCloud to version 8.1.4, shared images (including thumbnails) that require rotation are no longer being rotated in the UI. This is only happening when an account other than the owner of the image is accessing it. The owner can still see all the thumbnails and images with a correct orientation based on EXIF data.

Example of debug entries in the log when one of these shared images is accessed:

{"reqId":"8+4ddYHXwnHf3I\/TBn+e","remoteAddr":"192.168.1.2","app":"core","message":"Generating preview for \"\/shared folders\/photos\/Camera New (test)\/20151108_115921.jpg\" with \"OC\\Preview\\JPEG\"","level":0,"time":"November 12, 2015 07:06:22","method":"GET","url":"\/owncloud\/index.php\/core\/preview.png?file=%2Fshared+folders%2Fphotos%2FCamera+New+%28test%29%2F20151108_115921.jpg&c=3d7aac3fcadb160785927bd393424122&x=36&y=36&forceIcon=0"}    
{"reqId":"x5k0x0ZoC2s7vld9Qqa1","remoteAddr":"192.168.1.2","app":"core","message":"OC_Image->fixOrientation() No readable file path set.","level":0,"time":"November 12, 2015 07:06:22","method":"GET","url":"\/owncloud\/index.php\/core\/preview.png?file=%2Fshared+folders%2Fphotos%2FCamera+New+%28test%29%2F20151111_110734.jpg&c=0337a9dedd0e4906f82906be8fd4bf45&x=36&y=36&forceIcon=0"}
{"reqId":"8+4ddYHXwnHf3I\/TBn+e","remoteAddr":"192.168.1.2","app":"core","message":"OC_Image->fixOrientation() Orientation: -1","level":0,"time":"November 12, 2015 07:06:23","method":"GET","url":"\/owncloud\/index.php\/core\/preview.png?file=%2Fshared+folders%2Fphotos%2FCamera+New+%28test%29%2F20151108_115921.jpg&c=3d7aac3fcadb160785927bd393424122&x=36&y=36&forceIcon=0"}

The problem seems to be caused by a change in 'lib\private\preview\image.php'. Here's the diff between the code in 8.1.3 and 8.1.4:

49c49,50
<       if ($fileInfo['encrypted'] === true) {

---
>       $useTempFile = $fileInfo->isEncrypted() || !$fileInfo->getStorage()->isLocal();
>       if ($useTempFile) {
54a56,58
>       if ($useTempFile) {
>           unlink($fileName);
>       }

I have no external storage and no encryption configured on my server, but for some reason, the shared file path becomes something like '/tmp/oc_tmp_1SPFjp-.JPG', which causes the EXIF info lookup to fail. I didn't know what the right fix would be here since I'm not familiar with the "larger picture" but reverting to the code snippet above fixed my problem. Just raising this in case anyone else notices this issue. All this was happening on an otherwise unchanged CentOS 6 / Apache 2.2 / PHP 5.6 server with ownCloud running on it for over a year and a half without issues.

@karlitschek karlitschek added the bug label Nov 13, 2015
@MorrisJobke
Member

Ref #13448

@MorrisJobke
Member

cc @oparoz

@oparoz
Contributor
oparoz commented Nov 13, 2015

@icewind1991 made the change a281737

It would be good to know if this is also a problem on 8.2

@icewind1991
Member

The problem is due to the shared storage not implementing isLocal thus defaulting to false

Implementing isLocal for the shared storage isn't possible atm without major hacks due to the way the shared storages is implemented

@oparoz oparoz added the regression label Nov 16, 2015
@oparoz
Contributor
oparoz commented Nov 16, 2015

OK, so it's still happening on 8.2 and what would be the long term solution then and for which milestone?

@oparoz oparoz added the design label Nov 16, 2015
@icewind1991 icewind1991 was assigned by PVince81 Dec 3, 2015
@PVince81 PVince81 added this to the 9.0-current milestone Dec 3, 2015
@PVince81
Collaborator
PVince81 commented Dec 3, 2015

@icewind1991 what do you think ?

@fgenellini

sorry, there is a workaround for it to look good in the shared users?

@quassum
quassum commented Jan 11, 2016

Still having this problem on 8.2.2 when uploading pictures with an admin account into a folder shared by another user:

{"reqId":"VpOMWSUiPMMAAEYZa-cAAAAE","remoteAddr":"214.255.57.206","app":"core","message":"Generating preview for \"\/Photos\/Emily\/IMG_0526.JPG\" with \"OC\\Preview\\JPEG\"","level":0,"time":"2016-01-11T11:04:57+00:00","method":"GET","url":"\/cloud\/index.php\/core\/preview.png?file=%2FPhotos%2FEmily%2FIMG_0526.JPG&c=50477d876260f36725f9cecda1b6286c&x=32&y=32&forceIcon=0"}
{"reqId":"VpOMWSUiPMMAAEYZa-cAAAAE","remoteAddr":"214.255.57.206","app":"core","message":"OC_Image->fixOrientation() No readable file path set.","level":0,"time":"2016-01-11T11:04:58+00:00","method":"GET","url":"\/cloud\/index.php\/core\/preview.png?file=%2FPhotos%2FEmily%2FIMG_0526.JPG&c=50477d876260f36725f9cecda1b6286c&x=32&y=32&forceIcon=0"}
{"reqId":"VpOMWSUiPMMAAEYZa-cAAAAE","remoteAddr":"214.255.57.206","app":"core","message":"OC_Image->fixOrientation() Orientation: -1","level":0,"time":"2016-01-11T11:04:58+00:00","method":"GET","url":"\/cloud\/index.php\/core\/preview.png?file=%2FPhotos%2FEmily%2FIMG_0526.JPG&c=50477d876260f36725f9cecda1b6286c&x=32&y=32&forceIcon=0"}

When uploading the exact same file as the user that owns this share there isn't a problem at all and rotating works as expected:

{"reqId":"VpONAiUiPMMAAEaFCXsAAAAF","remoteAddr":"214.255.57.206","app":"core","message":"Generating preview for \"\/2015\/IMG_0526.JPG\" with \"OC\\Preview\\JPEG\"","level":0,"time":"2016-01-11T11:07:46+00:00","method":"GET","url":"\/cloud\/index.php\/core\/preview.png?file=%2F2015%2FIMG_0526.JPG&c=ec6c526d4fc71e502e486097f2a09e75&x=32&y=32&forceIcon=0"}
{"reqId":"VpONAiUiPMMAAEaFCXsAAAAF","remoteAddr":"214.255.57.206","app":"core","message":"OC_Image->fixOrientation() Orientation: 3","level":0,"time":"2016-01-11T11:07:47+00:00","method":"GET","url":"\/cloud\/index.php\/core\/preview.png?file=%2F2015%2FIMG_0526.JPG&c=ec6c526d4fc71e502e486097f2a09e75&x=32&y=32&forceIcon=0"}
@datenfalke

Same problem here - would be very happy if this could be solved. Thank you!

@icewind1991
Member

Fix is here: #21615

@datenfalke

Thank you!

@quassum
quassum commented Jan 15, 2016

Thanks, works beautifully!

@michaelstingl

@MorrisJobke Could this be backported to 8.2 ?

00005170

@PVince81
Collaborator

@michaelstingl backport #24000 was merged for 8.2.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment