fix(win32): only increase ACL size if a new ACE is about to be added - #9491
Merged
Conversation
nilsding
requested review from
Aiiaiiio,
camilasan,
i2h3 and
mgallien
as code owners
February 23, 2026 14:29
nilsding
marked this pull request as draft
February 23, 2026 14:30
nilsding
force-pushed
the
bugfix/noid/acl-errors-again
branch
2 times, most recently
from
February 25, 2026 15:55
4381607 to
41caca0
Compare
nilsding
marked this pull request as ready for review
February 25, 2026 18:15
nilsding
changed the title
Member
Author
|
/backport to stable-33.0 |
Member
Author
|
/backport to stable-4.0 |
mgallien
approved these changes
Feb 26, 2026
mgallien
enabled auto-merge
February 26, 2026 13:17
mgallien
force-pushed
the
bugfix/noid/acl-errors-again
branch
from
February 26, 2026 13:17
41caca0 to
f00da81
Compare
Noticed that GetLastError() could return 0 (error code for a successful operation) -- probably due to win32 calls performed during logging --> Store the result in a variable before writing log messages, just in case ... Signed-off-by: Jyrki Gadinger <[email protected]>
Also added some extra logging when InitializeAcl fails, and a regression test. Signed-off-by: Jyrki Gadinger <[email protected]>
nilsding
force-pushed
the
bugfix/noid/acl-errors-again
branch
from
February 26, 2026 16:03
f00da81 to
254521d
Compare
Contributor
|
Artifact containing the AppImage: nextcloud-appimage-pr-9491.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
This was referenced Feb 26, 2026
|
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When changing the ACL to read-write, the size of the ACL struct does not increase as all previously defined (and -- due to a bug in < 4.0.2 -- wrongfully duplicated)
ACCESS_DENIED_ACEs are dropped.In case the current ACL size is already too big to fit a new
ACCESS_DENIED_ACE(e.g. when trying to make a folder read-only again), assume the ACL is full of duplicate entries and just™ reuse the same ACL size...Also added a test to verify that the client no longer fails modifying ACLs with too many of these access denied ACE entries. And logs printing
WindowsErrors are fixed once again -- seems like using it directly from withinqDebugwill end up changing the value returned by theGetLastError()function...Related previous PRs: #9109, #9157; may resolve #8860