WebDAV validates connections and preserves directory listings, with CI checking every change (#1)

Co-authored-by: Danny Kim <temeddix@gmail.com>
Co-committed-by: Danny Kim <temeddix@gmail.com>

src/dav.rs

@@ -23,6 +23,9 @@ fn segment(value: &str) -> String {
 
 /// Joins a connection-relative path onto the configured root.
 pub fn url(root: &str, path: &str) -> String {
+  if path == "/" || path.is_empty() {
+    return root.to_owned();
+  }
   let mut url = root.trim_end_matches('/').to_owned();
   for part in path.split('/').filter(|part| !part.is_empty()) {
     url.push('/');
@@ -238,23 +241,62 @@ 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),
+/// An href can be an absolute URL or an absolute path (RFC 4918, section 8.3).
+fn href_path(href: &str) -> &str {
+  let path = match href.split_once("://") {
+    Some((_, authority_and_path)) => authority_and_path
+      .find('/')
+      .map(|start| &authority_and_path[start..])
+      .unwrap_or("/"),
+    None => href,
+  };
+  path
+    .split(['?', '#'])
+    .next()
+    .unwrap_or(path)
+    .trim_end_matches('/')
+}
+
+/// Responses may arrive in any order. Match the requested resource by its
+/// path, allowing collection slashes and equivalent percent encodings.
+pub fn resource<'a>(
+  found: &'a [Found],
+  url: &str,
+) -> Result<&'a Found, ErrorCode> {
+  found
+    .iter()
+    .find(|entry| {
+      href_path(&entry.href)
+        .split('/')
+        .map(name_of)
+        .eq(href_path(url).split('/').map(name_of))
+    })
+    .ok_or(ErrorCode::NotFound)
+}
+
+/// The direct children in a Depth 1 reply, excluding the requested directory.
+pub fn children<'a>(
+  found: &'a [Found],
+  url: &str,
+) -> Result<Vec<&'a Found>, ErrorCode> {
+  let parent = resource(found, url)?;
+  if !parent.collection {
+    return Err(ErrorCode::InvalidInput);
   }
+  Ok(
+    found
+      .iter()
+      .filter(|entry| !std::ptr::eq(*entry, parent))
+      .collect(),
+  )
 }
 
 /// 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(()),
+pub fn removable(found: &[Found], url: &str) -> Result<(), ErrorCode> {
+  match children(found, url) {
+    Ok(children) if children.is_empty() => Ok(()),
     Ok(_) => Err(ErrorCode::Conflict),
     Err(ErrorCode::InvalidInput) => Ok(()),
     Err(error) => Err(error),
@@ -265,24 +307,24 @@ pub fn removable(found: &[Found]) -> Result<(), ErrorCode> {
 mod tests {
   use super::{
     Body, Found, basic, children, multistatus, name_of, outcome, range_outcome,
-    removable, url,
+    removable, resource, url,
   };
   use crate::sdk::ErrorCode;
 
-  fn found(collection: bool) -> Found {
+  fn found(href: &str, collection: bool) -> Found {
     Found {
-      href: String::new(),
+      href: href.to_owned(),
       collection,
       length: 0,
       etag: None,
     }
   }
 
-  #[test]
-  fn paths_join_onto_the_root_without_growing_a_boundary() {
+  #[tokio::test]
+  async fn paths_join_onto_the_root_without_growing_a_boundary() {
     assert_eq!(
       url("https://dav.example/files/", "/"),
-      "https://dav.example/files"
+      "https://dav.example/files/"
     );
     assert_eq!(
       url("https://dav.example/files", "/notes/today.md"),
@@ -299,6 +341,46 @@ mod tests {
     );
   }
 
+  #[tokio::test]
+  async fn listings_keep_children_when_the_directory_is_not_first()
+  -> Result<(), ErrorCode> {
+    let entries = [found("/files/a.md", false), found("/files/", true)];
+    let listed = children(&entries, "https://dav.example/files/")?;
+    assert_eq!(listed.len(), 1);
+    assert_eq!(listed[0].href, "/files/a.md");
+    Ok(())
+  }
+
+  #[tokio::test]
+  async fn a_file_before_its_directory_never_allows_recursive_delete() {
+    let entries = [found("/files/a.md", false), found("/files/", true)];
+    assert_eq!(
+      removable(&entries, "https://dav.example/files/"),
+      Err(ErrorCode::Conflict)
+    );
+  }
+
+  #[tokio::test]
+  async fn hrefs_match_absolute_urls_and_equivalent_encoded_paths()
+  -> Result<(), ErrorCode> {
+    let entries = [
+      found("https://dav.example/my%20files/%e9%9b%aa.md", false),
+      found("https://dav.example/my%20files/", true),
+    ];
+    let listed = children(&entries, "https://dav.example/my%20files")?;
+    assert_eq!(listed.len(), 1);
+    assert_eq!(name_of(&listed[0].href), "雪.md");
+    assert!(
+      !resource(&entries, "https://dav.example/my%20files/%E9%9B%AA.md")?
+        .collection
+    );
+    assert_eq!(
+      removable(&entries, "https://dav.example/unrelated/"),
+      Err(ErrorCode::NotFound)
+    );
+    Ok(())
+  }
+
   #[test]
   fn credentials_encode_the_way_basic_auth_expects() {
     assert_eq!(basic("user", "pass"), "Basic dXNlcjpwYXNz");
@@ -366,21 +448,30 @@ mod tests {
     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(()));
+  #[tokio::test]
+  async fn only_an_empty_directory_or_a_file_may_be_removed() {
+    let url = "https://dav.example/files/";
+    assert_eq!(removable(&[found("/files/", false)], url), Ok(()));
+    assert_eq!(removable(&[found("/files/", true)], url), Ok(()));
     assert_eq!(
-      removable(&[found(true), found(false)]),
+      removable(&[found("/files/", true), found("/files/a.md", false)], url),
       Err(ErrorCode::Conflict)
     );
-    assert_eq!(removable(&[]), Err(ErrorCode::NotFound));
+    assert_eq!(removable(&[], url), Err(ErrorCode::NotFound));
     assert_eq!(
-      children(&[found(false)]).err(),
+      children(&[found("/files/", false)], url).err(),
       Some(ErrorCode::InvalidInput)
     );
     assert_eq!(
-      children(&[found(true), found(false), found(true)]).map(<[Found]>::len),
+      children(
+        &[
+          found("/files/", true),
+          found("/files/a.md", false),
+          found("/files/notes/", true)
+        ],
+        url,
+      )
+      .map(|children| children.len()),
       Ok(2)
     );
   }

src/lib.rs

@@ -23,9 +23,43 @@ const MAX_CHUNK: u32 = 1024 * 1024;
 
 struct Session {
   root: String,
+  /// The server's canonical href is only used to identify listing responses.
+  root_href: String,
   authorization: Option<String>,
 }
 
+impl Session {
+  fn from_config(config: &[(String, Setting)]) -> Result<Self, ErrorCode> {
+    let lookup = |wanted: &str| {
+      config
+        .iter()
+        .find(|(key, _)| key == wanted)
+        .and_then(|(_, value)| text(Some(value.clone())))
+    };
+    let root = lookup("url").ok_or(ErrorCode::InvalidInput)?;
+    if !root.starts_with("https://") && !root.starts_with("http://") {
+      return Err(ErrorCode::InvalidInput);
+    }
+    let authorization = lookup("username").map(|username| {
+      dav::basic(&username, &lookup("password").unwrap_or_default())
+    });
+    Ok(Self {
+      root_href: root.clone(),
+      root,
+      authorization,
+    })
+  }
+
+  fn record_root(&mut self, found: &[dav::Found]) -> Result<(), ErrorCode> {
+    let root = found.first().ok_or(ErrorCode::NotFound)?;
+    if !root.collection || root.href.is_empty() {
+      return Err(ErrorCode::InvalidInput);
+    }
+    self.root_href.clone_from(&root.href);
+    Ok(())
+  }
+}
+
 thread_local! {
   static SESSION: RefCell<Option<Session>> = const { RefCell::new(None) };
 }
@@ -34,10 +68,11 @@ struct Webdav;
 
 fn text(setting: Option<Setting>) -> Option<String> {
   match setting {
-    Some(Setting::Text(value)) | Some(Setting::Secret(value)) => {
+    Some(Setting::Text(value)) => {
       let value = value.trim().to_owned();
       (!value.is_empty()).then_some(value)
     }
+    Some(Setting::Secret(value)) => Some(value),
     Some(Setting::Toggle(_)) | None => None,
   }
 }
@@ -225,26 +260,20 @@ impl Guest for Webdav {
     _provider_id: String,
     config: Vec<(String, Setting)>,
   ) -> Result<Capabilities, ErrorCode> {
-    let lookup = |wanted: &str| {
-      config
-        .iter()
-        .find(|(key, _)| key == wanted)
-        .and_then(|(_, value)| text(Some(value.clone())))
-    };
-    let root = lookup("url").ok_or(ErrorCode::InvalidInput)?;
-    if !root.starts_with("https://") && !root.starts_with("http://") {
-      return Err(ErrorCode::InvalidInput);
+    SESSION.set(None);
+    SESSION.set(Some(Session::from_config(&config)?));
+    let probe = describe("/", "0").and_then(|found| {
+      SESSION.with_borrow_mut(|session| {
+        session
+          .as_mut()
+          .ok_or(ErrorCode::Internal)?
+          .record_root(&found)
+      })
+    });
+    if let Err(error) = probe {
+      SESSION.set(None);
+      return Err(error);
     }
-    let authorization = match (lookup("username"), lookup("password")) {
-      (Some(username), Some(password)) => {
-        Some(dav::basic(&username, &password))
-      }
-      _ => None,
-    };
-    SESSION.set(Some(Session {
-      root,
-      authorization,
-    }));
     Ok(Capabilities {
       write: true,
       mkdir: true,
@@ -281,7 +310,8 @@ impl Guest for Webdav {
       None => 0,
     };
     let found = describe(&path, "1")?;
-    let children: Vec<_> = dav::children(&found)?
+    let url = session(|session| Ok(dav::url(&session.root_href, &path)))?;
+    let children: Vec<_> = dav::children(&found, &url)?
       .iter()
       .map(|entry| Entry {
         name: dav::name_of(&entry.href),
@@ -400,7 +430,8 @@ impl Guest for Webdav {
   /// first and refused unless it is empty.
   fn remove(target: Locator) -> Result<(), ErrorCode> {
     let path = route(&target);
-    dav::removable(&describe(&path, "1")?)?;
+    let url = session(|session| Ok(dav::url(&session.root_href, &path)))?;
+    dav::removable(&describe(&path, "1")?, &url)?;
     exchange(host::Method::Delete, &path, Vec::new(), &[])?;
     Ok(())
   }
@@ -410,4 +441,155 @@ impl Guest for Webdav {
   }
 }
 
+#[cfg(not(test))]
 sdk::export!(Webdav with_types_in sdk);
+
+#[cfg(test)]
+mod tests {
+  use super::{SESSION, Session, Webdav};
+  use crate::dav;
+  use crate::sdk::{ErrorCode, Guest, Setting};
+
+  #[tokio::test]
+  async fn passwords_keep_leading_and_trailing_whitespace()
+  -> Result<(), ErrorCode> {
+    let config = vec![
+      (
+        "url".to_owned(),
+        Setting::Text("https://dav.example/".to_owned()),
+      ),
+      ("username".to_owned(), Setting::Text("user".to_owned())),
+      (
+        "password".to_owned(),
+        Setting::Secret(" pass word \t".to_owned()),
+      ),
+    ];
+    assert_eq!(
+      Session::from_config(&config)?.authorization.as_deref(),
+      Some("Basic dXNlcjogcGFzcyB3b3JkIAk=")
+    );
+    Ok(())
+  }
+
+  #[tokio::test]
+  async fn a_username_without_a_password_setting_sends_basic_auth()
+  -> Result<(), ErrorCode> {
+    let config = vec![
+      (
+        "url".to_owned(),
+        Setting::Text("https://dav.example/".to_owned()),
+      ),
+      ("username".to_owned(), Setting::Text("user".to_owned())),
+    ];
+    assert_eq!(
+      Session::from_config(&config)?.authorization.as_deref(),
+      Some("Basic dXNlcjo=")
+    );
+    Ok(())
+  }
+
+  #[tokio::test]
+  async fn no_username_keeps_the_connection_anonymous() -> Result<(), ErrorCode>
+  {
+    let config = vec![
+      (
+        "url".to_owned(),
+        Setting::Text("https://dav.example/".to_owned()),
+      ),
+      ("password".to_owned(), Setting::Secret("unused".to_owned())),
+    ];
+    assert!(Session::from_config(&config)?.authorization.is_none());
+    Ok(())
+  }
+
+  #[tokio::test]
+  async fn a_redirected_root_lists_its_canonical_directory()
+  -> Result<(), ErrorCode> {
+    let config = vec![(
+      "url".to_owned(),
+      Setting::Text("https://dav.example/webdav".to_owned()),
+    )];
+    let mut session = Session::from_config(&config)?;
+    let entries = [
+      dav::Found {
+        href: "/files/notes.md".to_owned(),
+        collection: false,
+        length: 12,
+        etag: None,
+      },
+      dav::Found {
+        href: "/files/".to_owned(),
+        collection: true,
+        length: 0,
+        etag: None,
+      },
+    ];
+    session.record_root(&entries[1..])?;
+    let expected_url = dav::url(&session.root_href, "/");
+    let listed = dav::children(&entries, &expected_url)?;
+    assert_eq!(listed.len(), 1);
+    assert_eq!(listed[0].href, "/files/notes.md");
+
+    let nested = [
+      dav::Found {
+        href: "/files/documents/child.md".to_owned(),
+        collection: false,
+        length: 7,
+        etag: None,
+      },
+      dav::Found {
+        href: "/files/documents/".to_owned(),
+        collection: true,
+        length: 0,
+        etag: None,
+      },
+    ];
+    let expected_url = dav::url(&session.root_href, "/documents");
+    let listed = dav::children(&nested, &expected_url)?;
+    assert_eq!(listed.len(), 1);
+    assert_eq!(listed[0].href, "/files/documents/child.md");
+    assert_eq!(
+      dav::removable(&nested, &expected_url),
+      Err(ErrorCode::Conflict)
+    );
+    Ok(())
+  }
+
+  #[tokio::test]
+  async fn a_canonical_href_does_not_change_the_authenticated_request_origin()
+  -> Result<(), ErrorCode> {
+    let config = vec![
+      (
+        "url".to_owned(),
+        Setting::Text("https://dav.example/webdav".to_owned()),
+      ),
+      ("username".to_owned(), Setting::Text("user".to_owned())),
+    ];
+    let mut session = Session::from_config(&config)?;
+    session.record_root(&[dav::Found {
+      href: "https://other.example/files/".to_owned(),
+      collection: true,
+      length: 0,
+      etag: None,
+    }])?;
+    assert_eq!(dav::url(&session.root, "/"), "https://dav.example/webdav");
+    assert_eq!(
+      dav::url(&session.root, "/documents/child.md"),
+      "https://dav.example/webdav/documents/child.md"
+    );
+    assert_eq!(session.authorization.as_deref(), Some("Basic dXNlcjo="));
+    Ok(())
+  }
+
+  #[tokio::test]
+  async fn a_failed_reopen_discards_the_previous_connection() {
+    SESSION.set(Some(Session {
+      root: "https://dav.example/files/".to_owned(),
+      root_href: "/files/".to_owned(),
+      authorization: None,
+    }));
+    let result = Webdav::open("webdav".to_owned(), Vec::new());
+    assert_eq!(result.err(), Some(ErrorCode::InvalidInput));
+    assert!(SESSION.with_borrow(Option::is_none));
+  }
+}