Merge pull request #8 from panicbit/reduce_result_returns

Reduce the number of fallible constructors

CHANGELOG.md

@@ -0,0 +1,23 @@
+# Changelog
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+### Added
+- `GEMINI_MIME_STR`, the `&str` representation of the Gemini MIME
+- `Meta::new_lossy` constructor that never fails
+- `Meta::MAX_LEN`, which is `1024`
+- "lossy" constructors for `Response` and `Status` (see `Meta::new_lossy`)
+
+### Changed
+- `Meta::new` rejects strings exceeding `Meta::MAX_LEN` (`1024`)
+- Some `Response` and `Status` constructors are now infallible
+
+### Deprecated
+- Instead of `gemini_mime()` use `GEMINI_MIME`
+
+## [0.2.0] - 2020-11-14
+### Added
+- Access to client certificates by [@Alch-Emi](https://github.com/Alch-Emi)

examples/certificates.rs

@@ -32,7 +32,7 @@ fn handle_request(users: Arc<RwLock<HashMap<CertBytes, String>>>, request: Reque
             if let Some(user) = users_read.get(cert_bytes) {
                 // The user has already registered
                 Ok(
-                    Response::success(&GEMINI_MIME)?
+                    Response::success(&GEMINI_MIME)
                         .with_body(format!("Welcome {}!", user))
                 )
             } else {
@@ -44,7 +44,7 @@ fn handle_request(users: Arc<RwLock<HashMap<CertBytes, String>>>, request: Reque
                     let mut users_write = users.write().await;
                     users_write.insert(cert_bytes.clone(), username.to_owned());
                     Ok(
-                        Response::success(&GEMINI_MIME)?
+                        Response::success(&GEMINI_MIME)
                             .with_body(format!(
                                 "Your account has been created {}!  Welcome!",
                                 username
@@ -57,7 +57,7 @@ fn handle_request(users: Arc<RwLock<HashMap<CertBytes, String>>>, request: Reque
             }
         } else {
             // The user didn't provide a certificate
-            Response::client_certificate_required()
+            Ok(Response::client_certificate_required())
         }
     }.boxed()
 }

src/types.rs

@@ -88,11 +88,18 @@ impl ResponseHeader {
         })
     }
 
-    pub fn success(mime: &Mime) -> Result<Self> {
+    pub fn input_lossy(prompt: impl AsRef<str> + Into<String>) -> Self {
-        Ok(Self {
+        Self {
+            status: Status::INPUT,
+            meta: Meta::new_lossy(prompt),
+        }
+    }
+
+    pub fn success(mime: &Mime) -> Self {
+        Self {
             status: Status::SUCCESS,
-            meta: Meta::new(mime.to_string())?,
+            meta: Meta::new_lossy(mime.to_string()),
-        })
+        }
     }
 
     pub fn server_error(reason: impl AsRef<str> + Into<String>) -> Result<Self> {
@@ -102,25 +109,32 @@ impl ResponseHeader {
         })
     }
 
-    pub fn not_found() -> Result<Self> {
+    pub fn server_error_lossy(reason: impl AsRef<str> + Into<String>) -> Self {
-        Ok(Self {
+        Self {
+            status: Status::PERMANENT_FAILURE,
+            meta: Meta::new_lossy(reason),
+        }
+    }
+
+    pub fn not_found() -> Self {
+        Self {
             status: Status::NOT_FOUND,
-            meta: Meta::new("Not found")?,
+            meta: Meta::new_lossy("Not found"),
-        })
+        }
     }
 
-    pub fn client_certificate_required() -> Result<Self> {
+    pub fn client_certificate_required() -> Self {
-        Ok(Self {
+        Self {
             status: Status::CLIENT_CERTIFICATE_REQUIRED,
-            meta: Meta::new("No certificate provided")?,
+            meta: Meta::new_lossy("No certificate provided"),
-        })
+        }
     }
 
-    pub fn certificate_not_authorized() -> Result<Self> {
+    pub fn certificate_not_authorized() -> Self {
-        Ok(Self {
+        Self {
             status: Status::CERTIFICATE_NOT_AUTHORIZED,
-            meta: Meta::new("Your certificate is not authorized to view this content")?,
+            meta: Meta::new_lossy("Your certificate is not authorized to view this content"),
-        })
+        }
     }
 
     pub fn status(&self) -> &Status {
@@ -218,12 +232,34 @@ impl StatusCategory {
 pub struct Meta(String);
 
 impl Meta {
+    pub const MAX_LEN: usize = 1024;
+
+    /// Creates a new "Meta" string. Fails if `meta` contains `\n`.
     pub fn new(meta: impl AsRef<str> + Into<String>) -> Result<Self> {
         ensure!(!meta.as_ref().contains("\n"), "Meta must not contain newlines");
+        ensure!(meta.as_ref().len() <= Self::MAX_LEN, "Meta must not exceed {} bytes", Self::MAX_LEN);
 
         Ok(Self(meta.into()))
     }
 
+    /// Cretaes a new "Meta" string. Truncates `meta` to before the first occurrence of `\n`.
+    pub fn new_lossy(meta: impl AsRef<str> + Into<String>) -> Self {
+        let meta = meta.as_ref();
+        let truncate_pos = meta.char_indices().position(|(i, ch)| {
+            let is_newline = ch == '\n';
+            let exceeds_limit = (i + ch.len_utf8()) > Self::MAX_LEN;
+
+            is_newline || exceeds_limit
+        });
+
+        let meta: String = match truncate_pos {
+            None => meta.into(),
+            Some(truncate_pos) => meta.get(..truncate_pos).expect("northstar BUG").into(),
+        };
+
+        Self(meta)
+    }
+
     pub fn empty() -> Self {
         Self::default()
     }
@@ -256,9 +292,14 @@ impl Response {
         Ok(Self::new(header))
     }
 
-    pub fn success(mime: &Mime) -> Result<Self> {
+    pub fn input_lossy(prompt: impl AsRef<str> + Into<String>) -> Self {
-        let header = ResponseHeader::success(&mime)?;
+        let header = ResponseHeader::input_lossy(prompt);
-        Ok(Self::new(header))
+        Self::new(header)
+    }
+
+    pub fn success(mime: &Mime) -> Self {
+        let header = ResponseHeader::success(&mime);
+        Self::new(header)
     }
 
     pub fn server_error(reason: impl AsRef<str> + Into<String>) -> Result<Self>  {
@@ -266,19 +307,19 @@ impl Response {
         Ok(Self::new(header))
     }
 
-    pub fn not_found() -> Result<Self> {
+    pub fn not_found() -> Self {
-        let header = ResponseHeader::not_found()?;
+        let header = ResponseHeader::not_found();
-        Ok(Self::new(header))
+        Self::new(header)
     }
 
-    pub fn client_certificate_required() -> Result<Self> {
+    pub fn client_certificate_required() -> Self {
-        let header = ResponseHeader::client_certificate_required()?;
+        let header = ResponseHeader::client_certificate_required();
-        Ok(Self::new(header))
+        Self::new(header)
     }
 
-    pub fn certificate_not_authorized() -> Result<Self> {
+    pub fn certificate_not_authorized() -> Self {
-        let header = ResponseHeader::certificate_not_authorized()?;
+        let header = ResponseHeader::certificate_not_authorized();
-        Ok(Self::new(header))
+        Self::new(header)
     }
 
     pub fn with_body(mut self, body: impl Into<Body>) -> Self {
@@ -329,3 +370,85 @@ impl From<File> for Body {
         Self::Reader(Box::new(file))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::iter::repeat;
+
+    #[test]
+    fn meta_new_rejects_newlines() {
+        let meta = "foo\nbar";
+        let meta = Meta::new(meta);
+
+        assert!(meta.is_err());
+    }
+
+    #[test]
+    fn meta_new_accepts_max_len() {
+        let meta: String = repeat('x').take(Meta::MAX_LEN).collect();
+        let meta = Meta::new(meta);
+
+        assert!(meta.is_ok());
+    }
+
+    #[test]
+    fn meta_new_rejects_exceeding_max_len() {
+        let meta: String = repeat('x').take(Meta::MAX_LEN + 1).collect();
+        let meta = Meta::new(meta);
+
+        assert!(meta.is_err());
+    }
+
+    #[test]
+    fn meta_new_lossy_truncates() {
+        let meta = "foo\r\nbar\nquux";
+        let meta = Meta::new_lossy(meta);
+
+        assert_eq!(meta.as_str(), "foo\r");
+    }
+
+    #[test]
+    fn meta_new_lossy_no_truncate() {
+        let meta = "foo bar\r";
+        let meta = Meta::new_lossy(meta);
+
+        assert_eq!(meta.as_str(), "foo bar\r");
+    }
+
+    #[test]
+    fn meta_new_lossy_empty() {
+        let meta = "";
+        let meta = Meta::new_lossy(meta);
+
+        assert_eq!(meta.as_str(), "");
+    }
+
+    #[test]
+    fn meta_new_lossy_truncates_to_empty() {
+        let meta = "\n\n\n";
+        let meta = Meta::new_lossy(meta);
+
+        assert_eq!(meta.as_str(), "");
+    }
+
+    #[test]
+    fn meta_new_lossy_truncates_to_max_len() {
+        let meta: String = repeat('x').take(Meta::MAX_LEN + 1).collect();
+        let meta = Meta::new_lossy(meta);
+
+        assert_eq!(meta.as_str().len(), Meta::MAX_LEN);
+    }
+
+    #[test]
+    fn meta_new_lossy_truncates_multi_byte_sequences() {
+        let mut meta: String = repeat('x').take(Meta::MAX_LEN - 1).collect();
+        meta.push('🦀');
+
+        assert_eq!(meta.len(), Meta::MAX_LEN + 3);
+
+        let meta = Meta::new_lossy(meta);
+
+        assert_eq!(meta.as_str().len(), Meta::MAX_LEN - 1);
+    }
+}

src/util.rs

@@ -15,12 +15,12 @@ pub async fn serve_file<P: AsRef<Path>>(path: P, mime: &Mime) -> Result<Response
     let file = match File::open(path).await {
         Ok(file) => file,
         Err(err) => match err.kind() {
-            io::ErrorKind::NotFound => return Ok(Response::not_found()?),
+            io::ErrorKind::NotFound => return Ok(Response::not_found()),
             _ => return Err(err.into()),
         }
     };
 
-    Ok(Response::success(&mime)?.with_body(file))
+    Ok(Response::success(&mime).with_body(file))
 }
 
 pub async fn serve_dir<D: AsRef<Path>, P: AsRef<Path>>(dir: D, virtual_path: &[P]) -> Result<Response> {
@@ -35,7 +35,7 @@ pub async fn serve_dir<D: AsRef<Path>, P: AsRef<Path>>(dir: D, virtual_path: &[P
     let path = path.canonicalize()?;
 
     if !path.starts_with(&dir) {
-        return Ok(Response::not_found()?);
+        return Ok(Response::not_found());
     }
 
     if !path.is_dir() {
@@ -52,7 +52,7 @@ async fn serve_dir_listing<P: AsRef<Path>, B: AsRef<Path>>(path: P, virtual_path
     let mut dir = match fs::read_dir(path).await {
         Ok(dir) => dir,
         Err(err) => match err.kind() {
-            io::ErrorKind::NotFound => return Ok(Response::not_found()?),
+            io::ErrorKind::NotFound => return Ok(Response::not_found()),
             _ => return Err(err.into()),
         }
     };
@@ -82,7 +82,7 @@ async fn serve_dir_listing<P: AsRef<Path>, B: AsRef<Path>>(path: P, virtual_path
         )?;
     }
 
-    Ok(Response::success(&GEMINI_MIME)?.with_body(listing))
+    Ok(Response::success(&GEMINI_MIME).with_body(listing))
 }
 
 pub fn guess_mime_from_path<P: AsRef<Path>>(path: P) -> Mime {