Skip to content

tempfile: add new_with_modes and is_named methods #390

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

champtar
Copy link

@champtar champtar commented May 26, 2025

On filesystems that do not support O_TMPFILE (e.g. vfat), TempFile::new()
automatically falls back to a potentially world readable named temp file.

Add TempFile::new_with_modes() to create temp files with a chosen mode directly.
We pass 2 modes, one for unnamed tempfile (happy path, created with O_TMPFILE),
and one for named tempfile.

The final mode is still restricted by the process umask.

@cgwalters this will help improve cap-std-ext atomic* permissions handling

@champtar champtar force-pushed the TempFile_new_with_mode branch 4 times, most recently from f1e7b23 to 0c6abfb Compare May 26, 2025 10:06
@champtar
Copy link
Author

Still pretty new to rust, not sure if this is ok to use #[cfg(unix)] for the mode argument, but 'it works (TM)'

Comment on lines 76 to 83
let mode = match mode {
Some(mode) => Mode::from(mode),
None => Mode::from(0o666),
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mode = match mode {
Some(mode) => Mode::from(mode),
None => Mode::from(0o666),
};
let mode = mode.unwrap_or(0o666).map(Mode::from);

would be slightly shorter.

Actually for internal APIs I'd say we should actually use rustic::fs::Mode as much as possible; in other words make this function take a Mode instead. (We can't for public APIs yet as we aren't depending on rustix there yet AFAIK)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now using rustic::fs::Mode

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails for Darwin

error[E0308]: mismatched types
   --> cap-tempfile/src/tempfile.rs:130:19
    |
130 |         opts.mode(mode.as_raw_mode());
    |              ---- ^^^^^^^^^^^^^^^^^^ expected `u32`, found `u16`
    |              |
    |              arguments to this method are incorrect
    |
note: method defined here
   --> /home/runner/work/cap-std/cap-std/cap-primitives/src/fs/open_options.rs:257:8
    |
257 |     fn mode(&mut self, mode: u32) -> &mut Self;
    |        ^^^^
help: you can convert a `u16` to a `u32`
    |
130 |         opts.mode(mode.as_raw_mode().into());
    |                                     +++++++

error[E0277]: the trait bound `Mode: From<u32>` is not satisfied
   --> cap-tempfile/src/tempfile.rs:159:56
    |
159 |         let (fd, name) = new_tempfile(dir, false, Some(rustix::fs::Mode::from(mode)))?;
    |                                                        ^^^^^^^^^^^^^^^^ the trait `From<u32>` is not implemented for `Mode`
    |
    = help: the trait `From<u32>` is not implemented for `Mode`
            but trait `From<u16>` is implemented for it
    = help: for that trait implementation, expected `u16`, found `u32`

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back to u32 for now

@@ -295,6 +325,29 @@ mod test {
eprintln!("notice: Detected older Windows");
}

// Test that we can create with 0o000 mode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unusual, how about 0o200 instead so we can at least write to the file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely unusual, but we can write to the fd as it was open with O_RDWR

@champtar champtar force-pushed the TempFile_new_with_mode branch 3 times, most recently from 242819e to 62a9a6f Compare May 26, 2025 18:58
On filesystems that do not support `O_TMPFILE` (e.g. `vfat`), TempFile::new()
automatically falls back to a potentially world readable named temp file.

Add TempFile::new_with_modes() to create temp files with a chosen mode directly.
We pass 2 modes, one for unnamed tempfile (happy path, created with `O_TMPFILE`),
and one for named tempfile.

The final mode is still restricted by the process umask.
@champtar champtar force-pushed the TempFile_new_with_mode branch from 62a9a6f to 72ee5b6 Compare May 26, 2025 18:59
@champtar champtar changed the title tempfile: add a new_with_mode method tempfile: add new_with_modes and is_named methods May 26, 2025
@champtar
Copy link
Author

champtar commented May 26, 2025

I switched to passing 2 modes (for unnamed and named case) + adding is_named() method.
This will allow to keep the happy path (O_TMPFILE) simpler/faster in coreos/cap-std-ext#73

@champtar champtar marked this pull request as ready for review May 27, 2025 15:19
@champtar champtar requested a review from cgwalters May 29, 2025 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants