Skip to content
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

fs::copy() unix: set file mode early #58909

Closed
wants to merge 1 commit into from

Conversation

haraldh
Copy link
Contributor

@haraldh haraldh commented Mar 4, 2019

same fix as commit fb98ca7
PR: #58803

A convenience method like fs::copy() should try to prevent pitfalls a
normal user doesn't think about.

In case of an empty umask, setting the file mode early prevents
temporarily world readable or even writeable files,
because the default mode is 0o666.

In case the target is a named pipe or special device node, setting the
file mode can lead to unwanted side effects, like setting permissons on
/dev/stdout or for root setting permissions on /dev/null.

same fix as commit fb98ca7
PR: rust-lang#58803

A convenience method like fs::copy() should try to prevent pitfalls a
normal user doesn't think about.

In case of an empty umask, setting the file mode early prevents
temporarily world readable or even writeable files,
because the default mode is 0o666.

In case the target is a named pipe or special device node, setting the
file mode can lead to unwanted side effects, like setting permissons on
/dev/stdout or for root setting permissions on /dev/null.
@rust-highfive
Copy link
Collaborator

r? @aidanhs

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2019
@haraldh
Copy link
Contributor Author

haraldh commented Mar 4, 2019

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned aidanhs Mar 4, 2019
@alexcrichton
Copy link
Member

Thanks! I wonder, could the logic here be shared between the two functions?

@haraldh
Copy link
Contributor Author

haraldh commented Mar 4, 2019

sure, we can extract the permission part in a helper function

@haraldh
Copy link
Contributor Author

haraldh commented Mar 6, 2019

waiting on #58803 to be merged for a refactored version

@haraldh
Copy link
Contributor Author

haraldh commented Mar 6, 2019

or should I force update #58803 with a new version?

@alexcrichton
Copy link
Member

Our queue is moving a bit slowly recently (sorry about that!) so want to go ahead and merge this with that PR?

@haraldh
Copy link
Contributor Author

haraldh commented Mar 12, 2019

done

@haraldh haraldh closed this Mar 12, 2019
@haraldh haraldh deleted the unix_other_fs_copy_fix branch March 12, 2019 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants