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

we don't need to check permissions twice #284

Merged
merged 1 commit into from
Jul 4, 2016
Merged

Conversation

schiessle
Copy link
Member

@schiessle schiessle added the 3. to review Waiting for reviews label Jul 1, 2016
@schiessle schiessle added this to the Nextcloud Next milestone Jul 1, 2016
@LukasReschke
Copy link
Member

👍

@@ -320,16 +320,10 @@ public static function rollback($file, $revision) {
// add expected leading slash
$file = '/' . ltrim($file, '/');
list($uid, $filename) = self::getUidAndFilename($file);
if ($uid === null || trim($filename, '/') === '') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this is needed to handle some edge cases but I don't remember the details

Copy link
Member Author

@schiessle schiessle Jul 4, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of any, but I will put it back... can't hurt. Also the tests seems not to handle such edge cases

@schiessle schiessle force-pushed the cleanup-after-sync branch from 1400030 to 2cf4530 Compare July 4, 2016 08:48
@schiessle
Copy link
Member Author

@icewind1991 I added the additional check again, please review. Thanks!

@LukasReschke
Copy link
Member

@icewind1991 I added the additional check again, please review. Thanks!

Can you backport the additional check also to stable9?

@schiessle
Copy link
Member Author

@LukasReschke done: #304

@icewind1991
Copy link
Member

👍

1 similar comment
@MorrisJobke
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants