feat(rome_fs): implement permission checking in the memory filesystem
MODIFY
crates/rome_cli/tests/snap_test.rs
crates/rome_cli/tests/snap_test.rs
@@ -241,7 +241,7 @@ impl From<SnapshotPayload<'_>> for CliSnapshot {
} = payload;
let mut cli_snapshot = CliSnapshot::from_result(result);
let config_path = PathBuf::from("rome.json");
- let configuration = fs.read(&config_path).ok();
+ let configuration = fs.open(&config_path).ok();
if let Some(mut configuration) = configuration {
let mut buffer = String::new();
if configuration.read_to_string(&mut buffer).is_ok() {

MODIFY
crates/rome_fs/src/fs.rs
crates/rome_fs/src/fs.rs
@@ -46,11 +46,10 @@ pub trait File {
/// This struct is a "mirror" of [std::fs::FileOptions].
/// Refer to their documentation for more details
-#[derive(Default)]
+#[derive(Default, Debug)]
pub struct OpenOptions {
read: bool,
write: bool,
- append: bool,
truncate: bool,
create: bool,
create_new: bool,
@@ -65,10 +64,6 @@ impl OpenOptions {
self.write = write;
self
}
- pub fn append(mut self, append: bool) -> Self {
- self.append = append;
- self
- }
pub fn truncate(mut self, truncate: bool) -> Self {
self.truncate = truncate;
self
@@ -86,7 +81,6 @@ impl OpenOptions {
options
.read(self.read)
.write(self.write)
- .append(self.append)
.truncate(self.truncate)
.create(self.create)
.create_new(self.create_new)
@@ -95,20 +89,42 @@ impl OpenOptions {
/// Trait that contains additional methods to work with [FileSystem]
pub trait FileSystemExt: FileSystem {
- /// Open a file with `read` and `write` options
+ /// Open a file with the `read` option
+ ///
+ /// Equivalent to [std::fs::File::open]
fn open(&self, path: &Path) -> io::Result<Box<dyn File>> {
- self.open_with_options(path, OpenOptions::default().read(true).write(true))
+ self.open_with_options(path, OpenOptions::default().read(true))
}
- /// Open a file with `write` and `create_new` options
+
+ /// Open a file with the `write` and `create` options
+ ///
+ /// Equivalent to [std::fs::File::create]
fn create(&self, path: &Path) -> io::Result<Box<dyn File>> {
- self.open_with_options(path, OpenOptions::default().write(true).create_new(true))
+ self.open_with_options(
+ path,
+ OpenOptions::default()
+ .write(true)
+ .create(true)
+ .truncate(true),
+ )
}
- /// Opens a file with read options
- fn read(&self, path: &Path) -> io::Result<Box<dyn File>> {
- self.open_with_options(path, OpenOptions::default().read(true).create_new(true))
+
+ /// Opens a file with the `read`, `write` and `create_new` options
+ ///
+ /// Equivalent to [std::fs::File::create_new]
+ fn create_new(&self, path: &Path) -> io::Result<Box<dyn File>> {
+ self.open_with_options(
+ path,
+ OpenOptions::default()
+ .read(true)
+ .write(true)
+ .create_new(true),
+ )
}
}
+impl<T: ?Sized> FileSystemExt for T where T: FileSystem {}
+
type BoxedTraversal<'fs, 'scope> = Box<dyn FnOnce(&dyn TraversalScope<'scope>) + Send + 'fs>;
pub trait TraversalScope<'scope> {

MODIFY
crates/rome_fs/src/fs/memory.rs
crates/rome_fs/src/fs/memory.rs
+use std::collections::hash_map::Entry;
use std::collections::{hash_map::IntoIter, HashMap};
use std::io;
use std::panic::AssertUnwindSafe;
@@ -8,7 +9,7 @@ use std::sync::Arc;
use parking_lot::{lock_api::ArcMutexGuard, Mutex, RawMutex, RwLock};
use rome_diagnostics::Error;
-use crate::fs::{FileSystemExt, OpenOptions};
+use crate::fs::OpenOptions;
use crate::{FileSystem, RomePath, TraversalContext, TraversalScope};
use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic};
@@ -71,35 +72,34 @@ impl MemoryFileSystem {
impl FileSystem for MemoryFileSystem {
fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result<Box<dyn File>> {
- if options.read && options.write {
- self.open(path)
- } else if options.create_new || options.write {
- self.create(path)
- } else if options.read {
- self.read(path)
+ let mut inner = if options.create || options.create_new {
+ // Acquire write access to the files map if the file may need to be created
+ let mut files = self.files.0.write();
+ match files.entry(PathBuf::from(path)) {
+ Entry::Vacant(entry) => {
+ // we create an empty file
+ let file: FileEntry = Arc::new(Mutex::new(vec![]));
+ let entry = entry.insert(file);
+ entry.lock_arc()
+ }
+ Entry::Occupied(entry) => {
+ if options.create {
+ // If `create` is true, truncate the file
+ let entry = entry.into_mut();
+ *entry = Arc::new(Mutex::new(vec![]));
+ entry.lock_arc()
} else {
- unimplemented!("the set of open options provided don't match any case")
- }
- }
-
- fn traversal<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>) {
- func(&MemoryTraversalScope { fs: self })
+ // This branch can only be reached if `create_new` was true,
+ // we should return an error if the file already exists
+ return Err(io::Error::new(
+ io::ErrorKind::AlreadyExists,
+ format!("path {path:?} already exists in memory filesystem"),
+ ));
}
}
-
-impl FileSystemExt for MemoryFileSystem {
- fn create(&self, path: &Path) -> io::Result<Box<dyn File>> {
- let files = &mut self.files.0.write();
- // we create an empty file
- let file: FileEntry = Arc::new(Mutex::new(vec![]));
- let path = PathBuf::from(path);
- files.insert(path, file.clone());
- let inner = file.lock_arc();
- Ok(Box::new(MemoryFile { inner }))
}
-
- fn open(&self, path: &Path) -> io::Result<Box<dyn File>> {
- let files = &self.files.0.read();
+ } else {
+ let files = self.files.0.read();
let entry = files.get(path).ok_or_else(|| {
io::Error::new(
io::ErrorKind::NotFound,
@@ -107,32 +107,41 @@ impl FileSystemExt for MemoryFileSystem {
)
})?;
- let lock = entry.lock_arc();
+ entry.lock_arc()
+ };
- Ok(Box::new(MemoryFile { inner: lock }))
+ if options.truncate {
+ // Clear the buffer if the file was open with `truncate`
+ inner.clear();
}
- fn read(&self, path: &Path) -> io::Result<Box<dyn File>> {
- let files = &self.files.0.read();
- let entry = files.get(path).ok_or_else(|| {
- io::Error::new(
- io::ErrorKind::NotFound,
- format!("path {path:?} does not exists in memory filesystem"),
- )
- })?;
-
- let lock = entry.lock_arc();
+ Ok(Box::new(MemoryFile {
+ inner,
+ can_read: options.read,
+ can_write: options.write,
+ }))
+ }
- Ok(Box::new(MemoryFile { inner: lock }))
+ fn traversal<'scope>(&'scope self, func: BoxedTraversal<'_, 'scope>) {
+ func(&MemoryTraversalScope { fs: self })
}
}
struct MemoryFile {
inner: ArcMutexGuard<RawMutex, Vec<u8>>,
+ can_read: bool,
+ can_write: bool,
}
impl File for MemoryFile {
fn read_to_string(&mut self, buffer: &mut String) -> io::Result<()> {
+ if !self.can_read {
+ return Err(io::Error::new(
+ io::ErrorKind::PermissionDenied,
+ "this file wasn't open with read access",
+ ));
+ }
+
// Verify the stored byte content is valid UTF-8
let content = str::from_utf8(&self.inner)
.map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?;
@@ -142,6 +151,13 @@ impl File for MemoryFile {
}
fn set_content(&mut self, content: &[u8]) -> io::Result<()> {
+ if !self.can_write {
+ return Err(io::Error::new(
+ io::ErrorKind::PermissionDenied,
+ "this file wasn't open with write access",
+ ));
+ }
+
// Resize the memory buffer to fit the new content
self.inner.resize(content.len(), 0);
// Copy the new content into the memory buffer
@@ -211,7 +227,7 @@ mod tests {
use parking_lot::Mutex;
use rome_diagnostics::Error;
- use crate::fs::FileSystemExt;
+ use crate::{fs::FileSystemExt, OpenOptions};
use crate::{FileSystem, MemoryFileSystem, PathInterner, RomePath, TraversalContext};
use rome_diagnostics::location::FileId;
@@ -226,7 +242,7 @@ mod tests {
fs.insert(path.into(), content_1.as_bytes());
let mut file = fs
- .open(path)
+ .open_with_options(path, OpenOptions::default().read(true).write(true))
.expect("the file should exist in the memory file system");
let mut buffer = String::new();
@@ -245,6 +261,82 @@ mod tests {
assert_eq!(buffer, content_2);
}
+ #[test]
+ fn file_create() {
+ let fs = MemoryFileSystem::default();
+
+ let path = Path::new("file.js");
+ let mut file = fs.create(path).expect("the file should not fail to open");
+
+ file.set_content(b"content".as_slice())
+ .expect("the file should be written without error");
+ }
+
+ #[test]
+ fn file_create_truncate() {
+ let mut fs = MemoryFileSystem::default();
+
+ let path = Path::new("file.js");
+ fs.insert(path.into(), b"content".as_slice());
+
+ let file = fs.create(path).expect("the file should not fail to create");
+
+ drop(file);
+
+ let mut file = fs.open(path).expect("the file should not fail to open");
+
+ let mut buffer = String::new();
+ file.read_to_string(&mut buffer)
+ .expect("the file should be read without error");
+
+ assert!(
+ buffer.is_empty(),
+ "fs.create() should truncate the file content"
+ );
+ }
+
+ #[test]
+ fn file_create_new() {
+ let fs = MemoryFileSystem::default();
+
+ let path = Path::new("file.js");
+ let content = "content";
+
+ let mut file = fs
+ .create_new(path)
+ .expect("the file should not fail to create");
+
+ file.set_content(content.as_bytes())
+ .expect("the file should be written without error");
+
+ drop(file);
+
+ let mut file = fs.open(path).expect("the file should not fail to open");
+
+ let mut buffer = String::new();
+ file.read_to_string(&mut buffer)
+ .expect("the file should be read without error");
+
+ assert_eq!(buffer, content);
+ }
+
+ #[test]
+ fn file_create_new_exists() {
+ let mut fs = MemoryFileSystem::default();
+
+ let path = Path::new("file.js");
+ fs.insert(path.into(), b"content".as_slice());
+
+ let result = fs.create_new(path);
+
+ match result {
+ Ok(_) => panic!("fs.create_new() for an existing file should return an error"),
+ Err(error) => {
+ assert_eq!(error.kind(), io::ErrorKind::AlreadyExists);
+ }
+ }
+ }
+
#[test]
fn missing_file() {
let fs = MemoryFileSystem::default();

MODIFY
crates/rome_fs/src/fs/os.rs
crates/rome_fs/src/fs/os.rs
//! Implementation of the [FileSystem] and related traits for the underlying OS filesystem
use super::{BoxedTraversal, ErrorKind, File, FileSystemDiagnostic};
-use crate::fs::{FileSystemExt, OpenOptions};
+use crate::fs::OpenOptions;
use crate::{
fs::{TraversalContext, TraversalScope},
FileSystem, RomePath,
@@ -21,10 +21,13 @@ pub struct OsFileSystem;
impl FileSystem for OsFileSystem {
fn open_with_options(&self, path: &Path, options: OpenOptions) -> io::Result<Box<dyn File>> {
+ tracing::debug_span!("OsFileSystem::open_with_options", path = ?path, options = ?options)
+ .in_scope(move || -> io::Result<Box<dyn File>> {
let mut fs_options = fs::File::options();
Ok(Box::new(OsFile {
inner: options.into_fs_options(&mut fs_options).open(path)?,
}))
+ })
}
fn traversal(&self, func: BoxedTraversal) {
@@ -34,24 +37,6 @@ impl FileSystem for OsFileSystem {
}
}
-impl FileSystemExt for OsFileSystem {
- fn create(&self, path: &Path) -> io::Result<Box<dyn File>> {
- tracing::debug_span!("OsFileSystem::create", path = ?path).in_scope(
- move || -> io::Result<Box<dyn File>> {
- self.open_with_options(path, OpenOptions::default().write(true).create_new(true))
- },
- )
- }
-
- fn open(&self, path: &Path) -> io::Result<Box<dyn File>> {
- tracing::debug_span!("OsFileSystem::open", path = ?path).in_scope(
- move || -> io::Result<Box<dyn File>> {
- self.open_with_options(path, OpenOptions::default().read(true).write(true))
- },
- )
- }
-}
-
struct OsFile {
inner: fs::File,
}

feat(rome_fs): implement permission checking in the memory filesystem (#3882)