fix(files_external): propagate child copy failures in AmazonS3::copy() - #63123
Conversation
When copying a directory, copy() discarded the return values of its recursive calls and always returned true. rename() relies on that value, so a failed copy still led to rmdir() on the source: every file in the directory was deleted, the destination stayed empty, and the UI reported success. This is reachable whenever a provider rejects CopyObject for objects it otherwise serves — Hetzner Object Storage answers 501 NotImplemented for SSE-C encrypted objects, which makes every single child copy fail. Signed-off-by: Robert Niederreiter <rnix@squarewave.at>
23c1a82 to
964d9a5
Compare
|
Thanks for the review — comments shortened as suggested. For the test setup I could only apply
|
|
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Summary
When copying a directory, copy() discarded the return values of its recursive calls and always returned true. rename() relies on that value, so a failed copy still led to rmdir() on the source: every file in the directory was deleted, the destination stayed empty, and the UI reported success.
This is reachable whenever a provider rejects CopyObject for objects it otherwise serves — Hetzner Object Storage answers 501 NotImplemented for SSE-C encrypted objects, which makes every single child copy fail.
Behaviour with the fix (measured on 34.0.1, Hetzner Object Storage + SSE-C)
Renaming a folder now fails visibly instead of destroying its contents:
500, the files stay where they are. Thedestination directory created by
mkdir()before the failure remains as anempty folder and can be deleted normally.
MOVEfails with500, and the client rolls thelocal rename back and re-fetches the files. It does not fall back to
upload+delete, so nothing is duplicated and nothing is lost.
Renaming remains impossible on such providers — that part is outside
Nextcloud's control. What changes is that the failure is now visible instead
of silently deleting the data.
Related
Previously reported without an issue:
https://help.nextcloud.com/t/nc-copy-move-not-working-and-destroying-files-in-s3-external-storage-with-sse-c-key/231896
(NC 31.0.8, same provider, same 501 NotImplemented, same silent data loss)
Checklist
3. to review, feature component)stable32)AI (if applicable)