-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add std::fs::rename_noreplace
#131
Comments
Python 3 includes both os.rename and os.replace. |
But does python promise atomicity? |
Are you sure about that? "If dst is a non-empty directory, OSError will be raised." In that sense I don't really understand the semantics of
I prefer framing the question the other way around: Does the python potentially suffer from race hazards? But to answer the question, the docs say yes: "If successful, the renaming will be an atomic operation (this is a POSIX requirement)." |
It says the rename will be atomic, that doesn't tell us if the check is also part of that. But if it's allowed to be not-atomic in the failure case then hardlink + unlink(src) approach could be used. |
This is a hard blocker IMO. Functionality that's sometimes available seems better suited to a crate than the stdlib. |
If there is wide enough support then we could have unsupported platforms always return an error. We do that sometimes. And an argument can be made that std is the right place for these kinds of building blocks even if they fall short of being truly universal. In this case all tier 1 targets support it, no? However, it is admittedly janky and I don't want to deemphasize the downsides. I just don't think it's a hard blocker. It's more of a strong warning to tread very carefully. |
This is a common misconception and is often not true in the case of OS power loss or crash (which is a fairly common scenario in non-server usage -- users tend to hard-reset machines pretty often). In that case, it's filesystem dependent. As an example of how this can be fairly subtle: With btrfs, rename is only atomic on crash with if it is replacing a file, so on that filesystem That said I don't largely have an opinion beyond this, since it's probably "more atomic" (if such a thing is a sensible comparator) than the alternative folks are likely to write. |
Yeah, I think "atomic" is the wrong way to describe this. The goal is to prevent overwrites in a way that mitigates TOCTOU issues. The whole operation being atomic (and crash safe) would be nice but isn't the main purpose and in any case can't be guaranteed. |
We discussed this in today's @rust-lang/libs-api meeting. We came up with a potential solution that would work as a fallback on all platforms, with some limitations. If the native platform doesn't support a rename-without-replacement operation, hardlink the source to the target, then unlink the source. This is close to what the underlying filesystem does already. From the documentation of rename: "However, there will probably be a window in which both oldpath and newpath refer to the file being renamed." This wouldn't work for directories, but for directories the ordinary OSes that have neither a rename-without-replace syscall nor hardlinks won't be able to implement this, but that's fine: on those platforms, it can just fail unconditionally, and callers will have to use something else. |
I think that's technically not atomic (due to old and new existing at the same time) but, as I said above, I also agree it doesn't matter for mitigating TOCTOU issues. A temp file not being deleted is a gc issue rather than a bug or security issue. |
See the text I quoted from the rename manpage; the rename syscall can also result in both files existing at the same time. |
Is this reliable? E.g. when network filesystems are involved? Does the hardlink fallback work there? |
From MS-FSCC
So according to the protocol, it "MUST" fail if the name already exists. |
Proposal
Add
std::fs::rename_noreplace
, which is equivalent tostd::fs::rename
but with the semantic that an existing target location will never be overwritten and always yield an error instead.Problem statement
There is no easy way to rename/move a file without accidentally overwriting the target location should it exist. There is no way to emulate the desired behavior in an atomic way, meaning that all workarounds suffer from TOCTTOU issues.
Motivation, use-cases
It is a common pattern for software to do file system writes into a temporary location next to the target and then move it in place once it is ready. This is useful because move operations are atomic. In many cases overwriting the target when it exists is a desired semantic, but in some cases it is not.
For example when extracting a compressed directory, the software would first extract the content into a temporary location then move it over. When destination files exist, it wants to handle it in some special way (e.g. chose a different name or ask the user). The existence check needs to be done atomically with the move operation, otherwise race hazards may occur.
Solution sketches
A new function
std::fs::rename_noreplace
is introduced. On Linux, it defers to therenameat2
syscall with theRENAME_NOREPLACE
flag set. On MacOS, it uses therenameatx_np
call withRENAME_EXCL
. On Windows, this is the default behavior so theMOVEFILE_REPLACE_EXISTING
need to be omitted.I could not find any equivalent syscall on non-Linux unix systems (*BSD), and don't know how to deal with that aspect.
Alternatively, we could change the semantics of the current
rename
function (they are explicitly documented as unstable), and optionally add arename_replace
command.As a second alternative, we could expose more of the platform specific syscalls: for example, Linux
renameat2
also has a flag for atomically swapping two paths.Links and related work
https://internals.rust-lang.org/t/rename-file-without-overriding-existing-target/17637
What happens now?
This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.
The text was updated successfully, but these errors were encountered: