From bb06e3344dc99aad9edf26b666d3502d26ab5254 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Mon, 28 Apr 2025 19:17:34 -0700 Subject: [PATCH 1/2] Make Drive deal with bytes --- client/src/drive.rs | 14 ++++---------- repl/src/demos.rs | 6 +++--- repl/src/lib.rs | 10 +++++++++- std/src/program.rs | 15 ++++++++++++++- std/src/storage/fs.rs | 10 +++++----- std/src/storage/mem.rs | 6 +++--- std/src/storage/mod.rs | 8 ++++---- std/src/testutils.rs | 3 ++- 8 files changed, 44 insertions(+), 28 deletions(-) diff --git a/client/src/drive.rs b/client/src/drive.rs index 627db7c3..565986a6 100644 --- a/client/src/drive.rs +++ b/client/src/drive.rs @@ -61,18 +61,12 @@ impl Drive for CloudDrive { )) } - async fn get(&self, filename: &str) -> io::Result { + async fn get(&self, filename: &str) -> io::Result> { let request = GetFileRequest::default().with_get_content(); let response = self.service.borrow_mut().get_file(&self.username, filename, &request).await?; match response.decoded_content()? { - Some(content) => match String::from_utf8(content) { - Ok(s) => Ok(s), - Err(e) => Err(io::Error::new( - io::ErrorKind::InvalidData, - format!("Requested file is not valid UTF-8: {}", e), - )), - }, + Some(content) => Ok(content), None => Err(io::Error::new( io::ErrorKind::InvalidData, "Server response is missing the file content".to_string(), @@ -93,8 +87,8 @@ impl Drive for CloudDrive { } } - async fn put(&mut self, filename: &str, content: &str) -> io::Result<()> { - let request = PatchFileRequest::default().with_content(content.as_bytes()); + async fn put(&mut self, filename: &str, content: &[u8]) -> io::Result<()> { + let request = PatchFileRequest::default().with_content(content); self.service.borrow_mut().patch_file(&self.username, filename, &request).await } diff --git a/repl/src/demos.rs b/repl/src/demos.rs index aa990895..1f929fd4 100644 --- a/repl/src/demos.rs +++ b/repl/src/demos.rs @@ -125,18 +125,18 @@ impl Drive for DemosDrive { Ok(DriveFiles::new(entries, disk_quota, disk_free)) } - async fn get(&self, name: &str) -> io::Result { + async fn get(&self, name: &str) -> io::Result> { let uc_name = name.to_ascii_uppercase(); match self.demos.get(&uc_name.as_ref()) { Some(value) => { let (_metadata, content) = value; - Ok(content.to_string()) + Ok(content.as_bytes().to_owned()) } None => Err(io::Error::new(io::ErrorKind::NotFound, "Demo not found")), } } - async fn put(&mut self, _name: &str, _content: &str) -> io::Result<()> { + async fn put(&mut self, _name: &str, _content: &[u8]) -> io::Result<()> { Err(io::Error::new(io::ErrorKind::PermissionDenied, "The demos drive is read-only")) } } diff --git a/repl/src/lib.rs b/repl/src/lib.rs index 7f5f2f71..8201cac5 100644 --- a/repl/src/lib.rs +++ b/repl/src/lib.rs @@ -70,7 +70,7 @@ pub async fn try_load_autoexec( } }; - match machine.exec(&mut code.as_bytes()).await { + match machine.exec(&mut code.as_slice()).await { Ok(_) => Ok(()), Err(e) => { console.borrow_mut().print(&format!("AUTOEXEC.BAS failed: {}", e))?; @@ -107,6 +107,14 @@ pub async fn run_from_cloud( console.borrow_mut().print(&format!("Loading {}...", path))?; let content = storage.borrow().get(&path).await?; + let content = match String::from_utf8(content) { + Ok(text) => text, + Err(e) => { + let mut console = console.borrow_mut(); + console.print(&format!("Invalid program to run '{}': {}", path, e))?; + return Ok(1); + } + }; program.borrow_mut().load(Some(&path), &content); console.borrow_mut().print("Starting...")?; diff --git a/std/src/program.rs b/std/src/program.rs index ee251490..2e702c5d 100644 --- a/std/src/program.rs +++ b/std/src/program.rs @@ -394,6 +394,15 @@ impl Callable for LoadCommand { .make_canonical_with_extension(&pathname, DEFAULT_EXTENSION) .map_err(|e| scope.io_error(e))?; let content = storage.get(&full_name).await.map_err(|e| scope.io_error(e))?; + let content = match String::from_utf8(content) { + Ok(text) => text, + Err(e) => { + return Err(scope.io_error(io::Error::new( + io::ErrorKind::InvalidData, + format!("Invalid file content: {}", e), + ))); + } + }; (full_name, content) }; self.program.borrow_mut().load(Some(&full_name), &content); @@ -593,7 +602,11 @@ impl Callable for SaveCommand { .make_canonical_with_extension(&name, DEFAULT_EXTENSION) .map_err(|e| scope.io_error(e))?; let content = self.program.borrow().text(); - self.storage.borrow_mut().put(&full_name, &content).await.map_err(|e| scope.io_error(e))?; + self.storage + .borrow_mut() + .put(&full_name, content.as_bytes()) + .await + .map_err(|e| scope.io_error(e))?; self.program.borrow_mut().set_name(&full_name); self.console diff --git a/std/src/storage/fs.rs b/std/src/storage/fs.rs index 6ba79665..ebdc3b5d 100644 --- a/std/src/storage/fs.rs +++ b/std/src/storage/fs.rs @@ -98,18 +98,18 @@ impl Drive for DirectoryDrive { Ok(DriveFiles::new(entries, None, None)) } - async fn get(&self, name: &str) -> io::Result { + async fn get(&self, name: &str) -> io::Result> { let path = self.dir.join(name); let input = File::open(path)?; - let mut content = String::new(); - io::BufReader::new(input).read_to_string(&mut content)?; + let mut content = vec![]; + io::BufReader::new(input).read_to_end(&mut content)?; Ok(content) } - async fn put(&mut self, name: &str, content: &str) -> io::Result<()> { + async fn put(&mut self, name: &str, content: &[u8]) -> io::Result<()> { let path = self.dir.join(name); let mut output = OpenOptions::new().create(true).write(true).truncate(true).open(path)?; - output.write_all(content.as_bytes())?; + output.write_all(content)?; output.sync_all() } diff --git a/std/src/storage/mem.rs b/std/src/storage/mem.rs index a747769a..8156b15f 100644 --- a/std/src/storage/mem.rs +++ b/std/src/storage/mem.rs @@ -24,7 +24,7 @@ use std::str; /// A drive that records all data in memory only. #[derive(Default)] pub struct InMemoryDrive { - programs: HashMap)>, + programs: HashMap, HashSet)>, // TODO(jmmv): These fields are currently exposed only to allow testing for the consumers of // these details and are not enforced in the drive. It might be nice to actually implement @@ -52,7 +52,7 @@ impl Drive for InMemoryDrive { Ok(DriveFiles::new(entries, self.fake_disk_quota, self.fake_disk_free)) } - async fn get(&self, name: &str) -> io::Result { + async fn get(&self, name: &str) -> io::Result> { match self.programs.get(name) { Some((content, _readers)) => Ok(content.to_owned()), None => Err(io::Error::new(io::ErrorKind::NotFound, "Entry not found")), @@ -72,7 +72,7 @@ impl Drive for InMemoryDrive { } } - async fn put(&mut self, name: &str, content: &str) -> io::Result<()> { + async fn put(&mut self, name: &str, content: &[u8]) -> io::Result<()> { if let Some((prev_content, _readers)) = self.programs.get_mut(name) { content.clone_into(prev_content); return Ok(()); diff --git a/std/src/storage/mod.rs b/std/src/storage/mod.rs index aec89863..9ad78d2c 100644 --- a/std/src/storage/mod.rs +++ b/std/src/storage/mod.rs @@ -150,7 +150,7 @@ pub trait Drive { async fn enumerate(&self) -> io::Result; /// Loads the contents of the program given by `name`. - async fn get(&self, name: &str) -> io::Result; + async fn get(&self, name: &str) -> io::Result>; /// Gets the ACLs of the file `_name`. async fn get_acls(&self, _name: &str) -> io::Result { @@ -158,7 +158,7 @@ pub trait Drive { } /// Saves the in-memory program given by `content` into `name`. - async fn put(&mut self, name: &str, content: &str) -> io::Result<()>; + async fn put(&mut self, name: &str, content: &[u8]) -> io::Result<()>; /// Updates the ACLs of the file `_name` by extending them with the contents of `_add` and /// removing the existing entries listed in `_remove`. @@ -589,7 +589,7 @@ impl Storage { } /// Loads the contents of the program given by `raw_location`. - pub async fn get(&self, raw_location: &str) -> io::Result { + pub async fn get(&self, raw_location: &str) -> io::Result> { let location = Location::new(raw_location)?; match location.leaf_name() { Some(name) => self.get_drive(&location)?.get(name).await, @@ -613,7 +613,7 @@ impl Storage { } /// Saves the in-memory program given by `content` into `raw_location`. - pub async fn put(&mut self, raw_location: &str, content: &str) -> io::Result<()> { + pub async fn put(&mut self, raw_location: &str, content: &[u8]) -> io::Result<()> { let location = Location::new(raw_location)?; match location.leaf_name() { Some(name) => self.get_drive_mut(&location)?.put(name, content).await, diff --git a/std/src/testutils.rs b/std/src/testutils.rs index 23a96f40..28fc10cf 100644 --- a/std/src/testutils.rs +++ b/std/src/testutils.rs @@ -501,7 +501,7 @@ impl Tester { /// Creates or overwrites a file in the storage medium. pub fn write_file(self, name: &str, content: &str) -> Self { - block_on(self.storage.borrow_mut().put(name, content)).unwrap(); + block_on(self.storage.borrow_mut().put(name, content.as_bytes())).unwrap(); self } @@ -748,6 +748,7 @@ impl<'a> Checker<'a> { for name in block_on(storage.enumerate(&root)).unwrap().dirents().keys() { let path = format!("{}{}", root, name); let content = block_on(storage.get(&path)).unwrap(); + let content = String::from_utf8(content).unwrap(); files.insert(path, content); } } From 5adaa6f4ad6b0e61033c85fad3a1db57735aba13 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Mon, 28 Apr 2025 19:06:22 -0700 Subject: [PATCH 2/2] Add the COPY command --- std/src/storage/cmds.rs | 58 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/std/src/storage/cmds.rs b/std/src/storage/cmds.rs index ce55ee0c..80f6af6d 100644 --- a/std/src/storage/cmds.rs +++ b/std/src/storage/cmds.rs @@ -165,6 +165,63 @@ impl Callable for CdCommand { } } +/// The `COPY` command. +pub struct CopyCommand { + metadata: CallableMetadata, + storage: Rc>, +} + +impl CopyCommand { + /// Creates a new `COPY` command that copies a file. + pub fn new(storage: Rc>) -> Rc { + Rc::from(Self { + metadata: CallableMetadataBuilder::new("COPY") + .with_syntax(&[( + &[ + SingularArgSyntax::RequiredValue( + RequiredValueSyntax { + name: Cow::Borrowed("src"), + vtype: ExprType::Text, + }, + ArgSepSyntax::Exactly(ArgSep::Long), + ), + SingularArgSyntax::RequiredValue( + RequiredValueSyntax { + name: Cow::Borrowed("dest"), + vtype: ExprType::Text, + }, + ArgSepSyntax::End, + ), + ], + None, + )]) + .with_category(CATEGORY) + .with_description("Copies src to dest.") + .build(), + storage, + }) + } +} + +#[async_trait(?Send)] +impl Callable for CopyCommand { + fn metadata(&self) -> &CallableMetadata { + &self.metadata + } + + async fn exec(&self, mut scope: Scope<'_>, _machine: &mut Machine) -> Result<()> { + debug_assert_eq!(2, scope.nargs()); + let src = scope.pop_string(); + let dest = scope.pop_string(); + + let mut storage = self.storage.borrow_mut(); + let content = storage.get(&src).await.map_err(|e| scope.io_error(e))?; + storage.put(&dest, &content).await.map_err(|e| scope.io_error(e))?; + + Ok(()) + } +} + /// The `DIR` command. pub struct DirCommand { metadata: CallableMetadata, @@ -403,6 +460,7 @@ pub fn add_all( storage: Rc>, ) { machine.add_callable(CdCommand::new(storage.clone())); + machine.add_callable(CopyCommand::new(storage.clone())); machine.add_callable(DirCommand::new(console.clone(), storage.clone())); machine.add_callable(MountCommand::new(console.clone(), storage.clone())); machine.add_callable(PwdCommand::new(console.clone(), storage.clone()));