Skip to content

Commit 9489339

Browse files
tgross35tgross35
authored
Rollup merge of #144964 - 0xdeafbeef:fix-open-options, r=ibraheemdev
std: clarify `OpenOptions` error for create without write access Fixes #140621
2 parents 523d399 + 0858b14 commit 9489339

File tree

4 files changed

+91
-22
lines changed

4 files changed

+91
-22
lines changed

‎library/std/src/fs.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1614,6 +1614,10 @@ impl OpenOptions {
16141614
/// See also [`std::fs::write()`][self::write] for a simple function to
16151615
/// create a file with some given data.
16161616
///
1617+
/// # Errors
1618+
///
1619+
/// If `.create(true)` is set without `.write(true)` or `.append(true)`,
1620+
/// calling [`open`](Self::open) will fail with [`InvalidInput`](io::ErrorKind::InvalidInput) error.
16171621
/// # Examples
16181622
///
16191623
/// ```no_run
@@ -1685,7 +1689,8 @@ impl OpenOptions {
16851689
/// * [`AlreadyExists`]: `create_new` was specified and the file already
16861690
/// exists.
16871691
/// * [`InvalidInput`]: Invalid combinations of open options (truncate
1688-
/// without write access, no access mode set, etc.).
1692+
/// without write access, create without write or append access,
1693+
/// no access mode set, etc.).
16891694
///
16901695
/// The following errors don't match any existing [`io::ErrorKind`] at the moment:
16911696
/// * One of the directory components of the specified file path

‎library/std/src/fs/tests.rs

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1265,12 +1265,7 @@ fn open_flavors() {
12651265
let mut ra = OO::new();
12661266
ra.read(true).append(true);
12671267

1268-
#[cfg(windows)]
1269-
let invalid_options = 87; // ERROR_INVALID_PARAMETER
1270-
#[cfg(all(unix, not(target_os = "vxworks")))]
1271-
let invalid_options = "Invalid argument";
1272-
#[cfg(target_os = "vxworks")]
1273-
let invalid_options = "invalid argument";
1268+
let invalid_options = "creating or truncating a file requires write or append access";
12741269

12751270
// Test various combinations of creation modes and access modes.
12761271
//
@@ -1293,10 +1288,10 @@ fn open_flavors() {
12931288
check!(c(&w).open(&tmpdir.join("a")));
12941289

12951290
// read-only
1296-
error!(c(&r).create_new(true).open(&tmpdir.join("b")), invalid_options);
1297-
error!(c(&r).create(true).truncate(true).open(&tmpdir.join("b")), invalid_options);
1298-
error!(c(&r).truncate(true).open(&tmpdir.join("b")), invalid_options);
1299-
error!(c(&r).create(true).open(&tmpdir.join("b")), invalid_options);
1291+
error_contains!(c(&r).create_new(true).open(&tmpdir.join("b")), invalid_options);
1292+
error_contains!(c(&r).create(true).truncate(true).open(&tmpdir.join("b")), invalid_options);
1293+
error_contains!(c(&r).truncate(true).open(&tmpdir.join("b")), invalid_options);
1294+
error_contains!(c(&r).create(true).open(&tmpdir.join("b")), invalid_options);
13001295
check!(c(&r).open(&tmpdir.join("a"))); // try opening the file created with write_only
13011296

13021297
// read-write
@@ -1308,21 +1303,21 @@ fn open_flavors() {
13081303

13091304
// append
13101305
check!(c(&a).create_new(true).open(&tmpdir.join("d")));
1311-
error!(c(&a).create(true).truncate(true).open(&tmpdir.join("d")), invalid_options);
1312-
error!(c(&a).truncate(true).open(&tmpdir.join("d")), invalid_options);
1306+
error_contains!(c(&a).create(true).truncate(true).open(&tmpdir.join("d")), invalid_options);
1307+
error_contains!(c(&a).truncate(true).open(&tmpdir.join("d")), invalid_options);
13131308
check!(c(&a).create(true).open(&tmpdir.join("d")));
13141309
check!(c(&a).open(&tmpdir.join("d")));
13151310

13161311
// read-append
13171312
check!(c(&ra).create_new(true).open(&tmpdir.join("e")));
1318-
error!(c(&ra).create(true).truncate(true).open(&tmpdir.join("e")), invalid_options);
1319-
error!(c(&ra).truncate(true).open(&tmpdir.join("e")), invalid_options);
1313+
error_contains!(c(&ra).create(true).truncate(true).open(&tmpdir.join("e")), invalid_options);
1314+
error_contains!(c(&ra).truncate(true).open(&tmpdir.join("e")), invalid_options);
13201315
check!(c(&ra).create(true).open(&tmpdir.join("e")));
13211316
check!(c(&ra).open(&tmpdir.join("e")));
13221317

13231318
// Test opening a file without setting an access mode
13241319
let mut blank = OO::new();
1325-
error!(blank.create(true).open(&tmpdir.join("f")), invalid_options);
1320+
error_contains!(blank.create(true).open(&tmpdir.join("f")), invalid_options);
13261321

13271322
// Test write works
13281323
check!(check!(File::create(&tmpdir.join("h"))).write("foobar".as_bytes()));
@@ -2084,3 +2079,34 @@ fn test_rename_junction() {
20842079
// Junction links are always absolute so we just check the file name is correct.
20852080
assert_eq!(fs::read_link(&dest).unwrap().file_name(), Some(not_exist.as_os_str()));
20862081
}
2082+
2083+
#[test]
2084+
fn test_open_options_invalid_combinations() {
2085+
use crate::fs::OpenOptions as OO;
2086+
2087+
let test_cases: &[(fn() -> OO, &str)] = &[
2088+
(|| OO::new().create(true).read(true).clone(), "create without write"),
2089+
(|| OO::new().create_new(true).read(true).clone(), "create_new without write"),
2090+
(|| OO::new().truncate(true).read(true).clone(), "truncate without write"),
2091+
(|| OO::new().truncate(true).append(true).clone(), "truncate with append"),
2092+
];
2093+
2094+
for (make_opts, desc) in test_cases {
2095+
let opts = make_opts();
2096+
let result = opts.open("nonexistent.txt");
2097+
assert!(result.is_err(), "{desc} should fail");
2098+
let err = result.unwrap_err();
2099+
assert_eq!(err.kind(), ErrorKind::InvalidInput, "{desc} - wrong error kind");
2100+
assert_eq!(
2101+
err.to_string(),
2102+
"creating or truncating a file requires write or append access",
2103+
"{desc} - wrong error message"
2104+
);
2105+
}
2106+
2107+
let result = OO::new().open("nonexistent.txt");
2108+
assert!(result.is_err(), "no access mode should fail");
2109+
let err = result.unwrap_err();
2110+
assert_eq!(err.kind(), ErrorKind::InvalidInput);
2111+
assert_eq!(err.to_string(), "must specify at least one of read, write, or append access");
2112+
}

‎library/std/src/sys/fs/unix.rs

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,21 @@ impl OpenOptions {
11231123
(true, true, false) => Ok(libc::O_RDWR),
11241124
(false, _, true) => Ok(libc::O_WRONLY | libc::O_APPEND),
11251125
(true, _, true) => Ok(libc::O_RDWR | libc::O_APPEND),
1126-
(false, false, false) => Err(Error::from_raw_os_error(libc::EINVAL)),
1126+
(false, false, false) => {
1127+
// If no access mode is set, check if any creation flags are set
1128+
// to provide a more descriptive error message
1129+
if self.create || self.create_new || self.truncate {
1130+
Err(io::Error::new(
1131+
io::ErrorKind::InvalidInput,
1132+
"creating or truncating a file requires write or append access",
1133+
))
1134+
} else {
1135+
Err(io::Error::new(
1136+
io::ErrorKind::InvalidInput,
1137+
"must specify at least one of read, write, or append access",
1138+
))
1139+
}
1140+
}
11271141
}
11281142
}
11291143

@@ -1132,12 +1146,18 @@ impl OpenOptions {
11321146
(true, false) => {}
11331147
(false, false) => {
11341148
if self.truncate || self.create || self.create_new {
1135-
return Err(Error::from_raw_os_error(libc::EINVAL));
1149+
return Err(io::Error::new(
1150+
io::ErrorKind::InvalidInput,
1151+
"creating or truncating a file requires write or append access",
1152+
));
11361153
}
11371154
}
11381155
(_, true) => {
11391156
if self.truncate && !self.create_new {
1140-
return Err(Error::from_raw_os_error(libc::EINVAL));
1157+
return Err(io::Error::new(
1158+
io::ErrorKind::InvalidInput,
1159+
"creating or truncating a file requires write or append access",
1160+
));
11411161
}
11421162
}
11431163
}

‎library/std/src/sys/fs/windows.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,19 @@ impl OpenOptions {
258258
Ok(c::GENERIC_READ | (c::FILE_GENERIC_WRITE & !c::FILE_WRITE_DATA))
259259
}
260260
(false, false, false, None) => {
261-
Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32))
261+
// If no access mode is set, check if any creation flags are set
262+
// to provide a more descriptive error message
263+
if self.create || self.create_new || self.truncate {
264+
Err(io::Error::new(
265+
io::ErrorKind::InvalidInput,
266+
"creating or truncating a file requires write or append access",
267+
))
268+
} else {
269+
Err(io::Error::new(
270+
io::ErrorKind::InvalidInput,
271+
"must specify at least one of read, write, or append access",
272+
))
273+
}
262274
}
263275
}
264276
}
@@ -268,12 +280,18 @@ impl OpenOptions {
268280
(true, false) => {}
269281
(false, false) => {
270282
if self.truncate || self.create || self.create_new {
271-
return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32));
283+
return Err(io::Error::new(
284+
io::ErrorKind::InvalidInput,
285+
"creating or truncating a file requires write or append access",
286+
));
272287
}
273288
}
274289
(_, true) => {
275290
if self.truncate && !self.create_new {
276-
return Err(Error::from_raw_os_error(c::ERROR_INVALID_PARAMETER as i32));
291+
return Err(io::Error::new(
292+
io::ErrorKind::InvalidInput,
293+
"creating or truncating a file requires write or append access",
294+
));
277295
}
278296
}
279297
}

0 commit comments

Comments
 (0)