Old file versions seem to be corrupted (Win 8.1, chrome, openSUSE) #22073

Closed
MTRichards opened this Issue Feb 2, 2016 · 19 comments

Projects

None yet

6 participants

@MTRichards
Contributor

I use the web interface, navigate into a folder and select a powerpoint presentation that has changed a lot. I then select to download a version that is not the current version. When I open the downloaded presentation, powerpoint says the file is corrupted, and the files is actually rendered as completely empty.

Environment
s3
openSUSE
Windows 8.1 Chrome latest browser
Powerpoint 2010 and 2013

Steps to reproduce:

First create a ppt and make and save lots of changes, let it sync. This will create a file with lots of versions in it.
Log into the web interface, download a previous version. It will open as corrupted and be empty.

@PVince81
Collaborator
PVince81 commented Feb 2, 2016

So this is 8.2.2, but the file was likely modified several times in older versions.

@MTRichards does the most recent version work ? Like, make a small change, then download the most recent version. If that doesn't work, then the bug is in 8.2.2.

If it's only very old versions, then it could be a bug that happened in older versions.

@PVince81 PVince81 added the needs info label Feb 2, 2016
@MTRichards
Contributor

It was in the lest 2 weeks with all new files.

Matt Richards
VP Products & Markets
ownCloud, inc.

On Feb 2, 2016, at 5:34 PM, Vincent Petry notifications@github.com wrote:

So this is 8.2.2, but the file was likely modified several times in older versions.

@MTRichards does the most recent version work ? Like, make a small change, then download the most recent version. If that doesn't work, then the bug is in 8.2.2.

If it's only very old versions, then it could be a bug that happened in older versions.


Reply to this email directly or view it on GitHub.

@PVince81
Collaborator
PVince81 commented Feb 2, 2016

Okay, that's bad.

Is the ppt file uploaded through the sync client or using the web UI ?

@PVince81 PVince81 added this to the 9.0-current milestone Feb 2, 2016
@PVince81
Collaborator
PVince81 commented Feb 2, 2016

S3 is running PHP7 as far as I know, not sure which exact version.

I tried creating a ppt file and text file in a shared folder as recipient, and I cannot download any of the versions. However the file owner is able to properly download versions.

However on a local 8.2.2 instance it works fine.
So either it's related to PHP 7 or something else.

@PVince81
Collaborator
PVince81 commented Feb 2, 2016

The "Content-Length" header of the download is 0 bytes.
And "Content-Type" is application/octet-stream. It's almost as if the server didn't find the file and just returns whatever.

@PVince81
Collaborator
PVince81 commented Feb 2, 2016

Steps:

  1. Have two users "user1" and "user2"
  2. "user2" shares a folder "test" with "user1"
  3. "user1" creates a text file as "test/file.txt"
  4. "user1" edits the text file several times and saves it, to create versions
  5. "user1" opens the sidebar and "versions" tab
  6. Try downloading any of the versions

Expected result

Versions can be downloaded, version thumbnail appears.

Actual result

Versions file have 0 bytes when downloaded and the thumbnail appears as a broken image.

Env

ownCloud 8.2.2
PHP 7 (not sure which minor version)

@PVince81
Collaborator
PVince81 commented Feb 2, 2016
@Xenopathic
Member

Seems to work fine on master...

@PVince81
Collaborator
PVince81 commented Feb 3, 2016

On my private ownCloud instance, it works fine for new files.
However, for some older files the recipient cannot download the versions but the owner can.

This appears when the previews get created:

{"reqId":"qh9ugH5XcZgWfGkdAmbg","remoteAddr":"85.180.6.139","app":"core","message":"File:\"\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1450290905\" not found","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1450290905"}
{"reqId":"qh9ugH5XcZgWfGkdAmbg","remoteAddr":"85.180.6.139","app":"core","message":"File not found.","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1450290905"}
{"reqId":"9e8UnWV9572xNq8UKueX","remoteAddr":"85.180.6.139","app":"core","message":"File:\"\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1449674527\" not found","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1449674527"}
{"reqId":"9e8UnWV9572xNq8UKueX","remoteAddr":"85.180.6.139","app":"core","message":"File not found.","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1449674527"}
{"reqId":"vI+nKb0Hx129zcgYJjhb","remoteAddr":"85.180.6.139","app":"core","message":"File:\"\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1447009824\" not found","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1447009824"}
{"reqId":"vI+nKb0Hx129zcgYJjhb","remoteAddr":"85.180.6.139","app":"core","message":"File not found.","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1447009824"}
{"reqId":"iT9fA0uncGI5EsDFdZEr","remoteAddr":"85.180.6.139","app":"core","message":"File:\"\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1442141555\" not found","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1442141555"}
{"reqId":"iT9fA0uncGI5EsDFdZEr","remoteAddr":"85.180.6.139","app":"core","message":"File not found.","level":0,"time":"2016-02-03T09:47:01+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/preview?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&version=1442141555"}

and this when trying to download the version as recipient:

{"reqId":"tzL\/tc21KQqnJmwX5rCE","remoteAddr":"85.180.6.139","app":"PHP","message":"filesize(): stat failed for \/srv\/www\/vhosts\/owncloud\/data\/tingting\/files_versions\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1450290905 at \/srv\/www\/htdocs\/owncloud\/lib\/private\/files\/storage\/local.php#139","level":3,"time":"2016-02-03T09:47:10+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/download.php?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&revision=1450290905"}
{"reqId":"tzL\/tc21KQqnJmwX5rCE","remoteAddr":"85.180.6.139","app":"PHP","message":"fopen(\/srv\/www\/vhosts\/owncloud\/data\/tingting\/files_versions\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1450290905): failed to open stream: No such file or directory at \/srv\/www\/htdocs\/owncloud\/lib\/private\/files\/storage\/local.php#247","level":3,"time":"2016-02-03T09:47:10+00:00","method":"GET","url":"\/index.php\/apps\/files_versions\/download.php?file=%2Fclientsync%2Fpetry%2Fdoc%2FShoppen%2Fasia_laden.txt&revision=1450290905"}
@PVince81
Collaborator
PVince81 commented Feb 3, 2016

It looks like it's using the wrong home folder when downloading the version:
owncloud\/data\/tingting\/files_versions\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1450290905 (recipient)
instead of
owncloud\/data\/vincent\/files_versions\/clientsync\/petry\/doc\/Shoppen\/asia_laden.txt.v1450290905 (owner)

The share entry looks fine:

MariaDB [owncloud]> select * from oc_share where item_source=844 \G
*************************** 1. row ***************************
         id: 16
 share_type: 0
 share_with: tingting
  uid_owner: vincent
     parent: NULL
  item_type: folder
item_source: 844
item_target: /844
file_source: 844
file_target: /petry
permissions: 31
      stime: 1379851467
   accepted: 0
 expiration: NULL
      token: NULL
  mail_send: 0
1 row in set (0.00 sec)

So it's not clear yet in what conditions the versions will be resolved wrongly.
There are situations in which it works fine.

Needs further research.

@PVince81
Collaborator
PVince81 commented Feb 3, 2016

Ok, I think I got it, the shared folder must not be in root for the owner:

Here are the steps to reproduce it:

  1. Create two users "user1" and "user2"
  2. Log in as "user1"
  3. Create a folder "test/sub"
  4. Share "sub" with "user2"
  5. Log in as "user2"
  6. Create a text file with the text editor in "sub/file.txt"
  7. Edit+save the file several times to create versions
  8. Try downloading any of the versions

At this point, the versions are not resolved properly.
However if as "user1" you move "sub" to the root, then it will work again.

I suspect that the path resolution for version files in shares as recipient doesn't work properly.

@PVince81
Collaborator
PVince81 commented Feb 3, 2016

Also broken on master / 9.0.

Works properly in 8.0.10, 8.1.5.
Broken since 8.2.0.

This is bisectable.

@PVince81 PVince81 added the sev2-high label Feb 3, 2016
@nickvergessen
Contributor

I ended up on f9b0557 which makes no sense, so not sure what exactly caused this.

But it's important to note, that the version-files on the server are all valid for me all the time. So the state is recoverable.

git bisect start
# good: [015323c1cb239ab242ff43183fd2edb8e3103025] 8.1.3
git bisect good 015323c1cb239ab242ff43183fd2edb8e3103025
# bad: [2b9fcac462006fe4d36532e4e9b2b12aed6ec6a6] Merge pull request #22047 from owncloud/stable8.2-backport-21953
git bisect bad 2b9fcac462006fe4d36532e4e9b2b12aed6ec6a6
# good: [16ff6cff54768c15f126e523d195a6993e0e2aea] Merge pull request #17256 from owncloud/locking-disablecallbackwrapperwhendisabled
git bisect good 16ff6cff54768c15f126e523d195a6993e0e2aea
# good: [2e7d50b7233dd812a0cca8bdc433fbfa41b8b31e] Merge pull request #18991 from owncloud/public-resolve
git bisect good 2e7d50b7233dd812a0cca8bdc433fbfa41b8b31e
# bad: [daf9a63d43c9a7975b17ebee11e3fb92c431d3d7] Merge pull request #19386 from owncloud/ocs_shareapi_extenstion
git bisect bad daf9a63d43c9a7975b17ebee11e3fb92c431d3d7
# bad: [8b086156b1504c31fb93768d1dc8f413b5262c00] Merge pull request #19177 from owncloud/docker-check-state
git bisect bad 8b086156b1504c31fb93768d1dc8f413b5262c00
# bad: [afc7eeacaffac3034f34c880597f6655ce1bfc25] Merge pull request #18185 from owncloud/share-dialog-files-sidebar
git bisect bad afc7eeacaffac3034f34c880597f6655ce1bfc25
# bad: [d1f7087b6cf1f6c706f3a4f66cb0aa91bd5c28c1] Merge pull request #18979 from owncloud/sidebare-preview-fixes
git bisect bad d1f7087b6cf1f6c706f3a4f66cb0aa91bd5c28c1
# bad: [decdaf001855060862817e47d2a8d3d7dbb64043] Merge pull request #19024 from owncloud/remove-get_temp_dir
git bisect bad decdaf001855060862817e47d2a8d3d7dbb64043
# bad: [f9b05578a62973e8fa0fee619cf84474efc3650b] Merge pull request #19012 from owncloud/occ_encrypt_all_fix_name
git bisect bad f9b05578a62973e8fa0fee619cf84474efc3650b
# good: [442f5269ef229afa09cbe1e06b2a73bb9656e8c4] Fix Swift legacy auth mechanism fallback
git bisect good 442f5269ef229afa09cbe1e06b2a73bb9656e8c4
# good: [3adbfbfd694d09b165be41df108480ea16bb4d29] Use / instead of an empty string as cookie path
git bisect good 3adbfbfd694d09b165be41df108480ea16bb4d29
# good: [04db96adaf9faff80aaf26c181215d9986c97263] Merge pull request #19006 from owncloud/individual-it-patch-1
git bisect good 04db96adaf9faff80aaf26c181215d9986c97263
# good: [69b64b5b10f66e2826d9b59e26695ae2fa9ef161] use the same pattern for the command name like every other command
git bisect good 69b64b5b10f66e2826d9b59e26695ae2fa9ef161
# good: [341b733dec40aacf7ecbc66b2ca0381c3a119a24] Merge pull request #19010 from owncloud/use-proper-web-root
git bisect good 341b733dec40aacf7ecbc66b2ca0381c3a119a24
# first bad commit: [f9b05578a62973e8fa0fee619cf84474efc3650b] Merge pull request #19012 from owncloud/occ_encrypt_all_fix_name
@MorrisJobke
Member

Also noticed by @pako81 00004660

@PVince81
Collaborator

I'll investigate this

@PVince81 PVince81 self-assigned this Feb 10, 2016
@PVince81
Collaborator

Looks like the versions ajax endpoint is returning the wrong path, the one from the owner instead of recipient! ("path" attribute)

@PVince81
Collaborator

Looks like it was always returning the wrong path, so there might be some weird JS magic that used to fix it in 8.1. Let's see...

@PVince81
Collaborator

Ok got it. In 8.1 the versions.js code ignored the path from the response and just used whatever the real path of the file was.

There are two possible fixes then:

  1. Make the sidebar versions code behave like the old one, read the path from the file list, not the versions response
    or
  2. Fix the versions response to return the correct path and not disclose the owner's original path!

I'll go with 2)

@PVince81
Collaborator

I went for both, PR is here #22273

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