Merge pull request #33 from jwodder/href

Add an `Href` type for representing `href`-ready links

src/dav/html.rs

@@ -1,3 +1,4 @@
+use super::util::Href;
 use super::{DavCollection, DavItem, DavResource, ResourceKind};
 use crate::consts::HTML_TIMESTAMP_FORMAT;
 use serde::{ser::Serializer, Serialize};
@@ -51,7 +52,7 @@ pub(super) struct CollectionContext {
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize)]
 pub(super) struct ColRow {
     name: String,
-    href: String,
+    href: Href,
     is_dir: bool,
     kind: ResourceKind,
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -69,7 +70,7 @@ pub(super) struct ColRow {
 }
 
 impl ColRow {
-    pub(super) fn parentdir(href: String) -> ColRow {
+    pub(super) fn parentdir(href: Href) -> ColRow {
         ColRow {
             name: "..".to_owned(),
             href,

src/dav/types.rs

@@ -1,4 +1,4 @@
-use super::util::{format_creationdate, format_modifieddate, urlencode, version_path};
+use super::util::{format_creationdate, format_modifieddate, version_path, Href};
 use super::xml::{PropValue, Property};
 use super::VersionSpec;
 use crate::consts::{DEFAULT_CONTENT_TYPE, YAML_CONTENT_TYPE};
@@ -10,7 +10,7 @@ use time::OffsetDateTime;
 
 #[enum_dispatch]
 pub(super) trait HasProperties {
-    fn href(&self) -> String;
+    fn href(&self) -> Href;
     fn creationdate(&self) -> Option<String>;
     fn displayname(&self) -> Option<String>;
     fn getcontentlength(&self) -> Option<i64>;
@@ -164,17 +164,17 @@ impl DavCollection {
         self.path.as_ref().map(PureDirPath::name)
     }
 
-    pub(super) fn web_link(&self) -> String {
+    pub(super) fn web_link(&self) -> Href {
         match self.path {
-            Some(ref p) => urlencode(&format!("/{p}")),
-            None => "/".to_owned(),
+            Some(ref p) => Href::from_path(&format!("/{p}")),
+            None => Href::from_path("/"),
         }
     }
 
-    pub(super) fn parent_web_link(&self) -> String {
+    pub(super) fn parent_web_link(&self) -> Href {
         match self.path.as_ref().and_then(PureDirPath::parent) {
-            Some(ref p) => urlencode(&format!("/{p}")),
-            None => "/".to_owned(),
+            Some(ref p) => Href::from_path(&format!("/{p}")),
+            None => Href::from_path("/"),
         }
     }
 
@@ -242,7 +242,7 @@ impl DavCollection {
 }
 
 impl HasProperties for DavCollection {
-    fn href(&self) -> String {
+    fn href(&self) -> Href {
         self.web_link()
     }
 
@@ -344,13 +344,13 @@ impl DavItem {
         self.path.name()
     }
 
-    pub(super) fn web_link(&self) -> String {
+    pub(super) fn web_link(&self) -> Href {
         if let DavContent::Redirect(ref url) = self.content {
             // Link directly to the download URL in the web view in order to
             // save a request
-            url.to_string()
+            url.into()
         } else {
-            urlencode(&format!("/{}", self.path))
+            Href::from_path(&format!("/{}", self.path))
         }
     }
 
@@ -366,9 +366,8 @@ impl DavItem {
 }
 
 impl HasProperties for DavItem {
-    fn href(&self) -> String {
-        // TODO: Should this match DavItem::web_link?
-        urlencode(&format!("/{}", self.path))
+    fn href(&self) -> Href {
+        Href::from_path(&format!("/{}", self.path))
     }
 
     fn creationdate(&self) -> Option<String> {

src/dav/util.rs

@@ -11,6 +11,7 @@ use axum::{
 };
 use indoc::indoc;
 use percent_encoding::{percent_encode, AsciiSet, NON_ALPHANUMERIC};
+use serde::{ser::Serializer, Serialize};
 use std::fmt::{self, Write};
 use time::{
     format_description::{well_known::Rfc3339, FormatItem},
@@ -55,10 +56,6 @@ pub(super) fn version_path(dandiset_id: &DandisetId, version: &VersionSpec) -> P
         .expect("should be a valid dir path")
 }
 
-pub(super) fn urlencode(s: &str) -> String {
-    percent_encode(s.as_ref(), PERCENT_ESCAPED).to_string()
-}
-
 pub(super) fn format_creationdate(dt: OffsetDateTime) -> String {
     dt.format(&Rfc3339)
         .expect("formatting an OffsetDateTime in RFC 3339 format should not fail")
@@ -95,16 +92,55 @@ impl<S: Send + Sync> FromRequestParts<S> for FiniteDepth {
     }
 }
 
+/// A percent-encoded URI or URI path, for use in the `href` attribute of an
+/// HTML `<a>` tag or in a `<DAV:href>` tag in a `PROPFIND` response
+#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+pub(super) struct Href(String);
+
+impl Href {
+    /// Construct an `Href` from a non-percent-encoded URI path
+    pub(super) fn from_path(path: &str) -> Href {
+        Href(percent_encode(path.as_ref(), PERCENT_ESCAPED).to_string())
+    }
+}
+
+impl AsRef<str> for Href {
+    fn as_ref(&self) -> &str {
+        self.0.as_ref()
+    }
+}
+
+impl From<url::Url> for Href {
+    fn from(value: url::Url) -> Href {
+        Href(value.into())
+    }
+}
+
+impl From<&url::Url> for Href {
+    fn from(value: &url::Url) -> Href {
+        Href(value.to_string())
+    }
+}
+
+impl Serialize for Href {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        serializer.serialize_str(self.as_ref())
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
     use time::macros::datetime;
 
     #[test]
-    fn test_urlencode() {
+    fn test_href_from_path() {
         let s = "/~cleesh/foo bar/baz_quux.gnusto/red&green?blue";
         assert_eq!(
-            urlencode(s),
+            Href::from_path(s).as_ref(),
             "/~cleesh/foo%20bar/baz_quux.gnusto/red%26green%3Fblue"
         );
     }

src/dav/xml/multistatus.rs

@@ -1,5 +1,6 @@
 use super::*;
 use crate::consts::DAV_XMLNS;
+use crate::dav::util::Href;
 use std::collections::BTreeMap;
 use thiserror::Error;
 use xml::writer::{events::XmlEvent, EmitterConfig, Error as WriteError, EventWriter};
@@ -27,24 +28,24 @@ impl Multistatus {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub(in crate::dav) struct DavResponse {
-    pub(in crate::dav) href: String,
+    pub(in crate::dav) href: Href,
     // TODO: RFC 4918 says <response> can contain (href*, status) as an
     // alternative to propstat.  When does that apply?
     pub(in crate::dav) propstat: Vec<PropStat>,
     //error
     //responsedescription
-    pub(in crate::dav) location: Option<String>,
+    pub(in crate::dav) location: Option<Href>,
 }
 
 impl DavResponse {
     fn write_xml(&self, writer: &mut XmlWriter) -> Result<(), WriteError> {
         writer.tag("response", |writer| {
-            writer.text_tag("href", &self.href)?;
+            writer.text_tag("href", self.href.as_ref())?;
             for p in &self.propstat {
                 p.write_xml(writer)?;
             }
             if let Some(ref loc) = self.location {
-                writer.tag("location", |writer| writer.text_tag("href", loc))?;
+                writer.tag("location", |writer| writer.text_tag("href", loc.as_ref()))?;
             }
             Ok(())
         })
@@ -161,7 +162,7 @@ mod tests {
         let value = Multistatus {
             response: vec![
                 DavResponse {
-                    href: "/foo/".into(),
+                    href: Href::from_path("/foo/"),
                     propstat: vec![PropStat {
                         prop: BTreeMap::from([
                             (Property::ResourceType, PropValue::Collection),
@@ -172,7 +173,7 @@ mod tests {
                     location: None,
                 },
                 DavResponse {
-                    href: "/foo/bar.txt".into(),
+                    href: Href::from_path("/foo/bar.txt"),
                     propstat: vec![PropStat {
                         prop: BTreeMap::from([
                             (
@@ -200,7 +201,7 @@ mod tests {
                     location: None,
                 },
                 DavResponse {
-                    href: "/foo/quux.dat".into(),
+                    href: Href::from_path("/foo/quux.dat"),
                     propstat: vec![PropStat {
                         prop: BTreeMap::from([
                             (Property::DisplayName, PropValue::String("quux.dat".into())),
@@ -221,7 +222,11 @@ mod tests {
                         ]),
                         status: "HTTP/1.1 307 TEMPORARY REDIRECT".into(),
                     }],
-                    location: Some("https://www.example.com/data/quux.dat".into()),
+                    location: Some(
+                        url::Url::parse("https://www.example.com/data/quux.dat")
+                            .unwrap()
+                            .into(),
+                    ),
                 },
             ],
         };