Bound reads to their limit, treat 416 as end of file, and refuse to remove a non-empty directory

Cargo.lock

@@ -96,7 +96,7 @@ checksum = "cf8baf1c55e62ffcace7a9f06f4bd9cd3f0c4beb022d3b367256b91b87513d98"
 
 [[package]]
 name = "memona-plugin-webdav"
-version = "0.1.0"
+version = "0.1.1"
 dependencies = [
  "quick-xml",
  "wit-bindgen",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "memona-plugin-webdav"
-version = "0.1.0"
+version = "0.1.1"
 edition = "2024"
 license = "MIT"
 description = "A WebDAV filesystem provider for Memona, built as a WASI component."

README.md

@@ -19,7 +19,9 @@ Listing, reading, writing, creating directories, renaming and deleting are
 supported. Reads and writes stream in pieces of at most 1 MiB, so a file of any
 size moves without either side holding it whole. Conditional writes use
 `If-Match` and `If-None-Match`, which servers that return etags honour. Change
-polling is not supported, and a rename never replaces an existing target.
+polling is not supported, a rename never replaces an existing target, and
+deleting takes one file or an empty directory: a folder with contents is
+refused rather than removed whole.
 
 ## Building
 

manifest.json

@@ -2,7 +2,7 @@
   "schemaVersion": 1,
   "id": "memona/webdav",
   "name": "WebDAV",
-  "version": "0.1.0",
+  "version": "0.1.1",
   "apiVersion": "0.1.0",
   "description": "Open a WebDAV server as a folder, and let agents work in it.",
   "license": "MIT",

src/dav.rs

@@ -72,6 +72,58 @@ pub fn outcome(status: u16) -> Result<(), ErrorCode> {
   }
 }
 
+/// What a ranged GET's status means to a reader: whether a body follows. A
+/// range that starts at the end of the file draws 416 from Apache and most
+/// other servers, which for a reader is simply end of file, not a failure.
+pub fn range_outcome(status: u16) -> Result<bool, ErrorCode> {
+  match status {
+    416 => Ok(false),
+    _ => outcome(status).map(|()| true),
+  }
+}
+
+/// A response body served in pieces no larger than the caller asked for. The
+/// host may hand over more than one pull wanted, so the surplus waits here for
+/// the next pull rather than being dropped or passed through oversize.
+#[derive(Default)]
+pub struct Body {
+  spare: Vec<u8>,
+  ended: bool,
+}
+
+impl Body {
+  /// A body that was never sent: every pull reports end of file.
+  pub fn empty() -> Self {
+    Self {
+      spare: Vec::new(),
+      ended: true,
+    }
+  }
+
+  /// Returns at most `limit` bytes, drawing on `next` until the limit is met
+  /// or the body ends. `next` is asked for the shortfall and may answer with
+  /// more or less than that.
+  pub fn pull(
+    &mut self,
+    limit: usize,
+    mut next: impl FnMut(u32) -> Result<Vec<u8>, ErrorCode>,
+  ) -> Result<Vec<u8>, ErrorCode> {
+    let mut data = std::mem::take(&mut self.spare);
+    while data.len() < limit && !self.ended {
+      let chunk = next((limit - data.len()) as u32)?;
+      if chunk.is_empty() {
+        self.ended = true;
+      } else {
+        data.extend_from_slice(&chunk);
+      }
+    }
+    if data.len() > limit {
+      self.spare = data.split_off(limit);
+    }
+    Ok(data)
+  }
+}
+
 /// One entry from a multistatus reply.
 pub struct Found {
   pub href: String,
@@ -186,11 +238,46 @@ pub fn name_of(href: &str) -> String {
   String::from_utf8_lossy(&decoded).into_owned()
 }
 
+/// The direct children in a Depth 1 reply. The first response describes the
+/// resource asked about, so it must be a collection before the rest can be
+/// called its children.
+pub fn children(found: &[Found]) -> Result<&[Found], ErrorCode> {
+  match found.split_first() {
+    None => Err(ErrorCode::NotFound),
+    Some((first, rest)) if first.collection => Ok(rest),
+    Some(_) => Err(ErrorCode::InvalidInput),
+  }
+}
+
+/// Whether a Depth 1 reply describes something `remove` may delete: a file,
+/// or a collection with no children. DELETE on a collection is recursive in
+/// WebDAV, so this check is the only thing keeping a whole tree.
+pub fn removable(found: &[Found]) -> Result<(), ErrorCode> {
+  match children(found) {
+    Ok([]) => Ok(()),
+    Ok(_) => Err(ErrorCode::Conflict),
+    Err(ErrorCode::InvalidInput) => Ok(()),
+    Err(error) => Err(error),
+  }
+}
+
 #[cfg(test)]
 mod tests {
-  use super::{basic, multistatus, name_of, outcome, url};
+  use super::{
+    Body, Found, basic, children, multistatus, name_of, outcome, range_outcome,
+    removable, url,
+  };
   use crate::sdk::ErrorCode;
 
+  fn found(collection: bool) -> Found {
+    Found {
+      href: String::new(),
+      collection,
+      length: 0,
+      etag: None,
+    }
+  }
+
   #[test]
   fn paths_join_onto_the_root_without_growing_a_boundary() {
     assert_eq!(
@@ -230,6 +317,74 @@ mod tests {
     assert!(matches!(outcome(500), Err(ErrorCode::Unavailable)));
   }
 
+  #[test]
+  fn a_range_past_the_end_is_end_of_file_not_a_failure() {
+    assert_eq!(range_outcome(206), Ok(true));
+    assert_eq!(range_outcome(200), Ok(true));
+    assert_eq!(range_outcome(416), Ok(false));
+    assert_eq!(range_outcome(404), Err(ErrorCode::NotFound));
+  }
+
+  #[test]
+  fn a_body_never_hands_over_more_than_the_limit() -> Result<(), ErrorCode> {
+    // The host answers every pull with a whole 10-byte piece, whatever the
+    // shortfall asked for.
+    let mut asked = Vec::new();
+    let mut source = |wanted: u32| {
+      asked.push(wanted);
+      Ok(if asked.len() > 2 {
+        Vec::new()
+      } else {
+        vec![asked.len() as u8; 10]
+      })
+    };
+    let mut body = Body::default();
+    assert_eq!(body.pull(4, &mut source)?, vec![1, 1, 1, 1]);
+    // The surplus of the first piece is served before anything new is read.
+    assert_eq!(body.pull(8, &mut source)?, vec![1, 1, 1, 1, 1, 1, 2, 2]);
+    assert_eq!(body.pull(100, &mut source)?, vec![2; 8]);
+    assert_eq!(body.pull(100, &mut source)?, Vec::<u8>::new());
+    // Only the shortfall is asked for, and nothing after the body ended.
+    assert_eq!(asked, vec![4, 2, 92]);
+    Ok(())
+  }
+
+  #[test]
+  fn a_body_keeps_reading_until_the_limit_is_met() -> Result<(), ErrorCode> {
+    // A host that answers in pieces smaller than the limit is drained until
+    // the limit is met, so a short piece is never mistaken for end of file.
+    let mut body = Body::default();
+    let mut left = 7;
+    let mut source = |_wanted: u32| {
+      let piece = left.min(3);
+      left -= piece;
+      Ok(vec![9; piece])
+    };
+    assert_eq!(body.pull(5, &mut source)?, vec![9; 5]);
+    assert_eq!(body.pull(5, &mut source)?, vec![9; 2]);
+    assert_eq!(Body::empty().pull(5, &mut source)?, Vec::<u8>::new());
+    Ok(())
+  }
+
+  #[test]
+  fn only_an_empty_directory_or_a_file_may_be_removed() {
+    assert_eq!(removable(&[found(false)]), Ok(()));
+    assert_eq!(removable(&[found(true)]), Ok(()));
+    assert_eq!(
+      removable(&[found(true), found(false)]),
+      Err(ErrorCode::Conflict)
+    );
+    assert_eq!(removable(&[]), Err(ErrorCode::NotFound));
+    assert_eq!(
+      children(&[found(false)]).err(),
+      Some(ErrorCode::InvalidInput)
+    );
+    assert_eq!(
+      children(&[found(true), found(false), found(true)]).map(<[Found]>::len),
+      Ok(2)
+    );
+  }
+
   #[test]
   fn a_multistatus_reply_yields_the_collection_then_its_children()
   -> Result<(), ErrorCode> {

src/lib.rs

@@ -9,7 +9,7 @@ use sdk::{
   GuestReader, GuestWriter, ListPage, Locator, Reader, Setting, WriteCondition,
   Writer,
 };
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
 
 /// The PROPFIND body. Asking for named properties rather than allprop keeps
 /// replies small on servers that store a great many of them.
@@ -131,6 +131,20 @@ fn content_type() -> host::Header {
   }
 }
 
+/// Asks the server about one path, and its children when `depth` is 1.
+fn describe(
+  path: &str,
+  depth_value: &str,
+) -> Result<Vec<dav::Found>, ErrorCode> {
+  let body = exchange(
+    host::Method::Propfind,
+    path,
+    vec![depth(depth_value), content_type()],
+    PROPS.as_bytes(),
+  )?;
+  dav::multistatus(&body)
+}
+
 fn info_of(found: &dav::Found) -> FileInfo {
   FileInfo {
     kind: if found.collection {
@@ -147,11 +161,17 @@ fn info_of(found: &dav::Found) -> FileInfo {
 struct DavReader {
   call: host::Exchange,
   revision: Option<String>,
+  body: RefCell<dav::Body>,
 }
 
 impl GuestReader for DavReader {
   fn pull(&self, limit: u32) -> Result<Vec<u8>, ErrorCode> {
-    self.call.pull(limit.min(MAX_CHUNK)).map_err(transport)
+    self
+      .body
+      .borrow_mut()
+      .pull(limit.min(MAX_CHUNK) as usize, |wanted| {
+        self.call.pull(wanted).map_err(transport)
+      })
   }
   fn revision(&self) -> Option<String> {
     self.revision.clone()
@@ -162,6 +182,7 @@ impl GuestReader for DavReader {
 struct DavWriter {
   call: host::Exchange,
   done: RefCell<bool>,
+  written: Cell<u64>,
 }
 
 impl GuestWriter for DavWriter {
@@ -169,7 +190,9 @@ impl GuestWriter for DavWriter {
     if *self.done.borrow() {
       return Err(ErrorCode::InvalidInput);
     }
-    self.call.push(&data).map_err(transport)
+    self.call.push(&data).map_err(transport)?;
+    self.written.set(self.written.get() + data.len() as u64);
+    Ok(())
   }
   /// One call is enough: the request body is already complete, so the response
   /// head settles the write. The etag it returns, when the server sends one,
@@ -188,7 +211,7 @@ impl GuestWriter for DavWriter {
       .map(|header| header.value.clone());
     Ok(Some(FileInfo {
       kind: EntryKind::File,
-      size: 0,
+      size: self.written.get(),
       revision,
     }))
   }
@@ -240,14 +263,7 @@ impl Guest for Webdav {
   }
 
   fn stat(target: Locator) -> Result<FileInfo, ErrorCode> {
-    let path = route(&target);
-    let body = exchange(
-      host::Method::Propfind,
-      &path,
-      vec![depth("0"), content_type()],
-      PROPS.as_bytes(),
-    )?;
-    let found = dav::multistatus(&body)?;
+    let found = describe(&route(&target), "0")?;
     let entry = found.first().ok_or(ErrorCode::NotFound)?;
     Ok(info_of(entry))
   }
@@ -264,17 +280,9 @@ impl Guest for Webdav {
       Some(value) => value.parse().map_err(|_| ErrorCode::InvalidInput)?,
       None => 0,
     };
-    let body = exchange(
-      host::Method::Propfind,
-      &path,
-      vec![depth("1"), content_type()],
-      PROPS.as_bytes(),
-    )?;
-    let found = dav::multistatus(&body)?;
-    // The first response is the collection itself, which is not its own child.
-    let children: Vec<_> = found
+    let found = describe(&path, "1")?;
+    let children: Vec<_> = dav::children(&found)?
       .iter()
-      .skip(1)
       .map(|entry| Entry {
         name: dav::name_of(&entry.href),
         info: info_of(entry),
@@ -288,8 +296,15 @@ impl Guest for Webdav {
     })
   }
 
+  /// A GET on a collection answers with whatever page the server likes to
+  /// show, so the path is checked to be a file before any byte is read.
   fn open_read(target: Locator, offset: u64) -> Result<Reader, ErrorCode> {
     let path = route(&target);
+    let found = describe(&path, "0")?;
+    let entry = found.first().ok_or(ErrorCode::NotFound)?;
+    if entry.collection {
+      return Err(ErrorCode::InvalidInput);
+    }
     let call = begin(
       host::Method::Get,
       &path,
@@ -299,13 +314,25 @@ impl Guest for Webdav {
       }],
     )?;
     let head = call.respond().map_err(transport)?;
-    dav::outcome(head.status)?;
-    let revision = head
-      .headers
-      .iter()
-      .find(|header| header.name.eq_ignore_ascii_case("etag"))
-      .map(|header| header.value.clone());
-    Ok(Reader::new(DavReader { call, revision }))
+    let body = if dav::range_outcome(head.status)? {
+      dav::Body::default()
+    } else {
+      dav::Body::empty()
+    };
+    // The listing's etag is the one stat and list report, and the strong form
+    // a later conditional write must send; the GET's may be a weak variant.
+    let revision = entry.etag.clone().or_else(|| {
+      head
+        .headers
+        .iter()
+        .find(|header| header.name.eq_ignore_ascii_case("etag"))
+        .map(|header| header.value.clone())
+    });
+    Ok(Reader::new(DavReader {
+      call,
+      revision,
+      body: RefCell::new(body),
+    }))
   }
 
   fn open_write(
@@ -329,6 +356,7 @@ impl Guest for Webdav {
     Ok(Writer::new(DavWriter {
       call,
       done: RefCell::new(false),
+      written: Cell::new(0),
     }))
   }
 
@@ -368,8 +396,11 @@ impl Guest for Webdav {
     Self::stat(Locator::Path(destination))
   }
 
+  /// DELETE takes a whole collection at once, so a directory is looked at
+  /// first and refused unless it is empty.
   fn remove(target: Locator) -> Result<(), ErrorCode> {
     let path = route(&target);
+    dav::removable(&describe(&path, "1")?)?;
     exchange(host::Method::Delete, &path, Vec::new(), &[])?;
     Ok(())
   }