fix(storage): fallback to copy and unlink when rename fails #38623
Conversation
This file contains 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
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
kesselb
force-pushed
the
fix/14743/rename-fallback-copy-unlink
branch
from
a890d67
to
083c869
Compare
| $this->checkTreeForForbiddenItems($this->getSourcePath($source)); | ||
| } | ||
|
|
||
| return rename($this->getSourcePath($source), $this->getSourcePath($target)); | ||
| if (@rename($this->getSourcePath($source), $this->getSourcePath($target))) { |
Check failure
Code scanning / Psalm
TaintedFile
| $this->checkTreeForForbiddenItems($this->getSourcePath($source)); | ||
| } | ||
|
|
||
| return rename($this->getSourcePath($source), $this->getSourcePath($target)); | ||
| if (@rename($this->getSourcePath($source), $this->getSourcePath($target))) { |
Check failure
Code scanning / Psalm
TaintedFile
This comment was marked as resolved.
This comment was marked as resolved.
kesselb
force-pushed
the
fix/14743/rename-fallback-copy-unlink
branch
from
083c869
to
9122df3
Compare
kesselb
commented
Jun 3, 2023
kesselb
added
bug
3. to review
come-nc
approved these changes
Jun 5, 2023
kesselb
force-pushed
the
fix/14743/rename-fallback-copy-unlink
branch
from
9122df3
to
ea05970
Compare
kesselb
force-pushed
the
fix/14743/rename-fallback-copy-unlink
branch
from
ea05970
to
2e6f548
Compare
nfebe
reviewed
Jul 29, 2023
| @@ -335,7 +335,7 @@ private function checkTreeForForbiddenItems(string $path) { | |||
| } | |||
| } | |||
|
|
|||
| public function rename($source, $target) { | |||
| public function rename($source, $target): bool { | |||
There was a problem hiding this comment.
Nice opportunity to type the function params
nfebe
approved these changes
Jul 29, 2023
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
kesselb
force-pushed
the
fix/14743/rename-fallback-copy-unlink
branch
from
2e6f548
to
b4ff557
Compare
5 tasks
|
Follow-up to change rmdir to unlink: #39644 |
8 tasks
|
🎉🎉🎉 |
9 tasks
This was referenced Oct 22, 2023
8 tasks
QUIQQER
pushed a commit
to QUIQQER/QUtils
that referenced
this pull request
Jan 19, 2024
This could happen if a folder is mounted to another drive. The media folder is a good example for this. The problem also occurs in plain Docker containers. The docker magic seems to create different filesystems. This is a known problem (& solution), see: - nextcloud/server#38623 - https://bugs.php.net/bug.php?id=54097
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
rename(): The first argument tocopy()function cannot be a directory #38569Summary
Alternative approach to #19289
If you use docker's OverlayFS and mount two folders into your container from the same disk it's also not possible to rename, but
$stat['dev']is equal.1Many thanks again to @kubatek94 for the initial pull request.
TODO
Checklist
Footnotes
https://docs.docker.com/storage/storagedriver/overlayfs-driver/#modifying-files-or-directories ↩