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

haraldh · 2019-02-28T09:29:42Z

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.

copy_file_range() returns EINVAL, if the destination is a FIFO/pipe or
a device like "/dev/null", so fallback to io::copy, too.

Fixes: #26933
Fixed: #37885

rust-highfive · 2019-02-28T09:29:45Z

r? @withoutboats

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

haraldh · 2019-02-28T09:31:09Z

r? @alexcrichton

bors · 2019-02-28T14:44:43Z

☔ The latest upstream changes (presumably #58208) made this pull request unmergeable. Please resolve the merge conflicts.

src/libstd/sys/unix/fs.rs

haraldh · 2019-03-01T07:47:10Z

new refined version with better comments and logic

rust-highfive · 2019-03-01T07:54:07Z

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:end:0a8d1876:start=1551426417322061806,finish=1551426536556350387,duration=119234288581
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:04:51]    Compiling panic_unwind v0.0.0 (/checkout/src/libpanic_unwind)
[00:04:57] error: unused `core::result::Result` that must be used
[00:04:57]    --> src/libstd/sys/unix/fs.rs:903:9
[00:04:57]     |
[00:04:57] 903 |         writer.set_permissions(perm);
[00:04:57]     |
[00:04:57]     = note: `-D unused-must-use` implied by `-D warnings`
[00:04:57]     = note: `-D unused-must-use` implied by `-D warnings`
[00:04:57]     = note: this `Result` may be an `Err` variant, which should be handled
[00:04:57] error: aborting due to previous error
[00:04:57] 
[00:04:57] error: Could not compile `std`.
[00:04:57]

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

alexcrichton · 2019-03-01T15:15:50Z

@bors: r+

bors · 2019-03-01T15:15:51Z

📌 Commit fb98ca7 has been approved by alexcrichton

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.

fs::copy() linux: set file mode early 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`. copy_file_range() returns EINVAL, if the destination is a FIFO/pipe or a device like "/dev/null", so fallback to io::copy, too. Fixes: rust-lang#26933 Fixed: rust-lang#37885

bors · 2019-03-23T02:40:07Z

💔 Test failed - checks-travis

rust-highfive · 2019-03-23T02:40:07Z

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

haraldh · 2019-03-23T04:30:43Z

Hmpf, this filesystem test fails on asmjs-unknown-emscripten Does asmjs even handle files?

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`. copy_file_range() returns EINVAL, if the destination is a FIFO/pipe or a device like "/dev/null", so fallback to io::copy, too. Use `fcopyfile` on MacOS instead of `copyfile`. Fixes: rust-lang#26933 Fixed: rust-lang#37885

haraldh · 2019-03-23T06:37:51Z

added

// ignore-emscripten no files

to test

haraldh · 2019-03-23T07:05:06Z

removed the ignore and added

// If `dummy_file` does not exist, then we might get another unrelated error
#[cfg(not(target_os = "emscripten"))] let dummy_file = std::env::current_exe().unwrap();
#[cfg(target_os = "emscripten")] let dummy_file = "a";

haraldh · 2019-03-23T07:11:47Z

Another alternative would be to use TempDir as src/test/run-pass-fulldeps/rename-directory.rs does.

haraldh · 2019-03-23T07:14:16Z

Or just RUST_TEST_TMPDIR as src/test/run-pass-fulldeps/create-dir-all-bare.rs

alexcrichton · 2019-03-25T14:01:42Z

Hm I wonder if this test could just be ignored on emscripten? You can just add a #[cfg(not(...))] annotation to the whole test, and it's not too important to bend over backwards to get it running on emscripten.

haraldh · 2019-03-25T14:04:23Z

Hm I wonder if this test could just be ignored on emscripten? You can just add a #[cfg(not(...))] annotation to the whole test, and it's not too important to bend over backwards to get it running on emscripten.

Basically I did that with the latest version. For emscripten it's the old unchanged test (which should pass then) and for all the others it is hopefully picking up the current executable as the test file.

alexcrichton · 2019-03-25T15:48:44Z

Oh sure yeah and this might pass, but it's pretty jarring to be reading a test and randomly see a conditional statement for one particular platform. It's pretty common, however, to ignore tests on platforms for various reasons, so I think it's best to just ignore the whole test

haraldh · 2019-03-26T13:53:33Z

Oh sure yeah and this might pass, but it's pretty jarring to be reading a test and randomly see a conditional statement for one particular platform. It's pretty common, however, to ignore tests on platforms for various reasons, so I think it's best to just ignore the whole test

I didn't want to take away the test for emscripten, because it seems to pass in the past.

alexcrichton · 2019-03-26T14:32:06Z

Let's please ignore the test for emscripten, because we don't really need to keep tests running that just happened to pass before.

haraldh · 2019-03-26T14:50:24Z

Let's please ignore the test for emscripten, because we don't really need to keep tests running that just happened to pass before.

here we go, although that contradicts my understanding of a CI

alexcrichton · 2019-03-26T15:52:35Z

@bors: r+

bors · 2019-03-26T15:52:36Z

📌 Commit cf8347b has been approved by alexcrichton

fs::copy() unix: set file mode early 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`. copy_file_range() returns EINVAL, if the destination is a FIFO/pipe or a device like "/dev/null", so fallback to io::copy, too. Fixes: rust-lang#26933 Fixed: rust-lang#37885

@ghost

Rollup of 12 pull requests Successful merges: - #57987 (Fix some AArch64 typos) - #58581 (Refactor generic parameter encoder functions) - #58803 (fs::copy() unix: set file mode early) - #58848 (Prevent cache issues on version updates) - #59198 (Do not complain about unmentioned fields in recovered patterns) - #59351 (Include llvm-ar with llvm-tools component) - #59413 (HirIdify hir::ItemId) - #59441 (Remove the block on natvis for lld-link.) - #59448 (Use consistent phrasing for all macro summaries) - #59456 (Add documentation about `for` used as higher ranked trait bounds) - #59472 (Document that `std::io::BufReader` discards contents on drop) - #59474 (Fix link capitalization in documentation of std::io::BufWriter.) Failed merges: r? @ghost

haraldh · 2019-03-28T12:31:49Z

Halleluja 😀

rust-highfive assigned withoutboats Feb 28, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 28, 2019

haraldh mentioned this pull request Feb 28, 2019

fs::copy() linux: handle sparse files and set file mode early #58636

Closed

rust-highfive assigned alexcrichton and unassigned withoutboats Feb 28, 2019

alexcrichton reviewed Feb 28, 2019

View reviewed changes

src/libstd/sys/unix/fs.rs Outdated Show resolved Hide resolved

src/libstd/sys/unix/fs.rs Outdated Show resolved Hide resolved

src/libstd/sys/unix/fs.rs Outdated Show resolved Hide resolved

src/libstd/sys/unix/fs.rs Outdated Show resolved Hide resolved

haraldh force-pushed the fs_copy_fix branch 2 times, most recently from c7a6814 to d329ec4 Compare March 1, 2019 07:45

haraldh force-pushed the fs_copy_fix branch from d329ec4 to fb98ca7 Compare March 1, 2019 08:06

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2019

haraldh mentioned this pull request Mar 4, 2019

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

Closed

Centril mentioned this pull request Mar 10, 2019

Rollup of 16 pull requests #59066

Closed

Centril mentioned this pull request Mar 10, 2019

Rollup of 17 pull requests #59067

Closed

Centril mentioned this pull request Mar 10, 2019

Rollup of 16 pull requests #59069

Closed

Centril mentioned this pull request Mar 10, 2019

Rollup of 17 pull requests #59075

Closed

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 23, 2019

haraldh force-pushed the fs_copy_fix branch from d997eac to cf8347b Compare March 23, 2019 06:37

haraldh force-pushed the fs_copy_fix branch from cf8347b to 0bcda11 Compare March 23, 2019 07:03

haraldh force-pushed the fs_copy_fix branch from 0bcda11 to cf8347b Compare March 26, 2019 14:49

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2019

Centril mentioned this pull request Mar 28, 2019

Rollup of 12 pull requests #59478

Merged

bors merged commit cf8347b into rust-lang:master Mar 28, 2019

haraldh deleted the fs_copy_fix branch March 28, 2019 12:31

the8472 mentioned this pull request Feb 27, 2021

std::fs::write is not atomic - document it explicitly or make it atomic #82590

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

haraldh commented Feb 28, 2019

rust-highfive commented Feb 28, 2019

haraldh commented Feb 28, 2019

bors commented Feb 28, 2019

haraldh commented Mar 1, 2019

rust-highfive commented Mar 1, 2019

alexcrichton commented Mar 1, 2019

bors commented Mar 1, 2019

bors commented Mar 23, 2019

rust-highfive commented Mar 23, 2019

haraldh commented Mar 23, 2019 •

edited

Loading

haraldh commented Mar 23, 2019

haraldh commented Mar 23, 2019

haraldh commented Mar 23, 2019

haraldh commented Mar 23, 2019

alexcrichton commented Mar 25, 2019

haraldh commented Mar 25, 2019

alexcrichton commented Mar 25, 2019

haraldh commented Mar 26, 2019

alexcrichton commented Mar 26, 2019

haraldh commented Mar 26, 2019

alexcrichton commented Mar 26, 2019

bors commented Mar 26, 2019

haraldh commented Mar 28, 2019

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

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

Conversation

haraldh commented Feb 28, 2019

rust-highfive commented Feb 28, 2019

haraldh commented Feb 28, 2019

bors commented Feb 28, 2019

haraldh commented Mar 1, 2019

rust-highfive commented Mar 1, 2019

alexcrichton commented Mar 1, 2019

bors commented Mar 1, 2019

bors commented Mar 23, 2019

rust-highfive commented Mar 23, 2019

haraldh commented Mar 23, 2019 • edited Loading

haraldh commented Mar 23, 2019

haraldh commented Mar 23, 2019

haraldh commented Mar 23, 2019

haraldh commented Mar 23, 2019

alexcrichton commented Mar 25, 2019

haraldh commented Mar 25, 2019

alexcrichton commented Mar 25, 2019

haraldh commented Mar 26, 2019

alexcrichton commented Mar 26, 2019

haraldh commented Mar 26, 2019

alexcrichton commented Mar 26, 2019

bors commented Mar 26, 2019

haraldh commented Mar 28, 2019

haraldh commented Mar 23, 2019 •

edited

Loading