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

Fix wasi::fs::OpenOptions to imply write when append is on #75189

Merged
merged 1 commit into from
Aug 13, 2020

Conversation

kawamuray
Copy link
Contributor

This PR fixes a bug in OpenOptions of wasi platform that it currently doesn't imply write mode when only append is enabled.
As explained in the doc of OpenOptions#append, calling .append(true) should imply .write(true) as well.

Reproduce

Given below simple Rust program:

use std::fs::OpenOptions;
use std::io::Write;

fn main() {
    let mut file = OpenOptions::new()
        .write(true)
        .create(true)
        .open("foo.txt")
        .unwrap();
    writeln!(file, "abc").unwrap();
}

it can successfully compiled into wasm and execute by wasmtime runtime:

$ rustc --target wasm32-wasi write.rs
$ ~/wasmtime/target/debug/wasmtime run --dir=. write.wasm 
$ cat foo.txt 
abc

However when I change .write(true) to .append(true), it fails to execute by the error "Capabilities insufficient":

$ ~/wasmtime/target/debug/wasmtime run --dir=. append.wasm
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 76, kind: Other, message: "Capabilities insufficient" }', append.rs:10:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error: failed to run main module `append.wasm`
...

This is because of lacking "rights" on the opened file:

$ RUST_LOG=trace ~/wasmtime/target/debug/wasmtime run --dir=. append.wasm 2>&1 | grep validate_rights
 TRACE wasi_common::entry                                  >      | validate_rights failed: required rights = HandleRights { base: fd_write (0x40), inheriting: empty (0x0) }; actual rights = HandleRights { base: fd_seek|fd_fdstat_set_flags|fd_sync|fd_tell|fd_advise|fd_filestat_set_times|poll_fd_readwrite (0x88000bc), inheriting: empty (0x0) }

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 5, 2020
@joshtriplett
Copy link
Member

Looks good to me. A libs implementer should review as well, but I think this is reasonable.

@kawamuray
Copy link
Contributor Author

Hi @kennytm ,
do you think you can find some time for reviewing this? Or would I better try finding someone else as a reviewer?

@kennytm
Copy link
Member

kennytm commented Aug 11, 2020

🤔 i'm not a libs implementer though

@kawamuray
Copy link
Contributor Author

kawamuray commented Aug 11, 2020

i'm not a libs implementer though

oh! I thought you're a right person to review this as rust-highfive selected you automatically.. Thanks for your response anyway.

Let me try mentioning another person from https://www.rust-lang.org/governance/teams/library this list then.

@kawamuray
Copy link
Contributor Author

r? @KodrAus

@rust-highfive rust-highfive assigned KodrAus and unassigned kennytm Aug 11, 2020
@KodrAus
Copy link
Contributor

KodrAus commented Aug 12, 2020

Looks good to me! @bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 12, 2020

📌 Commit 165a6e5 has been approved by KodrAus

@bors 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 Aug 12, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 13, 2020
…rAus

Fix wasi::fs::OpenOptions to imply write when append is on

This PR fixes a bug in `OpenOptions` of `wasi` platform that it currently doesn't imply write mode when only `append` is enabled.
As explained in the [doc of OpenOptions#append](https://doc.rust-lang.org/std/fs/struct.OpenOptions.html#method.append), calling `.append(true)` should imply `.write(true)` as well.

## Reproduce

Given below simple Rust program:

```rust
use std::fs::OpenOptions;
use std::io::Write;

fn main() {
    let mut file = OpenOptions::new()
        .write(true)
        .create(true)
        .open("foo.txt")
        .unwrap();
    writeln!(file, "abc").unwrap();
}
```

it can successfully compiled into wasm and execute by `wasmtime` runtime:

```sh
$ rustc --target wasm32-wasi write.rs
$ ~/wasmtime/target/debug/wasmtime run --dir=. write.wasm
$ cat foo.txt
abc
```

However when I change `.write(true)` to `.append(true)`, it fails to execute by the error "Capabilities insufficient":

```sh
$ ~/wasmtime/target/debug/wasmtime run --dir=. append.wasm
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 76, kind: Other, message: "Capabilities insufficient" }', append.rs:10:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error: failed to run main module `append.wasm`
...
```

This is because of lacking "rights" on the opened file:

```sh
$ RUST_LOG=trace ~/wasmtime/target/debug/wasmtime run --dir=. append.wasm 2>&1 | grep validate_rights
 TRACE wasi_common::entry                                  >      | validate_rights failed: required rights = HandleRights { base: fd_write (0x40), inheriting: empty (0x0) }; actual rights = HandleRights { base: fd_seek|fd_fdstat_set_flags|fd_sync|fd_tell|fd_advise|fd_filestat_set_times|poll_fd_readwrite (0x88000bc), inheriting: empty (0x0) }
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 13, 2020
Rollup of 11 pull requests

Successful merges:

 - rust-lang#75189 (Fix wasi::fs::OpenOptions to imply write when append is on)
 - rust-lang#75201 (Fix some Clippy warnings in librustc_serialize)
 - rust-lang#75372 (Fix suggestion to use lifetime in type and in assoc const)
 - rust-lang#75400 (Fix minor things in the `f32` primitive docs)
 - rust-lang#75449 (add regression test for rust-lang#74739 (mir const-prop bug))
 - rust-lang#75451 (Clean up E0751 explanation)
 - rust-lang#75455 (Use explicit path link in place for doc in time)
 - rust-lang#75457 (Remove some dead variants in LLVM FFI)
 - rust-lang#75466 (Move to intra doc links whenever possible within std/src/lib.rs)
 - rust-lang#75469 (Switch to intra-doc links in `std/io/mod.rs`)
 - rust-lang#75473 (Flip order of const & type)

Failed merges:

r? @ghost
@bors bors merged commit ed543ae into rust-lang:master Aug 13, 2020
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants