Add a test to ensure we don't panic when the header is not given

Author: sgrif

src/config.rs

@@ -18,6 +18,7 @@ pub struct Config {
     pub mirror: Replica,
     pub api_protocol: String,
     pub publish_rate_limit: PublishRateLimit,
+    pub blocked_traffic: Vec<(String, Vec<String>)>,
 }
 
 impl Default for Config {
@@ -135,6 +136,48 @@ impl Default for Config {
             mirror,
             api_protocol,
             publish_rate_limit: Default::default(),
+            blocked_traffic: blocked_traffic(),
         }
     }
 }
+
+fn blocked_traffic() -> Vec<(String, Vec<String>)> {
+    let pattern_list = dotenv::var("BLOCKED_TRAFFIC").unwrap_or_default();
+    parse_traffic_patterns(&pattern_list)
+        .map(|(header, value_env_var)| {
+            let value_list = dotenv::var(value_env_var).unwrap_or_default();
+            let values = value_list.split(',').map(String::from).collect();
+            (header.into(), values)
+        })
+        .collect()
+}
+
+fn parse_traffic_patterns(patterns: &str) -> impl Iterator<Item = (&str, &str)> {
+    patterns.split_terminator(',').map(|pattern| {
+        if let Some(idx) = pattern.find('=') {
+            (&pattern[..idx], &pattern[(idx + 1)..])
+        } else {
+            panic!(
+                "BLOCKED_TRAFFIC must be in the form HEADER=VALUE_ENV_VAR, \
+                 got invalid pattern {}",
+                pattern
+            )
+        }
+    })
+}
+
+#[test]
+fn parse_traffic_patterns_splits_on_comma_and_looks_for_equal_sign() {
+    let pattern_string_1 = "Foo=BAR,Bar=BAZ";
+    let pattern_string_2 = "Baz=QUX";
+    let pattern_string_3 = "";
+
+    let patterns_1 = parse_traffic_patterns(pattern_string_1).collect::<Vec<_>>();
+    assert_eq!(vec![("Foo", "BAR"), ("Bar", "BAZ")], patterns_1);
+
+    let patterns_2 = parse_traffic_patterns(pattern_string_2).collect::<Vec<_>>();
+    assert_eq!(vec![("Baz", "QUX")], patterns_2);
+
+    let patterns_3 = parse_traffic_patterns(pattern_string_3).collect::<Vec<_>>();
+    assert!(patterns_3.is_empty());
+}

src/middleware.rs

@@ -38,7 +38,8 @@ use crate::{App, Env};
 
 pub fn build_middleware(app: Arc<App>, endpoints: R404) -> MiddlewareBuilder {
     let mut m = MiddlewareBuilder::new(endpoints);
-    let env = app.config.env;
+    let config = app.config.clone();
+    let env = config.env;
 
     if env != Env::Test {
         m.add(ensure_well_formed_500::EnsureWellFormed500);
@@ -69,7 +70,7 @@ pub fn build_middleware(app: Arc<App>, endpoints: R404) -> MiddlewareBuilder {
     ));
 
     if env == Env::Production {
-        m.add(SecurityHeaders::new(&app.config.uploader));
+        m.add(SecurityHeaders::new(&config.uploader));
     }
     m.add(AppMiddleware::new(app));
 
@@ -87,7 +88,7 @@ pub fn build_middleware(app: Arc<App>, endpoints: R404) -> MiddlewareBuilder {
 
     m.around(Head::default());
 
-    for (header, blocked_values) in blocked_traffic() {
+    for (header, blocked_values) in config.blocked_traffic {
         m.around(block_traffic::BlockTraffic::new(header, blocked_values));
     }
 
@@ -99,44 +100,3 @@ pub fn build_middleware(app: Arc<App>, endpoints: R404) -> MiddlewareBuilder {
 
     m
 }
-
-fn blocked_traffic() -> Vec<(String, Vec<String>)> {
-    let pattern_list = env::var("BLOCKED_TRAFFIC").unwrap_or_default();
-    parse_traffic_patterns(&pattern_list)
-        .map(|(header, value_env_var)| {
-            let value_list = env::var(value_env_var).unwrap_or_default();
-            let values = value_list.split(',').map(String::from).collect();
-            (header.into(), values)
-        })
-        .collect()
-}
-
-fn parse_traffic_patterns(patterns: &str) -> impl Iterator<Item = (&str, &str)> {
-    patterns.split_terminator(',').map(|pattern| {
-        if let Some(idx) = pattern.find('=') {
-            (&pattern[..idx], &pattern[(idx + 1)..])
-        } else {
-            panic!(
-                "BLOCKED_TRAFFIC must be in the form HEADER=VALUE_ENV_VAR, \
-                 got invalid pattern {}",
-                pattern
-            )
-        }
-    })
-}
-
-#[test]
-fn parse_traffic_patterns_splits_on_comma_and_looks_for_equal_sign() {
-    let pattern_string_1 = "Foo=BAR,Bar=BAZ";
-    let pattern_string_2 = "Baz=QUX";
-    let pattern_string_3 = "";
-
-    let patterns_1 = parse_traffic_patterns(pattern_string_1).collect::<Vec<_>>();
-    assert_eq!(vec![("Foo", "BAR"), ("Bar", "BAZ")], patterns_1);
-
-    let patterns_2 = parse_traffic_patterns(pattern_string_2).collect::<Vec<_>>();
-    assert_eq!(vec![("Baz", "QUX")], patterns_2);
-
-    let patterns_3 = parse_traffic_patterns(pattern_string_3).collect::<Vec<_>>();
-    assert!(patterns_3.is_empty());
-}

src/tests/all.rs

@@ -148,6 +148,7 @@ fn simple_config() -> Config {
         // sniff/record it, but everywhere else we use https
         api_protocol: String::from("http"),
         publish_rate_limit: Default::default(),
+        blocked_traffic: Default::default(),
     }
 }
 

src/tests/server.rs

@@ -26,3 +26,21 @@ fn user_agent_is_not_required_for_download() {
     let resp = anon.run::<()>(req);
     resp.assert_status(302);
 }
+
+#[test]
+fn blocked_traffic_doesnt_panic_if_checked_header_is_not_present() {
+    let (app, anon, user) = TestApp::init()
+        .with_config(|config| {
+            config.blocked_traffic = vec![("Never-Given".into(), vec!["1".into()])];
+        })
+        .with_user();
+
+    app.db(|conn| {
+        CrateBuilder::new("dl_no_ua", user.as_model().id).expect_build(conn);
+    });
+
+    let mut req = anon.request_builder(Method::Get, "/api/v1/crates/dl_no_ua/0.99.0/download");
+    req.header("User-Agent", "");
+    let resp = anon.run::<()>(req);
+    resp.assert_status(302);
+}

src/tests/util.rs

@@ -268,12 +268,18 @@ impl TestAppBuilder {
         (app, anon, user, token)
     }
 
-    pub fn with_publish_rate_limit(mut self, rate: Duration, burst: i32) -> Self {
-        self.config.publish_rate_limit.rate = rate;
-        self.config.publish_rate_limit.burst = burst;
+    pub fn with_config(mut self, f: impl FnOnce(&mut Config)) -> Self {
+        f(&mut self.config);
         self
     }
 
+    pub fn with_publish_rate_limit(self, rate: Duration, burst: i32) -> Self {
+        self.with_config(|config| {
+            config.publish_rate_limit.rate = rate;
+            config.publish_rate_limit.burst = burst;
+        })
+    }
+
     pub fn with_git_index(mut self) -> Self {
         use crate::git;