apply review

expiry -> expiration
hard fail on invalid expiration values

Author: HeapUnderflow

README.md

@@ -111,7 +111,7 @@ run-time behavior:
   until wastebin responds with 408, by default it is set to 5 seconds.
 * `WASTEBIN_MAX_BODY_SIZE` number of bytes to accept for POST requests. Defaults
   to 1 MB.
-* `WASTEBIN_MAX_PASTE_EXPIRY` maximum allowed lifetime of a paste. Leave empty or set to -1 for no limit. Defaults to no limit.
+* `WASTEBIN_MAX_PASTE_EXPIRATION` maximum allowed lifetime of a paste. Leave empty or set to -1 for no limit. Defaults to no limit.
 * `WASTEBIN_PASSWORD_SALT` salt used to hash user passwords used for encrypting
   pastes.
 * `WASTEBIN_SIGNING_KEY` sets the key to sign cookies. If not set, a random key

src/env.rs

@@ -27,7 +27,7 @@ const VAR_SIGNING_KEY: &str = "WASTEBIN_SIGNING_KEY";
 const VAR_BASE_URL: &str = "WASTEBIN_BASE_URL";
 const VAR_PASSWORD_SALT: &str = "WASTEBIN_PASSWORD_SALT";
 const VAR_HTTP_TIMEOUT: &str = "WASTEBIN_HTTP_TIMEOUT";
-const VAR_MAX_PASTE_EXPIRY: &str = "WASTEBIN_MAX_PASTE_EXPIRY";
+const VAR_MAX_PASTE_EXPIRATION: &str = "WASTEBIN_MAX_PASTE_EXPIRATION";
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -45,6 +45,13 @@ pub enum Error {
     SigningKey(String),
     #[error("failed to parse {VAR_HTTP_TIMEOUT}: {0}")]
     HttpTimeout(ParseIntError),
+    #[error("failed to parse {VAR_MAX_PASTE_EXPIRATION}: {0}")]
+    MaxPasteExpiration(ParseIntError),
+    #[error(
+        "{VAR_MAX_PASTE_EXPIRATION} is too large (max {}), pass -1 if you mean no expiry",
+        u32::MAX
+    )]
+    ExpirationTooLarge,
 }
 
 pub struct BasePath(String);
@@ -185,21 +192,16 @@ pub fn http_timeout() -> Result<Duration, Error> {
         .map_err(Error::HttpTimeout)
 }
 
-pub fn max_paste_expiry() -> Option<u32> {
-    std::env::var(VAR_MAX_PASTE_EXPIRY)
+pub fn max_paste_expiration() -> Result<Option<u32>, Error> {
+    std::env::var(VAR_MAX_PASTE_EXPIRATION)
         .ok()
-        .and_then(|raw_max_exp| {
+        .and_then(|raw_max_exp| -> Option<Result<u32, Error>> {
             match raw_max_exp.parse::<i64>() {
                 Ok(max_exp) if max_exp == -1 => None,
-                Ok(max_exp) if max_exp >= u32::MAX as i64 => {
-                    tracing::warn!("the expiry is larger than an u32 ({} seconds, ~136 Years), assuming you mean no timeout", u32::MAX);
-                    None
-                }
-                Ok(max_exp) => Some(max_exp as u32),
-                Err(why) => {
-                    tracing::warn!("unable to parse VAR_MAX_PASTE_EXPIRY `{raw_max_exp}`, defaulting to no expiry: {why}");
-                    None
-                }
+                Ok(max_exp) if max_exp >= u32::MAX as i64 => Some(Err(Error::ExpirationTooLarge)),
+                Ok(max_exp) => Some(Ok(max_exp as u32)),
+                Err(why) => Some(Err(Error::MaxPasteExpiration(why))),
             }
         })
+        .transpose()
 }

src/main.rs

@@ -33,7 +33,7 @@ pub struct AppState {
     cache: Cache,
     key: Key,
     base_url: Option<Url>,
-    max_expiry: Option<u32>,
+    max_expiration: Option<u32>,
 }
 
 impl FromRef<AppState> for Key {
@@ -92,7 +92,7 @@ async fn start() -> Result<(), Box<dyn std::error::Error>> {
     let max_body_size = env::max_body_size()?;
     let base_url = env::base_url()?;
     let timeout = env::http_timeout()?;
-    let max_expiry = env::max_paste_expiry();
+    let max_expiration = env::max_paste_expiration()?;
 
     let cache = Cache::new(cache_size);
     let db = Database::new(method)?;
@@ -101,7 +101,7 @@ async fn start() -> Result<(), Box<dyn std::error::Error>> {
         cache,
         key,
         base_url,
-        max_expiry,
+        max_expiration,
     };
 
     tracing::debug!("serving on {addr}");

src/pages.rs

@@ -1,5 +1,3 @@
-use std::cell::Cell;
-
 use crate::cache::Key as CacheKey;
 use crate::env;
 use crate::highlight::Html;
@@ -33,75 +31,78 @@ impl From<crate::Error> for ErrorResponse<'_> {
 
 /// Index page displaying a form for paste insertion and a selection box for languages.
 #[derive(Template)]
-#[template(path = "index.html")]
+#[template(path = "index.html", print="code")]
 pub struct Index<'a> {
     meta: &'a env::Metadata<'a>,
     base_path: &'static env::BasePath,
-    max_expiry: Option<u32>,
-
-    /// SAFETY: calls in the template are always sequential
-    default_has_been_written: Cell<bool>,
+    max_expiration: Option<u32>,
 }
 
 impl<'a> Default for Index<'a> {
     fn default() -> Self {
         Self {
             meta: env::metadata(),
             base_path: env::base_path(),
-            max_expiry: env::max_paste_expiry(),
-            default_has_been_written: Cell::new(false),
+            // exception should already have been handled in main
+            max_expiration: env::max_paste_expiration().unwrap(),
         }
     }
 }
 
-#[derive(Debug)]
-enum Expiry<'a> {
-    Special(&'a str),
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+enum Expiration {
+    None,
+    Burn,
     Time(u32),
 }
 
-impl<'a> Expiry<'a> {
-    fn as_str(&self) -> String {
+impl std::fmt::Display for Expiration {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Expiry::Special(s) => s.to_string(),
-            Expiry::Time(u) => u.to_string(),
+            Expiration::None => write!(f, ""),
+            Expiration::Burn => write!(f, "burn"),
+            Expiration::Time(t) => write!(f, "{}", t),
         }
     }
 }
 
-impl<'a> Index<'a> {
-    fn expiry(&self, name: &str, time: Expiry<'a>) -> String {
-        let sel_string = if self.default_has_been_written.get() {
-            ""
-        } else {
-            r#" selected"#
-        };
+const EXPIRATION_OPTIONS: [(&'static str, Expiration); 8] = [
+    ("never", Expiration::None),
+    ("10 minutes", Expiration::Time(600)),
+    ("1 hour", Expiration::Time(3600)),
+    ("1 day", Expiration::Time(86400)),
+    ("1 week", Expiration::Time(604800)),
+    ("1 month", Expiration::Time(2592000)),
+    ("1 year", Expiration::Time(31536000)),
+    ("🔥 after reading", Expiration::Burn),
+];
 
-        match self.max_expiry {
-            Some(exp) => {
-                match time {
-                    // never emit an never expire with a limit
-                    Expiry::Special("") => String::new(),
-                    Expiry::Special("burn") => {
-                        self.default_has_been_written.set(true);
-                        format!(r#"<option{} value="burn">{}</option>"#, sel_string, name)
-                    }
-                    Expiry::Special(_) => String::new(),
-                    Expiry::Time(t) if t > exp => String::new(),
-                    Expiry::Time(t) => {
-                        self.default_has_been_written.set(true);
-                        format!(r#"<option{} value="{}">{}</option>"#, sel_string, t, name)
-                    }
+impl<'a> Index<'a> {
+    fn expiry_options(&self) -> String {
+        let mut option_set = String::new();
+        let mut wrote_first = false;
+
+        option_set.push('\n');
+
+        for (opt_name, opt_val) in EXPIRATION_OPTIONS {
+            if self.max_expiration.is_none()
+                || opt_val == Expiration::Burn
+                || matches!((self.max_expiration, opt_val), (Some(exp), Expiration::Time(time)) if time <= exp)
+            {
+                option_set.push_str("<option");
+                if !wrote_first {
+                    option_set.push_str(" selected");
+                    wrote_first = true;
                 }
-            }
-            None => {
-                self.default_has_been_written.set(true);
-                format!(
-                    r#"<option{} value="{}">{}</option>"#,
-                    sel_string, time, name
-                )
+                option_set.push_str(" value=\"");
+                option_set.push_str(opt_val.to_string().as_ref());
+                option_set.push_str("\">");
+                option_set.push_str(opt_name);
+                option_set.push_str("</option>\n");
             }
         }
+
+        option_set
     }
 }
 

src/routes/form.rs

@@ -72,7 +72,7 @@ pub async fn insert(
 
     let url_with_base = base_path().join(&url);
 
-    if let Some(max_exp) = state.max_expiry {
+    if let Some(max_exp) = state.max_expiration {
         entry.expires = entry
             .expires
             .map_or_else(|| Some(max_exp), |value| Some(value.min(max_exp)));

src/routes/json.rs

@@ -49,7 +49,7 @@ pub async fn insert(
 
     let mut entry: write::Entry = entry.into();
 
-    if let Some(max_exp) = state.max_expiry {
+    if let Some(max_exp) = state.max_expiration {
         entry.expires = entry
             .expires
             .map_or_else(|| Some(max_exp), |value| Some(value.min(max_exp)));

src/test_helpers.rs

@@ -63,7 +63,7 @@ pub(crate) fn make_app() -> Result<Router, Box<dyn std::error::Error>> {
         cache,
         key,
         base_url,
-        max_expiry: None,
+        max_expiration: None,
     };
 
     Ok(crate::make_app(4096, Duration::new(30, 0)).with_state(state))

templates/index.html

@@ -76,14 +76,8 @@
           <input type="search" id="filter" placeholder="Filter ..." onchange="filterLangs(event);" onkeyup="filterLangs(event)"></input>
         </div>
         <div class="expiration-list">
-          <select name="expires" size="7">
-            {{ Self::expiry(self, "never",      Expiry::Special(""))|safe }}
-            {{ Self::expiry(self, "10 minutes", Expiry::Time(600))|safe }}
-            {{ Self::expiry(self, "1 hour",     Expiry::Time(3600))|safe }}
-            {{ Self::expiry(self, "1 day",      Expiry::Time(86400))|safe }}
-            {{ Self::expiry(self, "1 week",     Expiry::Time(604800))|safe }}
-            {{ Self::expiry(self, "1 year",     Expiry::Time(215308800))|safe }}
-            {{ Self::expiry(self, "burn",       Expiry::Special("burn"))|safe }}
+          <select name="expires" size="8">
+            {{- Self::expiry_options(self)|safe }}
           </select>
         </div>
         <div class="password">