Skip to content

Commit

Permalink
fix: Prevent invalid file system merging errors
Browse files Browse the repository at this point in the history
This commit changes the EmptyFileSystem read_dir() implementation to
always return an empty iterator when trying to read the root path "/".

This is done because some code paths expect the root to always be
readable.

This also makes sense theoretically, because an empty file system should
still be readable, just not contain any files.
  • Loading branch information
theduke committed Sep 6, 2023
1 parent cf51f38 commit 44ce9db
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 8 deletions.
24 changes: 22 additions & 2 deletions lib/virtual-fs/src/empty_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@ pub struct EmptyFileSystem {}
#[allow(unused_variables)]
impl FileSystem for EmptyFileSystem {
fn read_dir(&self, path: &Path) -> Result<ReadDir> {
Err(FsError::EntryNotFound)
// Special-case the root path by returning an empty iterator.
// An empty file system should still be readable, just not contain
// any entries.
if path == Path::new("/") {
Ok(ReadDir::new(Vec::new()))
} else {
Err(FsError::EntryNotFound)
}
}

fn create_dir(&self, path: &Path) -> Result<()> {
Expand All @@ -29,7 +36,20 @@ impl FileSystem for EmptyFileSystem {
}

fn metadata(&self, path: &Path) -> Result<Metadata> {
Err(FsError::EntryNotFound)
// Special-case the root path by returning an stub value.
// An empty file system should still be readable, just not contain
// any entries.
if path == Path::new("/") {
Ok(Metadata {
ft: FileType::new_dir(),
accessed: 0,
created: 0,
modified: 0,
len: 0,
})
} else {
Err(FsError::EntryNotFound)
}
}

fn symlink_metadata(&self, path: &Path) -> Result<Metadata> {
Expand Down
7 changes: 7 additions & 0 deletions lib/virtual-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,13 @@ pub struct FileType {
}

impl FileType {
pub fn new_dir() -> Self {
Self {
dir: true,
..Default::default()
}
}

pub fn is_dir(&self) -> bool {
self.dir
}
Expand Down
8 changes: 7 additions & 1 deletion lib/wasix/src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,13 @@ async fn merge_filesystems(
to_check.push_back(PathBuf::from("/"));

while let Some(path) = to_check.pop_front() {
let metadata = source.metadata(&path)?;
let metadata = match source.metadata(&path) {
Ok(m) => m,
Err(err) => {
tracing::debug!(path=%path.display(), source_fs=?source, ?err, "failed to get metadata for path while merging file systems");
return Err(err);
}
};

if metadata.is_dir() {
create_dir_all(destination, &path)?;
Expand Down
9 changes: 4 additions & 5 deletions lib/wasix/src/os/console/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,14 @@ impl Console {
.with_envs(self.env.clone().into_iter())
.with_args(args)
.with_capabilities(self.capabilities.clone())
.with_stdin(Box::new(self.stdin.clone()))
.with_stdout(Box::new(self.stdout.clone()))
.with_stderr(Box::new(self.stderr.clone()))
.prepare_webc_env(prog, &wasi_opts, &pkg, self.runtime.clone(), Some(root_fs))
// TODO: better error conversion
.map_err(|err| SpawnError::Other(err.into()))?;

let env = builder
.stdin(Box::new(self.stdin.clone()))
.stdout(Box::new(self.stdout.clone()))
.stderr(Box::new(self.stderr.clone()))
.build()?;
let env = builder.build()?;

// Display the welcome message
if !self.whitelabel && !self.no_welcome {
Expand Down
1 change: 1 addition & 0 deletions lib/wasix/src/state/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ impl WasiEnv {
/// [cmd-atom]: crate::bin_factory::BinaryPackageCommand::atom()
/// [pkg-fs]: crate::bin_factory::BinaryPackage::webc_fs
pub fn use_package(&self, pkg: &BinaryPackage) -> Result<(), WasiStateCreationError> {
tracing::trace!(packagae=%pkg.package_name, "merging package dependency into wasi environment");
let root_fs = &self.state.fs.root_fs;

// We first need to copy any files in the package over to the
Expand Down

0 comments on commit 44ce9db

Please sign in to comment.