Skip to content

fix(win32): only increase ACL size if a new ACE is about to be added#9491

Merged
mgallien merged 2 commits intomasterfrom
bugfix/noid/acl-errors-again
Feb 26, 2026
Merged

fix(win32): only increase ACL size if a new ACE is about to be added#9491
mgallien merged 2 commits intomasterfrom
bugfix/noid/acl-errors-again

Conversation

@nilsding
Copy link
Member

@nilsding nilsding commented Feb 23, 2026

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 within qDebug will end up changing the value returned by the GetLastError() function...

Related previous PRs: #9109, #9157; may resolve #8860

@nilsding nilsding added this to the 33.0.0 milestone Feb 23, 2026
@nilsding nilsding marked this pull request as draft February 23, 2026 14:30
@nilsding nilsding force-pushed the bugfix/noid/acl-errors-again branch 2 times, most recently from 4381607 to 41caca0 Compare February 25, 2026 15:55
@nilsding nilsding marked this pull request as ready for review February 25, 2026 18:15
@nilsding nilsding changed the title WIP: win32 readonly ACLs fix(win32): only increase ACL size if a new ACE is about to be added Feb 25, 2026
@nilsding
Copy link
Member Author

/backport to stable-33.0

@nilsding
Copy link
Member Author

/backport to stable-4.0

@mgallien mgallien enabled auto-merge February 26, 2026 13:17
@mgallien mgallien force-pushed the bugfix/noid/acl-errors-again branch from 41caca0 to f00da81 Compare February 26, 2026 13:17
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 <nilsding@nilsding.org>
Also added some extra logging when InitializeAcl fails, and a regression
test.

Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
@nilsding nilsding force-pushed the bugfix/noid/acl-errors-again branch from f00da81 to 254521d Compare February 26, 2026 16:03
@github-actions
Copy link

Artifact containing the AppImage: nextcloud-appimage-pr-9491.zip

Digest: sha256:400aa75b534067a418f29a26c2789ec4ba1f3d03acdb948b12e65365e40f48ea

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.

@mgallien mgallien merged commit 86c2bc7 into master Feb 26, 2026
22 of 27 checks passed
@mgallien mgallien deleted the bugfix/noid/acl-errors-again branch February 26, 2026 16:48
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
D Maintainability Rating on New Code (required ≥ A)
30 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Windows Client fails to sync new folders after Windows Update KB5065789 - ACL "insufficient memory" error

2 participants