Do not delete files open and folder content when cleaning log

[patrickelectric]

No description provided.

[joaoantoniocardoso]

good

[joaoantoniocardoso]

@patrickelectric I was thinking if this recursive implementation would be following soft links and if that is desirable or not. What do you think?

[patrickelectric]

@patrickelectric I was thinking if this recursive implementation would be following soft links and if that is desirable or not. What do you think?

Are you talking about recursive symlinks ? If someone ends up doing that they deserve the cpu lock.

[joaoantoniocardoso]

@patrickelectric I was thinking if this recursive implementation would be following soft links and if that is desirable or not. What do you think?

Are you talking about recursive symlinks ? If someone ends up doing that they deserve the cpu lock.

Nope, both is_file and is_dir follow symbolic links, so we are deleting links to files and entering linked directories to remove everything there, which is unsound in some scenarios. Just take a moment to think if this is desired in every situation in which this function will be used.

And the behavior was different before this patch - it was deleting using this other function, which, if I understood it correctly, wouldn't follow links to directories.

[patrickelectric]

@patrickelectric I was thinking if this recursive implementation would be following soft links and if that is desirable or not. What do you think?

Are you talking about recursive symlinks ? If someone ends up doing that they deserve the cpu lock.

Nope, both is_file and is_dir follow symbolic links, so we are deleting links to files and entering linked directories to remove everything there, which is unsound in some scenarios. Just take a moment to think if this is desired in every situation in which this function will be used.

And the behavior was different before this patch - it was deleting using this other function, which, if I understood it correctly, wouldn't follow links to directories.

Yes, I changed it to delete only files and not the folder structure.

[joaoantoniocardoso]

@patrickelectric I was thinking if this recursive implementation would be following soft links and if that is desirable or not. What do you think?

Are you talking about recursive symlinks ? If someone ends up doing that they deserve the cpu lock.

Nope, both is_file and is_dir follow symbolic links, so we are deleting links to files and entering linked directories to remove everything there, which is unsound in some scenarios. Just take a moment to think if this is desired in every situation in which this function will be used.
And the behavior was different before this patch - it was deleting using this other function, which, if I understood it correctly, wouldn't follow links to directories.

Yes, I changed it to delete only files and not the folder structure.

Yes I know, this is what we want. But now instead of deleting a link to a directory, we would be entering the linked directory and deleting everything there. This is quite a change in its behavior, can we write a comment on this function saying something like warning: every symbolic link to a directory present in this directory tree will be followed/entered and all the files in the destination's directory tree are going to be deleted.

[joaoantoniocardoso]

Maybe bikeshedding, but I was thinking again here... We might don't actually want to follow symbolic links to directories - considering that this function will be used against directories to which the user has access, if he creates a symbolic link to any system file, say / (to be more dramatic), it would lead to an undesired loss. Probably no one would do this for logs, but is this function used anywhere else, now here and in the future? 👀

[patrickelectric]

@joaoantoniocardoso changed to not follow subfolder symlinks of the desired bath.

[joaoantoniocardoso]

💯