Merge pull request #385 from halvko/main

yoshuawuyts · web-flow · commit 7d7323a2f679 · 2021-11-03T16:49:19.000+01:00
Change signature of insertion/appending of headers
diff --git a/src/auth/authorization.rs b/src/auth/authorization.rs
@@ -129,7 +129,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(AUTHORIZATION, "<nori ate the tag. yum.>");
+        headers
+            .insert(AUTHORIZATION, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = Authorization::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/auth/basic_auth.rs b/src/auth/basic_auth.rs
@@ -135,7 +135,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(AUTHORIZATION, "<nori ate the tag. yum.>");
+        headers
+            .insert(AUTHORIZATION, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = BasicAuth::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/auth/www_authenticate.rs b/src/auth/www_authenticate.rs
@@ -156,7 +156,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(WWW_AUTHENTICATE, "<nori ate the tag. yum.>");
+        headers
+            .insert(WWW_AUTHENTICATE, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = WwwAuthenticate::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/cache/age.rs b/src/cache/age.rs
@@ -102,7 +102,7 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(AGE, "<nori ate the tag. yum.>");
+        headers.insert(AGE, "<nori ate the tag. yum.>").unwrap();
         let err = Age::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/cache/cache_control/mod.rs b/src/cache/cache_control/mod.rs
@@ -37,7 +37,7 @@ mod test {
     #[test]
     fn ignore_unkonwn_directives() -> crate::Result<()> {
         let mut headers = Headers::new();
-        headers.insert(CACHE_CONTROL, "barrel_roll");
+        headers.insert(CACHE_CONTROL, "barrel_roll").unwrap();
         let entries = CacheControl::from_headers(headers)?.unwrap();
         let mut entries = entries.iter();
         assert!(entries.next().is_none());
@@ -47,7 +47,7 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(CACHE_CONTROL, "min-fresh=0.9"); // floats are not supported
+        headers.insert(CACHE_CONTROL, "min-fresh=0.9").unwrap(); // floats are not supported
         let err = CacheControl::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/cache/clear_site_data/mod.rs b/src/cache/clear_site_data/mod.rs
@@ -278,15 +278,16 @@ mod test {
     #[test]
     fn parse_quotes_correctly() -> crate::Result<()> {
         let mut res = Response::new(200);
-        res.insert_header("clear-site-data", r#""cookies""#);
+        res.insert_header("clear-site-data", r#""cookies""#)
+            .unwrap();
 
         let entries = ClearSiteData::from_headers(res)?.unwrap();
         assert!(!entries.wildcard());
         let mut entries = entries.iter();
         assert_eq!(entries.next().unwrap(), &ClearDirective::Cookies);
 
         let mut res = Response::new(200);
-        res.insert_header("clear-site-data", r#""*""#);
+        res.insert_header("clear-site-data", r#""*""#).unwrap();
 
         let entries = ClearSiteData::from_headers(res)?.unwrap();
         assert!(entries.wildcard());
diff --git a/src/cache/expires.rs b/src/cache/expires.rs
@@ -107,7 +107,7 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(EXPIRES, "<nori ate the tag. yum.>");
+        headers.insert(EXPIRES, "<nori ate the tag. yum.>").unwrap();
         let err = Expires::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/conditional/etag.rs b/src/conditional/etag.rs
@@ -164,7 +164,7 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(ETAG, "<nori ate the tag. yum.>");
+        headers.insert(ETAG, "<nori ate the tag. yum.>").unwrap();
         let err = ETag::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
@@ -179,7 +179,7 @@ mod test {
 
     fn assert_entry_err(s: &str, msg: &str) {
         let mut headers = Headers::new();
-        headers.insert(ETAG, s);
+        headers.insert(ETAG, s).unwrap();
         let err = ETag::from_headers(headers).unwrap_err();
         assert_eq!(format!("{}", err), msg);
     }
diff --git a/src/conditional/if_modified_since.rs b/src/conditional/if_modified_since.rs
@@ -104,7 +104,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(IF_MODIFIED_SINCE, "<nori ate the tag. yum.>");
+        headers
+            .insert(IF_MODIFIED_SINCE, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = IfModifiedSince::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/conditional/if_unmodified_since.rs b/src/conditional/if_unmodified_since.rs
@@ -104,7 +104,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(IF_UNMODIFIED_SINCE, "<nori ate the tag. yum.>");
+        headers
+            .insert(IF_UNMODIFIED_SINCE, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = IfUnmodifiedSince::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/conditional/last_modified.rs b/src/conditional/last_modified.rs
@@ -103,7 +103,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(LAST_MODIFIED, "<nori ate the tag. yum.>");
+        headers
+            .insert(LAST_MODIFIED, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = LastModified::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/content/content_length.rs b/src/content/content_length.rs
@@ -94,7 +94,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(CONTENT_LENGTH, "<nori ate the tag. yum.>");
+        headers
+            .insert(CONTENT_LENGTH, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = ContentLength::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/content/content_location.rs b/src/content/content_location.rs
@@ -118,7 +118,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(CONTENT_LOCATION, "htt://<nori ate the tag. yum.>");
+        headers
+            .insert(CONTENT_LOCATION, "htt://<nori ate the tag. yum.>")
+            .unwrap();
         let err =
             ContentLocation::from_headers(Url::parse("https://example.net").unwrap(), headers)
                 .unwrap_err();
diff --git a/src/content/content_type.rs b/src/content/content_type.rs
@@ -127,7 +127,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(CONTENT_TYPE, "<nori ate the tag. yum.>");
+        headers
+            .insert(CONTENT_TYPE, "<nori ate the tag. yum.>")
+            .unwrap();
         let err = ContentType::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/headers/header.rs b/src/headers/header.rs
@@ -15,7 +15,10 @@ pub trait Header {
     fn apply_header<H: AsMut<Headers>>(&self, mut headers: H) {
         let name = self.header_name();
         let value = self.header_value();
-        headers.as_mut().insert(name, value);
+
+        // The value should already have been validated when it was created, so this should
+        // possibly be done with an unsafe call
+        headers.as_mut().insert(name, value).unwrap();
     }
 }
 
diff --git a/src/headers/header_name.rs b/src/headers/header_name.rs
@@ -172,7 +172,8 @@ mod tests {
     #[test]
     fn pass_name_by_ref() {
         let mut res = crate::Response::new(200);
-        res.insert_header(&crate::headers::HOST, "127.0.0.1");
+        res.insert_header(&crate::headers::HOST, "127.0.0.1")
+            .unwrap();
     }
 
     #[test]
diff --git a/src/headers/headers.rs b/src/headers/headers.rs
@@ -49,27 +49,32 @@ impl Headers {
         &mut self,
         name: impl Into<HeaderName>,
         values: impl ToHeaderValues,
-    ) -> Option<HeaderValues> {
+    ) -> crate::Result<Option<HeaderValues>> {
         let name = name.into();
-        let values: HeaderValues = values.to_header_values().unwrap().collect();
-        self.headers.insert(name, values)
+        let values: HeaderValues = values.to_header_values()?.collect();
+        Ok(self.headers.insert(name, values))
     }
 
     /// Append a header to the headers.
     ///
     /// Unlike `insert` this function will not override the contents of a header, but insert a
     /// header if there aren't any. Or else append to the existing list of headers.
-    pub fn append(&mut self, name: impl Into<HeaderName>, values: impl ToHeaderValues) {
+    pub fn append(
+        &mut self,
+        name: impl Into<HeaderName>,
+        values: impl ToHeaderValues,
+    ) -> crate::Result<()> {
         let name = name.into();
         match self.get_mut(&name) {
             Some(headers) => {
-                let mut values: HeaderValues = values.to_header_values().unwrap().collect();
+                let mut values: HeaderValues = values.to_header_values()?.collect();
                 headers.append(&mut values);
             }
             None => {
-                self.insert(name, values);
+                self.insert(name, values)?;
             }
         }
+        Ok(())
     }
 
     /// Get a reference to a header.
@@ -208,9 +213,9 @@ mod tests {
         let non_static_header = HeaderName::from_str("hello")?;
 
         let mut headers = Headers::new();
-        headers.append(STATIC_HEADER, "foo0");
-        headers.append(static_header.clone(), "foo1");
-        headers.append(non_static_header.clone(), "foo2");
+        headers.append(STATIC_HEADER, "foo0").unwrap();
+        headers.append(static_header.clone(), "foo1").unwrap();
+        headers.append(non_static_header.clone(), "foo2").unwrap();
 
         assert_eq!(headers[STATIC_HEADER], ["foo0", "foo1", "foo2",][..]);
         assert_eq!(headers[static_header], ["foo0", "foo1", "foo2",][..]);
@@ -222,23 +227,23 @@ mod tests {
     #[test]
     fn index_into_headers() {
         let mut headers = Headers::new();
-        headers.insert("hello", "foo0");
+        headers.insert("hello", "foo0").unwrap();
         assert_eq!(headers["hello"], "foo0");
         assert_eq!(headers.get("hello").unwrap(), "foo0");
     }
 
     #[test]
     fn test_debug_single() {
         let mut headers = Headers::new();
-        headers.insert("single", "foo0");
+        headers.insert("single", "foo0").unwrap();
         assert_eq!(format!("{:?}", headers), r#"{"single": "foo0"}"#);
     }
 
     #[test]
     fn test_debug_multiple() {
         let mut headers = Headers::new();
-        headers.append("multi", "foo0");
-        headers.append("multi", "foo1");
+        headers.append("multi", "foo0").unwrap();
+        headers.append("multi", "foo1").unwrap();
         assert_eq!(format!("{:?}", headers), r#"{"multi": ["foo0", "foo1"]}"#);
     }
 }
diff --git a/src/hyperium_http.rs b/src/hyperium_http.rs
@@ -102,7 +102,7 @@ impl TryFrom<http::HeaderMap> for Headers {
                 if let Some(name) = name {
                     let value: HeaderValue = value.try_into()?;
                     let name: HeaderName = name.try_into()?;
-                    headers.append(name, value);
+                    headers.append(name, value)?;
                 }
                 Ok(())
             })
@@ -140,16 +140,20 @@ impl TryFrom<Headers> for http::HeaderMap {
     }
 }
 
-fn hyperium_headers_to_headers(hyperium_headers: http::HeaderMap, headers: &mut Headers) {
+fn hyperium_headers_to_headers(
+    hyperium_headers: http::HeaderMap,
+    headers: &mut Headers,
+) -> crate::Result<()> {
     for (name, value) in hyperium_headers {
         let value = value.as_bytes().to_owned();
         let value = unsafe { HeaderValue::from_bytes_unchecked(value) };
         if let Some(name) = name {
             let name = name.as_str().as_bytes().to_owned();
             let name = unsafe { HeaderName::from_bytes_unchecked(name) };
-            headers.append(name, value);
+            headers.append(name, value)?;
         }
     }
+    Ok(())
 }
 
 fn headers_to_hyperium_headers(headers: &mut Headers, hyperium_headers: &mut http::HeaderMap) {
@@ -176,7 +180,7 @@ fn from_url_to_uri(url: &Url) -> http::Uri {
 }
 
 impl TryFrom<http::Request<Body>> for Request {
-    type Error = crate::url::ParseError;
+    type Error = crate::Error;
 
     fn try_from(req: http::Request<Body>) -> Result<Self, Self::Error> {
         let (parts, body) = req.into_parts();
@@ -185,7 +189,7 @@ impl TryFrom<http::Request<Body>> for Request {
         let mut req = Request::new(method, url);
         req.set_body(body);
         req.set_version(Some(parts.version.into()));
-        hyperium_headers_to_headers(parts.headers, req.as_mut());
+        hyperium_headers_to_headers(parts.headers, req.as_mut())?;
         Ok(req)
     }
 }
@@ -203,14 +207,15 @@ impl From<Request> for http::Request<Body> {
     }
 }
 
-impl From<http::Response<Body>> for Response {
-    fn from(res: http::Response<Body>) -> Self {
+impl TryFrom<http::Response<Body>> for Response {
+    type Error = crate::Error;
+    fn try_from(res: http::Response<Body>) -> Result<Self, Self::Error> {
         let (parts, body) = res.into_parts();
         let mut res = Response::new(parts.status);
         res.set_body(body);
         res.set_version(Some(parts.version.into()));
-        hyperium_headers_to_headers(parts.headers, res.as_mut());
-        res
+        hyperium_headers_to_headers(parts.headers, res.as_mut())?;
+        Ok(res)
     }
 }
 
diff --git a/src/other/date.rs b/src/other/date.rs
@@ -129,7 +129,7 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(DATE, "<nori ate the tag. yum.>");
+        headers.insert(DATE, "<nori ate the tag. yum.>").unwrap();
         let err = Date::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/other/expect.rs b/src/other/expect.rs
@@ -87,7 +87,7 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(EXPECT, "<nori ate the tag. yum.>");
+        headers.insert(EXPECT, "<nori ate the tag. yum.>").unwrap();
         let err = Expect::from_headers(headers).unwrap_err();
         assert_eq!(err.status(), 400);
     }
diff --git a/src/other/referer.rs b/src/other/referer.rs
@@ -123,7 +123,9 @@ mod test {
     #[test]
     fn bad_request_on_parse_error() {
         let mut headers = Headers::new();
-        headers.insert(REFERER, "htt://<nori ate the tag. yum.>");
+        headers
+            .insert(REFERER, "htt://<nori ate the tag. yum.>")
+            .unwrap();
         let err =
             Referer::from_headers(Url::parse("https://example.net").unwrap(), headers).unwrap_err();
         assert_eq!(err.status(), 500);
@@ -132,7 +134,7 @@ mod test {
     #[test]
     fn fallback_works() -> crate::Result<()> {
         let mut headers = Headers::new();
-        headers.insert(REFERER, "/test.json");
+        headers.insert(REFERER, "/test.json").unwrap();
 
         let base_url = Url::parse("https://fallback.net/")?;
         let referer = Referer::from_headers(base_url, headers)?.unwrap();
@@ -142,7 +144,9 @@ mod test {
         );
 
         let mut headers = Headers::new();
-        headers.insert(REFERER, "https://example.com/test.json");
+        headers
+            .insert(REFERER, "https://example.com/test.json")
+            .unwrap();
 
         let base_url = Url::parse("https://fallback.net/")?;
         let referer = Referer::from_headers(base_url, headers)?.unwrap();
diff --git a/src/other/source_map.rs b/src/other/source_map.rs
diff --git a/src/proxies/forwarded.rs b/src/proxies/forwarded.rs
diff --git a/src/request.rs b/src/request.rs
diff --git a/src/response.rs b/src/response.rs
diff --git a/src/security/csp.rs b/src/security/csp.rs
diff --git a/src/security/mod.rs b/src/security/mod.rs
diff --git a/src/security/timing_allow_origin.rs b/src/security/timing_allow_origin.rs
diff --git a/src/trace/server_timing/mod.rs b/src/trace/server_timing/mod.rs
diff --git a/src/trace/trace_context.rs b/src/trace/trace_context.rs
diff --git a/src/trailers.rs b/src/trailers.rs

Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,7 @@ mod test {`
`102`	`102`	`#[test]`
`103`	`103`	`fn bad_request_on_parse_error() {`
`104`	`104`	`let mut headers = Headers::new();`
`105`		`- headers.insert(AGE, "<nori ate the tag. yum.>");`
	`105`	`+ headers.insert(AGE, "<nori ate the tag. yum.>").unwrap();`
`106`	`106`	`let err = Age::from_headers(headers).unwrap_err();`
`107`	`107`	`assert_eq!(err.status(), 400);`
`108`	`108`	`}`
Original file line number	Diff line number	Diff line change
`@@ -107,7 +107,7 @@ mod test {`
`107`	`107`	`#[test]`
`108`	`108`	`fn bad_request_on_parse_error() {`
`109`	`109`	`let mut headers = Headers::new();`
`110`		`- headers.insert(EXPIRES, "<nori ate the tag. yum.>");`
	`110`	`+ headers.insert(EXPIRES, "<nori ate the tag. yum.>").unwrap();`
`111`	`111`	`let err = Expires::from_headers(headers).unwrap_err();`
`112`	`112`	`assert_eq!(err.status(), 400);`
`113`	`113`	`}`