Fix a bug in the recursive rmdir #39074
Conversation
A directory containing a directory containing files where never deleted.
Example:
Let's consider this configuration:
```
/tmp/root
└── intermediate
├── file1
├── file2
└── file3
```
This configuration was able to occured during a plugin update.
If NC was running a `rmdirr("/tmp/root");` (which seems to be done during a cron, but, that's not important), it would never deleted anything.
Indeed, the `$files` variable would contians only the `intermediate` directory.
Thus, during the loop to delete the content of the `/tmp/root` content, when `$fileInfo` will contains the descriptor of `/tmp/root/intermediate`, the `rmdir` PHP function would fails because this directory is not empty.... And maked the final `rmdir` (in the `$deleteSelf` condition).
Tested on `PHP 8.1.20 (built: Jun 15 2023 01:23:25) (NTS)`, the PHP version of the docker container provided by NC-AIO.
Signed-off-by: Ajabep <ajabep@tutanota.com>
szaimen
requested review from
a team,
ArtificialOwl,
icewind1991,
nfebe and
kesselb
and removed request for
a team
|
The |
Actually, maybe it's a but in PHP (version used in your docker images), maybe it's a bad documentation, but, it was also what I thought initially ;-) |
kesselb
added
2. developing
|
We did a lot of debugging about iterators in nextcloud/updater#510 / php/doc-en#2903. It appears that in some constellations, it's not safe to modify the file system while iterating. For example: https://github.com/nextcloud/updater/blob/55eca48fc6ed8b0f773dc3f14db84befa9ee1da9/lib/Updater.php#L776-L816 or by using |
|
I no longer use NextCloud, so, I can't test if the problem persists on stable versions. Feel free to reuse and adapt this MR if still needed. |
Summary
The rmdirr tried to delete a non-empty directory, thus, it was failing.
A directory containing a directory containing files where never deleted.
Example:
Let's consider this configuration:
This configuration was able to occured during a plugin update.
If NC was running a
rmdirr("/tmp/root");(which seems to be done during a cron, but, that's not important), it would never deleted anything.Indeed, the
$filesvariable would contians only theintermediatedirectory.Thus, during the loop to delete the content of the
/tmp/rootcontent, when$fileInfowill contains the descriptor of/tmp/root/intermediate, thermdirPHP function would fails because this directory is not empty.... And maked the finalrmdir(in the$deleteSelfcondition).Tested on
PHP 8.1.20 (built: Jun 15 2023 01:23:25) (NTS), the PHP version of the docker container provided by NC-AIO.Checklist