Skip to content

Commit

Permalink
Fix path_open trailing slash edge case
Browse files Browse the repository at this point in the history
`path_open` discards trailing slashes when path_open is creating a new
file.  This is a bug because trailing slash is semantically
meaningful. Other runtimes like WasmEdge, Wazero, WAMR, Wasmtime, and
Node correctly preserves the trailing slash.

fixes #4833
  • Loading branch information
yagehu committed Jun 11, 2024
1 parent fb9a04b commit 1dcabe1
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/wasix/src/syscalls/wasi/path_open.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,12 @@ pub(crate) fn path_open_internal(
if o_flags.contains(Oflags::DIRECTORY) {
return Ok(Err(Errno::Notdir));
}

// Trailing slash matters. But the underlying opener normalizes it away later.
if path.ends_with('/') {
return Ok(Err(Errno::Isdir));
}

// strip end file name

let (parent_inode, new_entity_name) =
Expand Down
20 changes: 20 additions & 0 deletions tests/wasi-fyi/fs_open_trailing_slash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,18 @@ extern "C" {
}

const ERRNO_SUCCESS: i32 = 0;
const ERRNO_ISDIR: i32 = 31;
const ERRNO_NOTDIR: i32 = 54;
const OFLAGS_CREAT: i32 = 1;
const RIGHTS_FD_READ: i64 = 2;
const RIGHTS_FD_WRITE: i64 = 64;

fn main() {
unsafe {
let fd = 5;
let path_ok = "fyi/fs_open_trailing_slash.dir/file";
let path_bad = "fyi/fs_open_trailing_slash.dir/file/";
let path_bad_new_file = "fyi/fs_open_trailing_slash.dir/new-file/";
let errno = path_open(
fd,
0,
Expand Down Expand Up @@ -53,5 +57,21 @@ fn main() {
errno, ERRNO_NOTDIR,
"opening a regular file with a trailing slash should fail"
);

let errno = path_open(
fd,
0,
path_bad_new_file.as_ptr() as i32,
path_bad_new_file.len() as i32,
OFLAGS_CREAT,
RIGHTS_FD_READ | RIGHTS_FD_WRITE,
0,
0,
1024,
);
assert_eq!(
errno, ERRNO_ISDIR,
"creating a regular file with a trailing slash should fail"
);
}
}

0 comments on commit 1dcabe1

Please sign in to comment.