Do you want to open a pull request?

karlitschek on 29 Aug 2015

I don't fully understand how everything in OC_Image works together and there seem to be some more issues to get fixOrientation working. Commit 88db5a365b53e713fe31bcc032fc22e48a56accc sets the imagetype work loadFromData() and loadFromBase64(). It's still missing for load() if it gets passed a resource.
With this commit, fixOrientation fails with "OC_Image->fixOrientation() No readable file path set" if I upload a jpg into a contact. As it doesn't come from a file it looks like a more general issue here how to handle images loaded from other sources than files.
So I think someone who has a better understanding of that should have a look at this as further changes may have implications I don't see...

gvde on 29 Aug 2015

@georgehrke @oparoz

PVince81 on 30 Aug 2015

@gvde The first step would be to list all the methods in image which require a proper imagetype to be set and then the best thing to do is to provide a PR so that we can test various scenarios. The unit/integration tests of Preview should give us a first indication of if there are side-effects.

oparoz on 4 Oct 2015

OK, quick assessment.

Imagetype is only set when an image is loaded from file

Affected methods:

getOrientation() Should break orientation support
preciseResize() Should break transparency support
centerCrop()Should break transparency support
crop()Should break transparency support

So, it would be good to get a PR which also includes setting the imagetype for resources-

oparoz on 4 Oct 2015

Please try again with 9.1.4 or 10.0 beta

PVince81 on 6 Apr 2017

Has this issue been fixed in 10.0.9.5? Reason for asking is that EXIF based image rotation is failing for all JPG images in my vanilla 10.0.9.5 installation.
Looking at the code, it seems that fixOrientation is failing because loadFromFileHandle() does not set the imageType but I'm not an expert.

Relevant log file lines...

{"reqId":"BYsRLO4mFkPyHhjYrTDl","level":0,"time":"2018-08-22T20:03:50+02:00","remoteAddr":"172.16.172.83","user":"admin","app":"core","method":"GET","url":"\/owncloud\/index.php\/apps\/gallery\/thumbnails?ids=16&scale=2&square=0&requesttoken=EgooBxMNRR5vUggLO0thImBAA18VGDNhYC0iUhMoeQY%3D%3AuPDdC8tSDkPZbyTRStInvRW43iFfPfOgBf967XtJVkM%3D","message":"Generating preview for \"\/admin\/files\/IMG_1928.JPG\" with \"OC\\Preview\\JPEG\""} {"reqId":"BYsRLO4mFkPyHhjYrTDl","level":0,"time":"2018-08-22T20:03:50+02:00","remoteAddr":"172.16.172.83","user":"admin","app":"core","method":"GET","url":"\/owncloud\/index.php\/apps\/gallery\/thumbnails?ids=16&scale=2&square=0&requesttoken=EgooBxMNRR5vUggLO0thImBAA18VGDNhYC0iUhMoeQY%3D%3AuPDdC8tSDkPZbyTRStInvRW43iFfPfOgBf967XtJVkM%3D","message":"OC_Image->fixOrientation() Image is not a JPEG."} {"reqId":"BYsRLO4mFkPyHhjYrTDl","level":0,"time":"2018-08-22T20:03:50+02:00","remoteAddr":"172.16.172.83","user":"admin","app":"core","method":"GET","url":"\/owncloud\/index.php\/apps\/gallery\/thumbnails?ids=16&scale=2&square=0&requesttoken=EgooBxMNRR5vUggLO0thImBAA18VGDNhYC0iUhMoeQY%3D%3AuPDdC8tSDkPZbyTRStInvRW43iFfPfOgBf967XtJVkM%3D","message":"OC_Image->fixOrientation() Orientation: -1"}

zero0cool0 on 22 Aug 2018

do you have more details about setup and reproduction steps ? see https://raw.githubusercontent.com/owncloud/core/master/.github/issue_template.md

PVince81 on 23 Aug 2018

Steps to reproduce

Install owncloud 10.0.9.5 from scratch
Upload a JPG image which contains EXIF orientation information other than 0°, like the one attached. It doesn't matter if whether through a sync-client or through the web interface.

$ exiftool-5.26 IMG_1928.JPG
File Name : IMG_1928.JPG
File Type : JPEG
Orientation : Rotate 180
X Resolution : 72
Y Resolution : 72

Expected behaviour

The thumbnail in the web interface as well as the preview produced by the gallery app should be rotated correctly.

Actual behaviour

JPG images are not rotated at all and appear upside down or rotated by 90° in either direction. I did a very rough call backtrace and see that upon thumbnail generation, fixOrientation() is called by loadFromFileHandle() which does not set the imageType variable like loadFromFile() does, hence fixOrientation() will assume it to be PNG file (the default set in the class) and exit with -1.

Server configuration

Operating system:
Ubuntu 18.04.1

Web server:
Apache 2 bundled with Ubuntu

Database:
MariaDB

PHP version:
7.2.7

ownCloud version: (see ownCloud admin page)
10.0.9.5

Updated from an older ownCloud or fresh install:
Fresh install

Where did you install ownCloud from:
# cat /etc/apt/sources.list.d/owncloud.list deb https://download.owncloud.org/download/repositories/stable/Ubuntu_18.04/ /

Signing status (ownCloud 9.0 and above):

Technical information

The following list covers which files have failed the integrity check. Please read
the previous linked documentation to learn more about the errors and how to fix
them.

Results

market
- EXTRA_FILE
  - README.md

Raw output

Array
(
[market] => Array
(
[EXTRA_FILE] => Array
(
[README.md] => Array
(
[expected] =>
[current] => b53ebb407cb90a5cf9e28463013f38e6aec3d39b4281187366e02b5fd014be144216a088fc50851635acc50cb71c92ad2407b21e8a4f3433b0f73a32ea066c97
)

            )

    )

)

The content of config/config.php:

{
"system": {
"loglevel": 0,
"updatechecker": false,
"instanceid": "ocdp4pbe74dr",
"passwordsalt": "REMOVED SENSITIVE VALUE",
"secret": "REMOVED SENSITIVE VALUE",
"trusted_domains": [
"REMOVED SENSITIVE VALUE"
],
"datadirectory": "\/Data\/OwnCloud-Data",
"overwrite.cli.url": "REMOVED SENSITIVE VALUE",
"dbtype": "mysql",
"dbname": "owncloud",
"dbuser": "REMOVED SENSITIVE VALUE",
"dbpassword": "REMOVED SENSITIVE VALUE",
"dbhost": "localhost",
"dbtableprefix": "oc_",
"version": "10.0.9.5",
"logtimezone": "Europe\/Zurich",
"installed": true,
"mail_from_address": "REMOVED SENSITIVE VALUE",
"mail_smtpmode": "php",
"mail_domain": "REMOVED SENSITIVE VALUE",
"appstore.experimental.enabled": false,
"redis": {
"host": "localhost",
"port": 6379
},
"user_backends": [
{
"class": "OC_User_SMB",
"arguments": [
"localhost"
]
}
],
"enabledPreviewProviders": [
"OC\Preview\PNG",
"OC\Preview\JPEG",
"OC\Preview\GIF",
"OC\Preview\BMP",
"OC\Preview\XBitmap",
"OC\Preview\MP3",
"OC\Preview\TXT",
"OC\Preview\MarkDown",
"OC\Preview\Movie",
"OC\Preview\PDF",
"OC\Preview\Photoshop",
"OC\Preview\Postscript",
"OC\Preview\StarOffice",
"OC\Preview\SVG",
"OC\Preview\TIFF",
"OC\Preview\Font"
],
"theme": "",
"maintenance": false,
"memcache.local": "\OC\Memcache\APCu",
"memcache.distributed": "\OC\Memcache\Redis",
"memcache.locking": "\OC\Memcache\Redis",
"mail_smtpauthtype": "PLAIN",
"mail_smtpauth": 1,
"mail_smtpsecure": "tls",
"mail_smtphost": "REMOVED SENSITIVE VALUE",
"mail_smtpname": "REMOVED SENSITIVE VALUE",
"mail_smtppassword": "REMOVED SENSITIVE VALUE",
"mail_smtpport": "587",
"log_rotate_size": "10485760"
}
}

List of activated apps:

`
Enabled:

activity: 2.3.7
audioplayer: 2.3.1
comments: 0.3.0
configreport: 0.1.1
dav: 0.3.2
duo: 2.5.1
federatedfilesharing: 0.3.1
federation: 0.1.0
files: 1.5.1
files_external: 0.7.1
files_pdfviewer: 0.9.0
files_sharing: 0.10.1
files_texteditor: 2.2.1
files_trashbin: 0.9.1
files_versions: 1.3.0
files_videoplayer: 0.9.8
firstrunwizard: 1.1
gallery: 16.1.0
market: 0.2.4
notifications: 0.3.4
provisioning_api: 0.5.0
systemtags: 0.3.0
templateeditor: 0.3.1
updatenotification: 0.2.1
user_external: 0.4
Disabled:
encryption
external
`

Are you using external storage, if yes which one: local/smb/sftp/...
none

Are you using encryption: yes/no
no

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...
no

Client configuration

Browser:
Chrome Version 68.0.3440.106 (Official Build) (64-bit)

Operating system:
MAC OS 10.13.6

Logs

{"reqId":"BYsRLO4mFkPyHhjYrTDl","level":0,"time":"2018-08-22T20:03:50+02:00","remoteAddr":"172.16.172.83","user":"admin","app":"core","method":"GET","url":"\/owncloud\/index.php\/apps\/gallery\/thumbnails?ids=16&scale=2&square=0&requesttoken=EgooBxMNRR5vUggLO0thImBAA18VGDNhYC0iUhMoeQY%3D%3AuPDdC8tSDkPZbyTRStInvRW43iFfPfOgBf967XtJVkM%3D","message":"Generating preview for \"\/admin\/files\/IMG_1928.JPG\" with \"OC\\Preview\\JPEG\""} {"reqId":"BYsRLO4mFkPyHhjYrTDl","level":0,"time":"2018-08-22T20:03:50+02:00","remoteAddr":"172.16.172.83","user":"admin","app":"core","method":"GET","url":"\/owncloud\/index.php\/apps\/gallery\/thumbnails?ids=16&scale=2&square=0&requesttoken=EgooBxMNRR5vUggLO0thImBAA18VGDNhYC0iUhMoeQY%3D%3AuPDdC8tSDkPZbyTRStInvRW43iFfPfOgBf967XtJVkM%3D","message":"OC_Image->fixOrientation() Image is not a JPEG."} {"reqId":"BYsRLO4mFkPyHhjYrTDl","level":0,"time":"2018-08-22T20:03:50+02:00","remoteAddr":"172.16.172.83","user":"admin","app":"core","method":"GET","url":"\/owncloud\/index.php\/apps\/gallery\/thumbnails?ids=16&scale=2&square=0&requesttoken=EgooBxMNRR5vUggLO0thImBAA18VGDNhYC0iUhMoeQY%3D%3AuPDdC8tSDkPZbyTRStInvRW43iFfPfOgBf967XtJVkM%3D","message":"OC_Image->fixOrientation() Orientation: -1"}

zero0cool0 on 23 Aug 2018

Confirmed.

As for gallery, it's strange because someone worked on rotation in recent versions https://github.com/owncloud/gallery/pull/764 and was reported working. I wonder what changed since.

PVince81 on 23 Aug 2018

As for gallery, it's strange because someone worked on rotation in recent versions owncloud/gallery#764 and was reported working. I wonder what changed since.

this is a pure client side (in browser) feature - this has no influence on the thumbnail generation

DeepDiver1975 on 23 Aug 2018

Works fine in 10.0.8, broken in 10.0.9.

Regression introduced through one of these:
541178b75a8633f1ff46fae769999e5addfadb1b
d69e53d6d4ae13750a96a5544b5158125c98e06d
d810963be4594160e0c0ebcf33cc977dc5c3a764
73a54c1fb06aba41c11aa148c8f8e0dbc26fd635
b09846b6b809eec7d9017d5baad322482957aad6
7fe9293b0b2b403bd0efdc8a04257d3d6560c09e

from the thumbnail related rework in https://github.com/owncloud/core/pull/31050.

The code path in question leads to https://github.com/owncloud/core/blob/v10.0.9/lib/private/Preview/Image.php#L49 which does not pass image type. Might need to pre-detect based on extension ?

PVince81 on 23 Aug 2018

seems OC_Image::loadFromData as called by OC_Image::loadFromFileHandle does not set $this->imageType, maybe it should do so based on mime type https://github.com/owncloud/core/blob/v10.0.10/lib/private/legacy/image.php#L598

side note: seems this stores the full picture in memory ??

PVince81 on 16 Oct 2018

I had a local try setting imageType, now it says OC_Image->fixOrientation() No readable file path set.. seems that method expects to have a writable file...

PVince81 on 16 Oct 2018

any chance that this will be fixed soon?
The gallery in 10.0.10 is useless without the thumbnail rotation popper working.
Would be migrating to nextcloud a possible fix?

gramels on 5 Dec 2018

We're going to address various preview issues as part of this project https://github.com/owncloud/files_mediaviewer. Due to the current workload probably not until beginning next year or so (10.1.1)

PVince81 on 5 Dec 2018

Sorry, but I dont get this.
This bug was introduced with 10.0.10 and it will take now several months to be fixed? Do you expect current users to downgrade again?
A picture gallery is a core feature und working image rotation is a basic.

gramels on 5 Dec 2018

@gramels we're a small team and we currently have other priorities so we cannot start working on this right away. Fixing this is definitely planned, together with other fixes to improve the preview system in general.

I agree that it is unfortunate that a regression is taking that long to fix. There were changes that needed to be made in the preview system (related to object store support with versioning) so fixing this will not be trivial and might require bigger changes in the preview system. I do hope that said changes will be for the best.

Also note that this is open-source so anyone else can also help fixing this.

PVince81 on 5 Dec 2018

So is the message by the owncloud team to owncloud users not to use the gallery?

Did anybody experience the same issue with nextcloud?
This might be a trigger to migrate.

gramels on 5 Dec 2018

migrated to nextcloud
seemed fixed in nc 12 & 13 & 14

gramels on 5 Dec 2018

I have the same issue. @PVince81 : Any details when it will b e fixed?

TDannhauer on 19 Jan 2019

Analysis

I tried to figure out that there is something wrong with https://github.com/owncloud/core/blob/master/lib/private/legacy/image.php#L386, which is causing the rotation of the image shared https://github.com/owncloud/core/issues/18645#issuecomment-415394195. I will update more regarding the same in this comment. Since this analysis is in the initial stage, the more information found would lead us to the actual problem.

Found a method loadFromFile() in https://github.com/owncloud/core/blob/master/lib/private/legacy/image.php#L495.
If we use this method instead of loadFromFileHandle(), in https://github.com/owncloud/core/blob/master/lib/private/Preview/Image.php#L49, I found the orientation of exif image gets corrected. By uploading a few files, I did not found any impact on performance, as such ( at least notable to my eyes ). I tried with a few sample exif images which has orientation. And without loadFromFile() files are seen as mentioned in the issue. After using loadFromFile(), they were getting corrected.

I have also noticed https://github.com/owncloud/core/pull/29319, which replaced loadFromFile, to loadFromFileHandle method.

Using objectstore

Using gallery to verify the preview with
objectstore_preview

The preview from object store version preview for the image file
version_object_store_preview
The preview doesn't come up properly.
On the other hand preview of text file under version panel
objectstore_text_preview_version

PR

Created PR: https://github.com/owncloud/core/pull/34297
Let me know the suggestion if this approach looks good @PVince81 @VicDeo

[x] Unit test is not added, the coverage is happening.
[x] Verify the preview of text files

[x] Verify public link preview
[x] Verify trashbin preview

PS: The idea behind public link preview and trashbin preview is inspired from comment : https://github.com/owncloud/core/pull/29319#pullrequestreview-71166454

sharidas on 28 Jan 2019

@sharidas please also test with external storage and make sure the file is not downloaded to temp space when on local storage. With ext storage, getLocalFile should download the file from the ext storage to temp space.

PVince81 on 30 Jan 2019

@sharidas also verify version preview: open versions panel and see the thumbnail orientation

PVince81 on 30 Jan 2019

@sharidas please also test with external storage and make sure the file is not downloaded to temp space when on local storage. With ext storage, getLocalFile should download the file from the ext storage to temp space.

While testing with external storage SFTP, I have noticed that getLocalFile method returns file of /tmp/oc_tmp_*.jpg ( obviously for a jpg file ). And when I try to check the file in my local machine:

✘ sujith@sujith-ownCloud  ~/test/owncloud2   stable10  ls /tmp/oc_tmp_y9p5iR-.jpg
ls: cannot access '/tmp/oc_tmp_y9p5iR-.jpg': No such file or directory
 ✘ sujith@sujith-ownCloud  ~/test/owncloud2   stable10 

So I believe it is not downloaded to the tmp location. Let me know if we agree with this observation.

sharidas on 30 Jan 2019

@sharidas also verify version preview: open versions panel and see the thumbnail orientation

Regarding preview for version, I am missing bits how to get the version update for image :( I mean after uploading the file, may I know how to update the image so that version panel lists previous version?

Regarding preview for text file the version panel, it is working properly
text_version

Regarding preview for image, the version panel is as shown here
version

sharidas on 30 Jan 2019

@sharidas depending on your distro the tmp files might be in chroot somewhere in /tmp. try searching for the exact file name. you'll likely need to set a breakpoint to prevent the file getting deleted

PVince81 on 30 Jan 2019

Analysis with external storage

Scenario by having SFTP as externa storage

Tried to upload an image file to local storage ( not the sftp storage or the folder ), the file path points to the data directory where it resides. Meaning its not copied to the temperory folder ( say /tmp or so )
Tried to upload an image to sftp storage and below is the observation:
1. The code reaches here https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/Common.php#L254
2. And then https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/LocalTempFileTrait.php#L48
3. And reaches https://github.com/owncloud/core/blob/master/lib/private/Files/Storage/LocalTempFileTrait.php#L77
4. By this time the $tmpFile is /tmp/oc_tmp_H1JRmt-.jpg and this file is found in my local machine at /tmp/systemd-private-012a7f2341b643d9881e8537cbb42431-apache2.service-epLRm1/tmp/oc_tmp_H1JRmt-.jpg
5. After the image is successfully uploaded, I tried:
  
  ✘ sujith@sujith-ownCloud  /tmp  sudo ls /tmp/systemd-private-012a7f2341b643d9881e8537cbb42431-apache2.service-epLRm1/tmp/oc_tmp_H1JRmt-.jpg ls: cannot access '/tmp/systemd-private-012a7f2341b643d9881e8537cbb42431-apache2.service- epLRm1/tmp/oc_tmp_H1JRmt-.jpg': No such file or directory ✘ sujith@sujith-ownCloud  /tmp 
6. So yes with external storage it is downloaded to the temp space.

@PVince81 I assume this is what you wanted me to look at from https://github.com/owncloud/core/issues/18645#issuecomment-458899237 and https://github.com/owncloud/core/issues/18645#issuecomment-458953283

sharidas on 31 Jan 2019

👍1

fix will be in 10.1.1 (10.1.0 already is in code freeze)

PVince81 on 1 Feb 2019

The fix doesn't seem to be in 10.1.1: https://github.com/owncloud/core/blob/v10.1.1/lib/private/Preview/Image.php

depeje on 9 Apr 2019

fix will be in 10.1.1 (10.1.0 already is in code freeze)

Predicting the specific number of a future version is a risky undertaking ;)
There was a "hotfix patch" for a particular bug and that ended up being the version called 10.1.1

The fix for the issue here was backported to stable10 in PR #34356. That will be in the next "regular release". At the time of writing this post the release numbering "will be" 10.2 - but of course the next release could be 10.1.2 10.2 or 11.0 - that just depends on the semver-related content of whatever is in the release.

phil-davis on 9 Apr 2019

Thank you for the explanation :) , it's very helpful. I mainly wanted to save other readers here the hassle of having to find this out the hard way. And I wanted to verify that this patch is indeed coming to a release.

depeje on 9 Apr 2019

👍1

you can already test with the RC1 if you like: https://central.owncloud.org/t/owncloud-server-10-2-0-rc1-released/19499

PVince81 on 16 Apr 2019

Core: OC_Image doesn't always set imageType correctly

All 33 comments

Steps to reproduce

Expected behaviour

Actual behaviour

Server configuration

Technical information

Results

Raw output

Client configuration

Logs

Analysis

Using objectstore

PR

Analysis with external storage

Scenario by having SFTP as externa storage

Related issues