Skip to content

Commit 93d5bfa

Browse files
divybotlittledivy
andauthored
fix(ext/fs): retry without FILE_FLAG_BACKUP_SEMANTICS on Windows when driver rejects it (#34686)
## Summary Some Windows file system drivers — notably ImDisk-backed memory disks — reject `FILE_FLAG_BACKUP_SEMANTICS` for regular files and return `ERROR_INVALID_FUNCTION` (\`os error 1\`). Deno passes that flag so `CreateFile` can open directories with the same code path as files, which causes \`Deno.readTextFile\`, \`Deno.writeTextFile\`, \`Deno.stat\`, \`Deno.lstat\` (and anything that goes through them, such as \`std/fs/exists\`) to fail on those volumes: \`\`\`txt Error: Incorrect function. (os error 1): readfile 'y:\a.txt' Error: Incorrect function. (os error 1): writefile 'y:\test.txt' Error: Incorrect function. (os error 1): stat 'y:\a.txt' \`\`\` This PR transparently retries the open without the flag when we see `ERROR_INVALID_FUNCTION`. Directories will still fail to open on the problematic drivers (they need the backup-semantics flag), but on those drivers only regular-file operations actually need to succeed. Any other error from the original open still propagates unchanged. The fallback covers both code paths that explicitly set the flag on Windows: - `open_with_checked_path` (used by `read_file_*`, `write_file_*`, `open_*`, and the text-file variants that delegate to them). - `stat` / `lstat` (each open the path with `access_mode(0)` plus the flag to grab metadata via `GetFileInformationByHandle` / `NtQueryInformationFile`). ## Test plan - [x] `cargo +1.95.0 check -p deno_fs` (Linux host) - [x] `cargo +1.95.0 clippy -p deno_fs --no-deps` - [x] `cargo +1.95.0 fmt --check -p deno_fs` - [ ] CI Windows job exercises the standard read/write/stat code paths Hard to add an automated test for the ImDisk-only failure mode here since we'd need to provision a memory-disk volume on the Windows CI runner. Fixes #26257 Closes denoland/divybot#406 Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 68236a1 commit 93d5bfa

1 file changed

Lines changed: 94 additions & 43 deletions

File tree

ext/fs/std_fs.rs

Lines changed: 94 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -903,14 +903,7 @@ fn stat(path: &Path) -> FsResult<FsStat> {
903903

904904
#[cfg(windows)]
905905
fn stat(path: &Path) -> FsResult<FsStat> {
906-
use std::os::windows::fs::OpenOptionsExt;
907-
908-
use winapi::um::winbase::FILE_FLAG_BACKUP_SEMANTICS;
909-
910-
let mut opts = fs::OpenOptions::new();
911-
opts.access_mode(0); // no read or write
912-
opts.custom_flags(FILE_FLAG_BACKUP_SEMANTICS);
913-
let file = opts.open(path)?;
906+
let file = open_for_stat_windows(path, false)?;
914907
let metadata = file.metadata()?;
915908
let mut fsstat = FsStat::from_std(metadata);
916909
deno_io::stat_extra(&file, &mut fsstat)?;
@@ -925,21 +918,52 @@ fn lstat(path: &Path) -> FsResult<FsStat> {
925918

926919
#[cfg(windows)]
927920
fn lstat(path: &Path) -> FsResult<FsStat> {
921+
let file = open_for_stat_windows(path, true)?;
922+
let metadata = file.metadata()?;
923+
let mut fsstat = FsStat::from_std(metadata);
924+
deno_io::stat_extra(&file, &mut fsstat)?;
925+
Ok(fsstat)
926+
}
927+
928+
// Some Windows file system drivers (notably ImDisk-backed memory disks)
929+
// reject `FILE_FLAG_BACKUP_SEMANTICS` for regular files and return
930+
// `ERROR_INVALID_FUNCTION` (1). Deno passes that flag so `CreateFile` can
931+
// open directories; on those drivers we transparently retry without it.
932+
// See https://github.com/denoland/deno/issues/26257.
933+
#[cfg(windows)]
934+
fn open_for_stat_windows(
935+
path: &Path,
936+
do_not_follow_symlink: bool,
937+
) -> io::Result<fs::File> {
928938
use std::os::windows::fs::OpenOptionsExt;
929939

930940
use winapi::um::winbase::FILE_FLAG_BACKUP_SEMANTICS;
931941
use winapi::um::winbase::FILE_FLAG_OPEN_REPARSE_POINT;
932942

943+
let reparse_flag = if do_not_follow_symlink {
944+
FILE_FLAG_OPEN_REPARSE_POINT
945+
} else {
946+
0
947+
};
948+
933949
let mut opts = fs::OpenOptions::new();
934950
opts.access_mode(0); // no read or write
935-
opts.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT);
936-
let file = opts.open(path)?;
937-
let metadata = file.metadata()?;
938-
let mut fsstat = FsStat::from_std(metadata);
939-
deno_io::stat_extra(&file, &mut fsstat)?;
940-
Ok(fsstat)
951+
opts.custom_flags(FILE_FLAG_BACKUP_SEMANTICS | reparse_flag);
952+
match opts.open(path) {
953+
Ok(file) => Ok(file),
954+
Err(err) if err.raw_os_error() == Some(ERROR_INVALID_FUNCTION) => {
955+
let mut fallback = fs::OpenOptions::new();
956+
fallback.access_mode(0);
957+
fallback.custom_flags(reparse_flag);
958+
fallback.open(path).map_err(|_| err)
959+
}
960+
Err(err) => Err(err),
961+
}
941962
}
942963

964+
#[cfg(windows)]
965+
const ERROR_INVALID_FUNCTION: i32 = 1;
966+
943967
fn statfs(path: &Path, bigint: bool) -> FsResult<FsStatFs> {
944968
#[cfg(unix)]
945969
{
@@ -1221,39 +1245,66 @@ pub fn open_with_checked_path(
12211245
// if needed, then opening for read only.
12221246
if opts.create && !opts.write && !opts.append {
12231247
if !path.exists() {
1224-
let create_opts = open_options_for_checked_path(
1225-
OpenOptions {
1226-
read: false,
1227-
write: true,
1228-
create: true,
1229-
truncate: false,
1230-
append: false,
1231-
create_new: opts.create_new,
1232-
custom_flags: None,
1233-
mode: opts.mode,
1234-
},
1235-
path,
1236-
);
1237-
// Create and immediately close the file
1238-
drop(create_opts.open(path)?);
1239-
}
1240-
let read_opts = open_options_for_checked_path(
1241-
OpenOptions {
1242-
read: true,
1243-
write: false,
1244-
create: false,
1248+
let create_opts = OpenOptions {
1249+
read: false,
1250+
write: true,
1251+
create: true,
12451252
truncate: false,
12461253
append: false,
1247-
create_new: false,
1248-
custom_flags: opts.custom_flags,
1249-
mode: None,
1250-
},
1251-
path,
1252-
);
1253-
return Ok(read_opts.open(path)?);
1254+
create_new: opts.create_new,
1255+
custom_flags: None,
1256+
mode: opts.mode,
1257+
};
1258+
// Create and immediately close the file
1259+
drop(open_path_with_options(create_opts, path)?);
1260+
}
1261+
let read_opts = OpenOptions {
1262+
read: true,
1263+
write: false,
1264+
create: false,
1265+
truncate: false,
1266+
append: false,
1267+
create_new: false,
1268+
custom_flags: opts.custom_flags,
1269+
mode: None,
1270+
};
1271+
return Ok(open_path_with_options(read_opts, path)?);
12541272
}
1273+
Ok(open_path_with_options(opts, path)?)
1274+
}
1275+
1276+
// Open the path using the configured options. On Windows we set
1277+
// `FILE_FLAG_BACKUP_SEMANTICS` so directories can be opened, but a few
1278+
// filesystem drivers (notably ImDisk-backed memory disks) reject that flag
1279+
// for regular files and return `ERROR_INVALID_FUNCTION` (1). Retry without
1280+
// the flag in that case so reads/writes succeed on those volumes.
1281+
// See https://github.com/denoland/deno/issues/26257.
1282+
fn open_path_with_options(
1283+
opts: OpenOptions,
1284+
path: &CheckedPath,
1285+
) -> io::Result<fs::File> {
12551286
let std_opts = open_options_for_checked_path(opts, path);
1256-
Ok(std_opts.open(path)?)
1287+
match std_opts.open(path) {
1288+
Ok(file) => Ok(file),
1289+
#[cfg(windows)]
1290+
Err(err) if err.raw_os_error() == Some(ERROR_INVALID_FUNCTION) => {
1291+
let fallback = open_options_for_checked_path_no_backup(opts, path);
1292+
fallback.open(path).map_err(|_| err)
1293+
}
1294+
Err(err) => Err(err),
1295+
}
1296+
}
1297+
1298+
#[cfg(windows)]
1299+
fn open_options_for_checked_path_no_backup(
1300+
options: OpenOptions,
1301+
_path: &CheckedPath,
1302+
) -> fs::OpenOptions {
1303+
// Same as open_options_for_checked_path but without
1304+
// FILE_FLAG_BACKUP_SEMANTICS so drivers that reject it can still open
1305+
// regular files. Cannot open directories without the flag, but for the
1306+
// drivers in question that's acceptable.
1307+
open_options(options)
12571308
}
12581309

12591310
#[inline(always)]

0 commit comments

Comments
 (0)