Skip to content

Fix incorrect if conditions in View#31754

Merged
come-nc merged 3 commits intomasterfrom
fix/view-inconsistent-if-conditions
Apr 5, 2022
Merged

Fix incorrect if conditions in View#31754
come-nc merged 3 commits intomasterfrom
fix/view-inconsistent-if-conditions

Conversation

@come-nc
Copy link
Contributor

@come-nc come-nc commented Mar 30, 2022

Signed-off-by: Côme Chilliet come.chilliet@nextcloud.com

@come-nc come-nc added the 3. to review Waiting for reviews label Mar 30, 2022
@come-nc come-nc added this to the Nextcloud 24 milestone Mar 30, 2022
@come-nc come-nc requested a review from icewind1991 March 30, 2022 09:20
@come-nc come-nc self-assigned this Mar 30, 2022
@CarlSchwan
Copy link
Member

CarlSchwan commented Mar 30, 2022

Could you add a bit more info in the commit message about what this fix and why the previous code was incorrect?

@come-nc come-nc force-pushed the fix/view-inconsistent-if-conditions branch from adf4bc1 to dcd2a68 Compare March 30, 2022 10:33
@come-nc
Copy link
Contributor Author

come-nc commented Mar 30, 2022

Could you add a bit more info in the commit message about what this fix and why the previous code was incorrect?

Done.

@blizzz blizzz mentioned this pull request Mar 31, 2022
Merged
@PVince81 PVince81 requested review from CarlSchwan, Pytal and artonge April 4, 2022 07:52
CarlSchwan
CarlSchwan approved these changes Apr 4, 2022
View reviewed changes
Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

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

This probably only make a difference when you can't read a file but still have permission to do other stuff (file drop)

@artonge
Copy link
Contributor

artonge commented Apr 4, 2022

Could we take the opportunity to exit early on this condition instead of wrapping 100 lines into the if: https://github.com/nextcloud/server/blob/dcd2a6841e9f1f111f74c9f6006c3821bfb12e3d/lib/private/Files/View.php#L1446=

@come-nc
Copy link
Contributor Author

come-nc commented Apr 5, 2022

Could we take the opportunity to exit early on this condition instead of wrapping 100 lines into the if: https://github.com/nextcloud/server/blob/dcd2a6841e9f1f111f74c9f6006c3821bfb12e3d/lib/private/Files/View.php#L1446=

Done

CarlSchwan
CarlSchwan reviewed Apr 5, 2022
View reviewed changes
@come-nc
Copy link
Contributor Author

come-nc commented Apr 5, 2022

/rebase

@come-nc
Copy link
Contributor Author

come-nc commented Apr 5, 2022

/backport to stable23

@come-nc
Copy link
Contributor Author

come-nc commented Apr 5, 2022

/backport to stable22

come-nc added 2 commits April 5, 2022 13:58
@come-nc
Fix incorrect if conditions in View
916fbe8 
($something->getPermissions() && Constants::PERMISSION_READ) does not
  make sense as PERMISSION_READ contant is 1 this will always evaluate to
  true.
getPersmissions is returning an int which is a bitwise combination as
  documented in the interface, so it should be used with bit operators.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Exit early if $storage is falsy in View::getDirectoryContent
d36a1a7 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@nextcloud-command nextcloud-command force-pushed the fix/view-inconsistent-if-conditions branch from be6c7ba to d36a1a7 Compare April 5, 2022 13:58
@come-nc
composer run cs:fix
140624d 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc mentioned this pull request Apr 5, 2022
Merged
@come-nc come-nc merged commit fff26ad into master Apr 5, 2022
@come-nc come-nc deleted the fix/view-inconsistent-if-conditions branch April 5, 2022 16:06
@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Apr 5, 2022

The backport to stable23 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Apr 5, 2022

The backport to stable22 failed. Please do this backport manually.

@solracsf
Copy link
Member

solracsf commented Apr 5, 2022

/backport to stable23

@solracsf
Copy link
Member

solracsf commented Apr 5, 2022

/backport to stable22

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Apr 5, 2022

The backport to stable22 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Apr 5, 2022

The backport to stable23 failed. Please do this backport manually.

@blizzz blizzz mentioned this pull request Apr 7, 2022
Merged
@come-nc come-nc mentioned this pull request Apr 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@icewind1991 icewind1991 icewind1991 approved these changes

@artonge artonge artonge approved these changes

@Pytal Pytal Awaiting requested review from Pytal

Assignees

@come-nc come-nc

Labels

3. to review Waiting for reviews

Projects

None yet

Milestone

Nextcloud 24

Development

Successfully merging this pull request may close these issues.

5 participants

@come-nc @CarlSchwan @artonge @solracsf @icewind1991