From 966239eb3887e9b0db23fd5fa95ad75ca408af59 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 15 Dec 2023 00:03:08 +0800 Subject: [PATCH 1/4] fix: Reading from a `virtual_fs::StaticFile` now updates its internal cursor fix: Trying to modify a `virtual_fs::StaticFile` now returns a permission error instead of silently doing nothing --- lib/virtual-fs/src/static_file.rs | 136 ++++++++++++++++-------------- 1 file changed, 71 insertions(+), 65 deletions(-) diff --git a/lib/virtual-fs/src/static_file.rs b/lib/virtual-fs/src/static_file.rs index f57b10c8190..b87c50b12be 100644 --- a/lib/virtual-fs/src/static_file.rs +++ b/lib/virtual-fs/src/static_file.rs @@ -1,27 +1,23 @@ +use std::{ + convert::TryInto, + io::{self, Cursor}, + pin::Pin, + task::{Context, Poll}, +}; + use futures::future::BoxFuture; +use shared_buffer::OwnedBuffer; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; -use std::borrow::Cow; -use std::convert::TryInto; -use std::io::{self, SeekFrom}; -use std::pin::Pin; -use std::task::{Context, Poll}; - use crate::{FsError, VirtualFile}; -#[derive(Debug)] -pub struct StaticFile { - bytes: Cow<'static, [u8]>, - cursor: u64, - len: u64, -} +/// An immutable file backed by an [`OwnedBuffer`]. +#[derive(Debug, Clone, PartialEq)] +pub struct StaticFile(Cursor); + impl StaticFile { - pub fn new(bytes: Cow<'static, [u8]>) -> Self { - Self { - len: bytes.len() as u64, - bytes, - cursor: 0, - } + pub fn new(bytes: impl Into) -> Self { + StaticFile(Cursor::new(bytes.into())) } } @@ -30,49 +26,44 @@ impl VirtualFile for StaticFile { fn last_accessed(&self) -> u64 { 0 } + fn last_modified(&self) -> u64 { 0 } + fn created_time(&self) -> u64 { 0 } + fn size(&self) -> u64 { - self.len + self.0.get_ref().len().try_into().unwrap() } - fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { - Ok(()) + + fn set_len(&mut self, _new_size: u64) -> Result<(), FsError> { + Err(FsError::PermissionDenied) } + fn unlink(&mut self) -> BoxFuture<'static, Result<(), FsError>> { - Box::pin(async { Ok(()) }) + Box::pin(async { Err(FsError::PermissionDenied) }) } + fn poll_read_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { - let remaining = self.len - self.cursor; - Poll::Ready(Ok(remaining as usize)) + let remaining = self.0.position() - self.size(); + Poll::Ready(Ok(remaining.try_into().unwrap())) } + fn poll_write_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(0)) + Poll::Ready(Err(std::io::ErrorKind::PermissionDenied.into())) } } impl AsyncRead for StaticFile { fn poll_read( - self: Pin<&mut Self>, - _cx: &mut Context<'_>, + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, buf: &mut tokio::io::ReadBuf<'_>, ) -> Poll> { - let bytes = self.bytes.as_ref(); - - let cursor: usize = self.cursor.try_into().unwrap_or(u32::MAX as usize); - let _start = cursor.min(bytes.len()); - let bytes = &bytes[cursor..]; - - if bytes.len() > buf.remaining() { - let remaining = buf.remaining(); - buf.put_slice(&bytes[..remaining]); - } else { - buf.put_slice(bytes); - } - Poll::Ready(Ok(())) + Pin::new(&mut self.0).poll_read(cx, buf) } } @@ -82,9 +73,9 @@ impl AsyncWrite for StaticFile { fn poll_write( self: Pin<&mut Self>, _cx: &mut Context<'_>, - buf: &[u8], + _buf: &[u8], ) -> Poll> { - Poll::Ready(Ok(buf.len())) + Poll::Ready(Err(std::io::ErrorKind::PermissionDenied.into())) } fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { Poll::Ready(Ok(())) @@ -96,28 +87,43 @@ impl AsyncWrite for StaticFile { impl AsyncSeek for StaticFile { fn start_seek(mut self: Pin<&mut Self>, pos: io::SeekFrom) -> io::Result<()> { - let self_size = self.size(); - match pos { - SeekFrom::Start(s) => { - self.cursor = s.min(self_size); - } - SeekFrom::End(e) => { - let self_size_i64 = self_size.try_into().unwrap_or(i64::MAX); - self.cursor = ((self_size_i64).saturating_add(e)) - .min(self_size_i64) - .try_into() - .unwrap_or(i64::MAX as u64); - } - SeekFrom::Current(c) => { - self.cursor = (self - .cursor - .saturating_add(c.try_into().unwrap_or(i64::MAX as u64))) - .min(self_size); - } - } - Ok(()) - } - fn poll_complete(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { - Poll::Ready(Ok(self.cursor)) + Pin::new(&mut self.0).start_seek(pos) + } + + fn poll_complete(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.0).poll_complete(cx) + } +} + +#[cfg(test)] +mod tests { + use tokio::io::AsyncReadExt; + + use super::*; + + #[tokio::test] + async fn read_a_static_file_to_end() { + let mut file = StaticFile::new(OwnedBuffer::from_static(b"Hello, World!")); + let mut buffer = [0; 5]; + + let bytes_read = file.read(&mut buffer).await.unwrap(); + assert_eq!(bytes_read, 5); + assert_eq!(&buffer[..bytes_read], b"Hello"); + assert_eq!(file.0.position(), 5); + + let bytes_read = file.read(&mut buffer).await.unwrap(); + assert_eq!(bytes_read, 5); + assert_eq!(&buffer[..bytes_read], b", Wor"); + assert_eq!(file.0.position(), 10); + + let bytes_read = file.read(&mut buffer).await.unwrap(); + assert_eq!(bytes_read, 3); + assert_eq!(&buffer[..bytes_read], b"ld!"); + assert_eq!(file.0.position(), 13); + + let bytes_read = file.read(&mut buffer).await.unwrap(); + assert_eq!(bytes_read, 0); + assert_eq!(&buffer[..bytes_read], b""); + assert_eq!(file.0.position(), 13); } } From b36378a4512ee46138f21508ae9efb7a33762ee7 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 15 Dec 2023 00:13:27 +0800 Subject: [PATCH 2/4] Bumping the `shared-buffer` dependency so `virtual_fs::StaticFile` can use an `OwnedBuffer` while not breaking users of `StaticFile::new()` --- Cargo.lock | 5 +++-- Cargo.toml | 1 + lib/api/Cargo.toml | 2 +- lib/compiler/Cargo.toml | 2 +- lib/virtual-fs/Cargo.toml | 27 ++++++++++++++------------- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 848839e8714..6f919f2200c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4182,9 +4182,9 @@ dependencies = [ [[package]] name = "shared-buffer" -version = "0.1.3" +version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cf61602ee61e2f83dd016b3e6387245291cf728ea071c378b35088125b4d995" +checksum = "f6c99835bad52957e7aa241d3975ed17c1e5f8c92026377d117a606f36b84b16" dependencies = [ "bytes", "memmap2 0.6.2", @@ -5155,6 +5155,7 @@ dependencies = [ "pin-project-lite", "pretty_assertions", "replace_with", + "shared-buffer", "slab", "tempfile", "thiserror", diff --git a/Cargo.toml b/Cargo.toml index 241e34bd371..f0cfb60e128 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -87,6 +87,7 @@ enumset = "1.1.0" memoffset = "0.9.0" wasmer-toml = "0.9.2" webc = { version = "5.8.0", default-features = false, features = ["package"] } +shared-buffer = "0.1.4" [build-dependencies] test-generator = { path = "tests/lib/test-generator" } diff --git a/lib/api/Cargo.toml b/lib/api/Cargo.toml index f69645a66ab..84e84995cca 100644 --- a/lib/api/Cargo.toml +++ b/lib/api/Cargo.toml @@ -34,7 +34,7 @@ bytes = "1" wat = { version = "=1.0.71", optional = true } tracing = { version = "0.1", optional = true } rustc-demangle = "0.1" -shared-buffer = "0.1" +shared-buffer = { workspace = true } # Dependencies and Development Dependencies for `sys`. [target.'cfg(not(target_arch = "wasm32"))'.dependencies] diff --git a/lib/compiler/Cargo.toml b/lib/compiler/Cargo.toml index ad01b64eb74..e6c350ba6f3 100644 --- a/lib/compiler/Cargo.toml +++ b/lib/compiler/Cargo.toml @@ -35,7 +35,7 @@ enum-iterator = "0.7.0" bytes = "1.0" self_cell = "1.0" rkyv = { version = "0.7.40", features = ["indexmap", "validation", "strict"] } -shared-buffer = "0.1" +shared-buffer = { workspace = true } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] wasmer-vm = { path = "../vm", version = "=4.2.4" } diff --git a/lib/virtual-fs/Cargo.toml b/lib/virtual-fs/Cargo.toml index 477a01d4b60..9f20792963c 100644 --- a/lib/virtual-fs/Cargo.toml +++ b/lib/virtual-fs/Cargo.toml @@ -10,24 +10,25 @@ repository.workspace = true rust-version.workspace = true [dependencies] -libc = { version = "^0.2", default-features = false, optional = true } -thiserror = "1" -futures = { version = "0.3" } -tracing = { version = "0.1" } -typetag = { version = "0.1", optional = true } -webc = { version = "5.0", optional = true } -slab = { version = "0.4" } -derivative = "2.2.0" anyhow = { version = "1.0.66", optional = true } async-trait = { version = "^0.1" } -lazy_static = "1.4" -fs_extra = { version = "1.2.0", optional = true } -filetime = { version = "0.2.18", optional = true } bytes = "1" -tokio = { version = "1", features = ["io-util", "sync", "macros"], default_features = false } -pin-project-lite = "0.2.9" +derivative = "2.2.0" +filetime = { version = "0.2.18", optional = true } +fs_extra = { version = "1.2.0", optional = true } +futures = { version = "0.3" } indexmap = "1.9.2" +lazy_static = "1.4" +libc = { version = "^0.2", default-features = false, optional = true } +pin-project-lite = "0.2.9" replace_with = "0.1.7" +shared-buffer = { workspace = true } +slab = { version = "0.4" } +thiserror = "1" +tokio = { version = "1", features = ["io-util", "sync", "macros"], default_features = false } +tracing = { version = "0.1" } +typetag = { version = "0.1", optional = true } +webc = { version = "5.0", optional = true } [target.'cfg(not(all(target_arch = "wasm32", target_os = "unknown")))'.dependencies] getrandom = { version = "0.2" } From 9c2d2c6881e86498ca58d391867dc6e30034da25 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 15 Dec 2023 00:18:22 +0800 Subject: [PATCH 3/4] Gave users a way to access the underlying bytes in a `StaticFile` --- lib/virtual-fs/src/static_file.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/virtual-fs/src/static_file.rs b/lib/virtual-fs/src/static_file.rs index b87c50b12be..fed2b10a12b 100644 --- a/lib/virtual-fs/src/static_file.rs +++ b/lib/virtual-fs/src/static_file.rs @@ -19,6 +19,11 @@ impl StaticFile { pub fn new(bytes: impl Into) -> Self { StaticFile(Cursor::new(bytes.into())) } + + /// Access the underlying buffer. + pub fn contents(&self) -> &OwnedBuffer { + self.0.get_ref() + } } #[async_trait::async_trait] From 4a33c45a4e58868d2afee18db2830df0e74e6b14 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Fri, 15 Dec 2023 00:29:43 +0800 Subject: [PATCH 4/4] Updated a place where type inference now fails --- lib/wasix/src/state/env.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/wasix/src/state/env.rs b/lib/wasix/src/state/env.rs index 94aa03f4f15..44ab7213f60 100644 --- a/lib/wasix/src/state/env.rs +++ b/lib/wasix/src/state/env.rs @@ -937,7 +937,7 @@ impl WasiEnv { } WasiFsRoot::Backing(fs) => { let mut f = fs.new_open_options().create(true).write(true).open(path)?; - f.copy_reference(Box::new(StaticFile::new(atom.into()))); + f.copy_reference(Box::new(StaticFile::new(atom))); } }