Skip to content

Commit 13a16b9

Browse files
committed
Change filesystem errors to be consistent across operating systems
1 parent 33e0808 commit 13a16b9

File tree

5 files changed

+79
-31
lines changed

5 files changed

+79
-31
lines changed

packages/vm/src/cache.rs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use std::collections::HashSet;
2-
use std::fs::{create_dir_all, File, OpenOptions};
2+
use std::fs::{File, OpenOptions};
33
use std::io::{Read, Write};
44
use std::marker::PhantomData;
55
use std::path::{Path, PathBuf};
@@ -10,6 +10,7 @@ use crate::capabilities::required_capabilities_from_module;
1010
use crate::checksum::Checksum;
1111
use crate::compatibility::check_wasm;
1212
use crate::errors::{VmError, VmResult};
13+
use crate::filesystem::mkdir_p;
1314
use crate::instance::{Instance, InstanceOptions};
1415
use crate::modules::{FileSystemCache, InMemoryCache, PinnedMemoryCache};
1516
use crate::size::Size;
@@ -108,15 +109,9 @@ where
108109
let wasm_path = state_path.join(WASM_DIR);
109110

110111
// Ensure all the needed directories exist on disk.
111-
for path in [&state_path, &cache_path, &wasm_path].iter() {
112-
create_dir_all(path).map_err(|e| {
113-
VmError::cache_err(format!(
114-
"Error creating directory {}: {}",
115-
path.display(),
116-
e
117-
))
118-
})?;
119-
}
112+
mkdir_p(&state_path).map_err(|_e| VmError::cache_err("Error creating state directory"))?;
113+
mkdir_p(&cache_path).map_err(|_e| VmError::cache_err("Error creating cache directory"))?;
114+
mkdir_p(&wasm_path).map_err(|_e| VmError::cache_err("Error creating wasm directory"))?;
120115

121116
let fs_cache = FileSystemCache::new(cache_path.join(MODULES_DIR))
122117
.map_err(|e| VmError::cache_err(format!("Error file system cache: {}", e)))?;
@@ -373,7 +368,7 @@ mod tests {
373368
use crate::errors::VmError;
374369
use crate::testing::{mock_backend, mock_env, mock_info, MockApi, MockQuerier, MockStorage};
375370
use cosmwasm_std::{coins, Empty};
376-
use std::fs::OpenOptions;
371+
use std::fs::{create_dir_all, OpenOptions};
377372
use std::io::Write;
378373
use tempfile::TempDir;
379374

packages/vm/src/filesystem.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
use std::{fs::create_dir_all, path::PathBuf};
2+
3+
#[derive(Debug)]
4+
pub struct MkdirPFailure;
5+
6+
/// An implementation for `mkdir -p`.
7+
///
8+
/// This is a thin wrapper around fs::create_dir_all that
9+
/// hides all OS specific error messages to ensure they don't end up
10+
/// breaking consensus.
11+
pub fn mkdir_p(path: &PathBuf) -> Result<(), MkdirPFailure> {
12+
create_dir_all(path).map_err(|_e| MkdirPFailure {})
13+
}
14+
15+
#[cfg(test)]
16+
mod tests {
17+
use tempfile::TempDir;
18+
19+
use super::*;
20+
21+
#[test]
22+
fn mkdir_p_works() {
23+
let tmp_root = TempDir::new().unwrap();
24+
25+
// Can create
26+
let path = tmp_root.path().join("something");
27+
assert!(!path.is_dir());
28+
mkdir_p(&path).unwrap();
29+
assert!(path.is_dir());
30+
31+
// Can be called on existing dir
32+
let path = tmp_root.path().join("something else");
33+
assert!(!path.is_dir());
34+
mkdir_p(&path).unwrap();
35+
assert!(path.is_dir());
36+
mkdir_p(&path).unwrap(); // no-op
37+
assert!(path.is_dir());
38+
39+
// Fails for dir with null
40+
let path = tmp_root.path().join("something\0with NULL");
41+
mkdir_p(&path).unwrap_err();
42+
}
43+
}

packages/vm/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ mod compatibility;
99
mod conversion;
1010
mod environment;
1111
mod errors;
12+
mod filesystem;
1213
mod imports;
1314
mod instance;
1415
mod limited;

packages/vm/src/modules/file_system_cache.rs

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
use std::fs;
21
use std::io;
32
use std::path::PathBuf;
3+
use thiserror::Error;
44

55
use wasmer::{DeserializeError, Module, Store};
66

77
use crate::checksum::Checksum;
88
use crate::errors::{VmError, VmResult};
99

10+
use crate::filesystem::mkdir_p;
1011
use crate::modules::current_wasmer_module_version;
1112

1213
/// Bump this version whenever the module system changes in a way
@@ -42,6 +43,20 @@ pub struct FileSystemCache {
4243
wasmer_module_version: u32,
4344
}
4445

46+
/// An error type that hides system specific error information
47+
/// to ensure deterministic errors across operating systems.
48+
#[derive(Error, Debug)]
49+
pub enum NewFileSystemCacheError {
50+
#[error("Could not get metadata of cache path")]
51+
CouldntGetMetadata,
52+
#[error("The supplied path is readonly")]
53+
ReadonlyPath,
54+
#[error("The supplied path already exists but is no directory")]
55+
ExistsButNoDirectory,
56+
#[error("Could not create cache path")]
57+
CouldntCreatePath,
58+
}
59+
4560
impl FileSystemCache {
4661
/// Construct a new `FileSystemCache` around the specified directory.
4762
/// The contents of the cache are stored in sub-versioned directories.
@@ -50,38 +65,29 @@ impl FileSystemCache {
5065
///
5166
/// This method is unsafe because there's no way to ensure the artifacts
5267
/// stored in this cache haven't been corrupted or tampered with.
53-
pub unsafe fn new(path: impl Into<PathBuf>) -> io::Result<Self> {
68+
pub unsafe fn new(path: impl Into<PathBuf>) -> Result<Self, NewFileSystemCacheError> {
5469
let wasmer_module_version = current_wasmer_module_version();
5570

5671
let path: PathBuf = path.into();
5772
if path.exists() {
58-
let metadata = path.metadata()?;
73+
let metadata = path
74+
.metadata()
75+
.map_err(|_e| NewFileSystemCacheError::CouldntGetMetadata)?;
5976
if metadata.is_dir() {
6077
if !metadata.permissions().readonly() {
6178
Ok(Self {
6279
base_path: path,
6380
wasmer_module_version,
6481
})
6582
} else {
66-
// This directory is readonly.
67-
Err(io::Error::new(
68-
io::ErrorKind::PermissionDenied,
69-
format!("the supplied path is readonly: {}", path.display()),
70-
))
83+
Err(NewFileSystemCacheError::ReadonlyPath)
7184
}
7285
} else {
73-
// This path points to a file.
74-
Err(io::Error::new(
75-
io::ErrorKind::PermissionDenied,
76-
format!(
77-
"the supplied path already points to a file: {}",
78-
path.display()
79-
),
80-
))
86+
Err(NewFileSystemCacheError::ExistsButNoDirectory)
8187
}
8288
} else {
8389
// Create the directory and any parent directories if they don't yet exist.
84-
fs::create_dir_all(&path)?;
90+
mkdir_p(&path).map_err(|_e| NewFileSystemCacheError::CouldntCreatePath)?;
8591
Ok(Self {
8692
base_path: path,
8793
wasmer_module_version,
@@ -115,8 +121,9 @@ impl FileSystemCache {
115121
/// Stores a serialized module to the file system. Returns the size of the serialized module.
116122
pub fn store(&mut self, checksum: &Checksum, module: &Module) -> VmResult<()> {
117123
let modules_dir = self.latest_modules_path();
118-
fs::create_dir_all(&modules_dir)
119-
.map_err(|e| VmError::cache_err(format!("Error creating directory: {}", e)))?;
124+
mkdir_p(&modules_dir)
125+
.map_err(|_e| VmError::cache_err("Error creating modules directory"))?;
126+
120127
let filename = checksum.to_hex();
121128
let path = modules_dir.join(filename);
122129
module
@@ -137,6 +144,8 @@ impl FileSystemCache {
137144

138145
#[cfg(test)]
139146
mod tests {
147+
use std::fs;
148+
140149
use super::*;
141150
use crate::size::Size;
142151
use crate::wasm_backend::{compile, make_runtime_store};

packages/vm/src/modules/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod pinned_memory_cache;
44
mod sized_module;
55
mod versioning;
66

7-
pub use file_system_cache::FileSystemCache;
7+
pub use file_system_cache::{FileSystemCache, NewFileSystemCacheError};
88
pub use in_memory_cache::InMemoryCache;
99
pub use pinned_memory_cache::PinnedMemoryCache;
1010
pub use versioning::current_wasmer_module_version;

0 commit comments

Comments
 (0)