header marshaling should be case-insensitive and more robust

dropshot/src/extractor/header.rs

@@ -9,19 +9,29 @@ use schemars::JsonSchema;
 use serde::de::DeserializeOwned;
 
 use crate::{
-    from_map::from_map, ApiEndpointBodyContentType,
+    from_map::from_map_insensitive, ApiEndpointBodyContentType,
     ApiEndpointParameterLocation, HttpError, RequestContext, RequestInfo,
     ServerContext,
 };
 
 use super::{metadata::get_metadata, ExtractorMetadata, SharedExtractor};
 
 /// `Header<HeaderType>` is an extractor used to deserialize an instance of
-/// `HeaderType` from an HTTP request's header values. `PathType` may be any
+/// `HeaderType` from an HTTP request's header values. `HeaderType` may be any
 /// structure that implements [serde::Deserialize] and [schemars::JsonSchema].
 /// While headers are accessible through [RequestInfo::headers], using this
 /// extractor in an entrypoint causes header inputs to be documented in
 /// OpenAPI output. See the crate documentation for more information.
+///
+/// Note that (unlike the [`Query`] and [`Path`] extractors) headers are case-
+/// insensitive. You may rename fields with mixed casing (e.g. by using
+/// #[serde(rename = "X-Header-Foo")]) and that casing will appear in the
+/// OpenAPI document output. Name conflicts (including names differentiated by
+/// casing since headers are case-insensitive) may lead to unexpected behavior,
+/// and should be avoided. For example, only one of the conflicting fields may
+/// be deserialized, and therefore deserialization may fail if any conflicting
+/// field is required (i.e. not an `Option<T>` type)
+#[derive(Debug)]
 pub struct Header<HeaderType: DeserializeOwned + JsonSchema + Send + Sync> {
     inner: HeaderType,
 }
@@ -50,8 +60,13 @@ where
         .map_err(|message: http::header::ToStrError| {
             HttpError::for_bad_request(None, message.to_string())
         })?;
-    let x: HeaderType = from_map(&headers).unwrap();
-    Ok(Header { inner: x })
+    let inner = from_map_insensitive(&headers).map_err(|message| {
+        HttpError::for_bad_request(
+            None,
+            format!("error processing headers: {message}"),
+        )
+    })?;
+    Ok(Header { inner })
 }
 
 #[async_trait]
@@ -71,3 +86,74 @@ where
         get_metadata::<HeaderType>(&ApiEndpointParameterLocation::Header)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use schemars::JsonSchema;
+    use serde::Deserialize;
+
+    use crate::{extractor::header::http_request_load_header, RequestInfo};
+
+    #[test]
+    fn test_header_parsing() {
+        #[allow(dead_code)]
+        #[derive(Debug, Deserialize, JsonSchema)]
+        pub struct TestHeaders {
+            header_a: String,
+            #[serde(rename = "X-Header-B")]
+            header_b: String,
+        }
+
+        let addr = std::net::SocketAddr::new(
+            std::net::Ipv4Addr::LOCALHOST.into(),
+            8080,
+        );
+
+        let request =
+            hyper::Request::builder().uri("http://localhost").body(()).unwrap();
+        let info = RequestInfo::new(&request, addr);
+
+        let parsed = http_request_load_header::<TestHeaders>(&info);
+        assert!(parsed.is_err());
+
+        let request = hyper::Request::builder()
+            .header("header_a", "header_a value")
+            .header("X-Header-B", "header_b value")
+            .uri("http://localhost")
+            .body(())
+            .unwrap();
+        let info = RequestInfo::new(&request, addr);
+
+        let parsed = http_request_load_header::<TestHeaders>(&info);
+
+        match parsed {
+            Ok(headers) => {
+                assert_eq!(headers.inner.header_a, "header_a value");
+                assert_eq!(headers.inner.header_b, "header_b value");
+            }
+            Err(e) => {
+                panic!("unexpected error: {}", e);
+            }
+        }
+
+        let request = hyper::Request::builder()
+            .header("header_a", "header_a value")
+            .header("X-hEaDEr-b", "header_b value")
+            .uri("http://localhost")
+            .body(())
+            .unwrap();
+        let info = RequestInfo::new(&request, addr);
+
+        let parsed = http_request_load_header::<TestHeaders>(&info);
+
+        match parsed {
+            Ok(headers) => {
+                assert_eq!(headers.inner.header_a, "header_a value");
+                assert_eq!(headers.inner.header_b, "header_b value");
+            }
+            Err(e) => {
+                panic!("unexpected error: {}", e);
+            }
+        }
+    }
+}