Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stable8: 15145 #15193

Merged
merged 2 commits into from
Mar 26, 2015
Merged

Stable8: 15145 #15193

merged 2 commits into from
Mar 26, 2015

Conversation

LukasReschke
Copy link
Member

Backport of #15145

@icewind1991 As discussed.

LukasReschke and others added 2 commits March 25, 2015 14:02
Despite it's PHPDoc the function might return `null` which was not properly catched and thus in some situations the share was resolved to the sharing users root directory.

To test this perform the following steps:

* Share file in owncloud 7 (7.0.4.2)
* Delete the parent folder of the shared file
* The share stays is in the DB and the share via the sharelink is inaccessible. (which is good)
* Upgrade to owncloud 8 (8.0.2) (This step is crucial. The bug is not reproduceable without upgrading from 7 to 8. It seems like the old tokens are handled different than the newer ones)
* Optional Step: Logout, Reset Browser Session, etc.
* Access the share via the old share url: almost empty page, but there is a dowload button which adds a "/download" to the URL.
* Upon clicking, a download.zip is downloaded which contains EVERYTHING from the owncloud directory (of the user who shared the file)
* No exception is thrown and no error is logged.

This will add a check whether the share is a valid one and also adds unit tests to prevent further regressions in the future. Needs to be backported to ownCloud 8.

Adding a proper clean-up of the orphaned shares is out-of-scope and would probably require some kind of FK or so.

Fixes #15097
@LukasReschke LukasReschke added this to the 8.0.3-current-maintenance milestone Mar 25, 2015
@ghost
Copy link

ghost commented Mar 25, 2015

Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/10823/
Test PASSed.

@icewind1991
Copy link
Contributor

👍

@MorrisJobke
Copy link
Contributor

@PVince81 please review ;)

@PVince81
Copy link
Contributor

Tested, works 👍

@PVince81
Copy link
Contributor

@karlitschek please confirm this backport

@karlitschek
Copy link
Contributor

backport is fine :-)

PVince81 pushed a commit that referenced this pull request Mar 26, 2015
@PVince81 PVince81 merged commit bc76760 into stable8 Mar 26, 2015
@PVince81 PVince81 deleted the stable8-15145 branch March 26, 2015 16:25
@jvillafanez
Copy link
Member

Validated using the steps from #15145

@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants