fix(ads-client): panic on a malformed OHTTP preflight response

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Ads-Client
 
 - Removed the `MozAdsContextIdProvider` callback interface and the `context_id_provider()` builder method. Wiring it up from Firefox Desktop crashed off the main thread ([Bug 2062806](https://bugzilla.mozilla.org/show_bug.cgi?id=2062806)) and nothing else used it, so the ads client now always uses its embedded `ContextIDComponent`. ([AC-99](https://mozilla-hub.atlassian.net/browse/AC-99))
+- Fixed a panic on the OHTTP request path when the MARS `/v1/ads-preflight` response carries a non-ASCII or CRLF geo location or user agent. The request now fails instead. No binding API change.
 
 ### Glean
 - Updated to v70.0.0 ([#7598](https://github.com/mozilla/application-services/pull/7598))

components/ads-client/src/mars.rs

@@ -87,7 +87,7 @@ where
         if ohttp {
             ad_request
                 .headers
-                .extend(Headers::from(self.fetch_preflight()?));
+                .extend(Headers::try_from(self.fetch_preflight()?)?);
         }
 
         let response = self.transport.send(ad_request, &cache_policy, ohttp)?;
@@ -146,7 +146,7 @@ where
         if ohttp {
             request
                 .headers
-                .extend(Headers::from(self.fetch_preflight()?));
+                .extend(Headers::try_from(self.fetch_preflight()?)?);
         }
         self.transport.fire(request, ohttp).map_err(Into::into)
     }

components/ads-client/src/mars/environment.rs

@@ -25,6 +25,8 @@ impl Environment {
     pub fn into_url(self, path: &str) -> Url {
         let mut url = self.base_url();
         url.path_segments_mut()
+            // Cannot fail: every `base_url()` arm is an `https` URL, which `url`
+            // guarantees is hierarchical.
             .expect("base URL must be hierarchical")
             .pop_if_empty()
             .extend(path.split('/').filter(|segment| !segment.is_empty()));

components/ads-client/src/mars/preflight.rs

@@ -31,17 +31,64 @@ pub struct PreflightResponse {
     pub normalized_ua: String,
 }
 
-impl From<PreflightResponse> for Headers {
-    fn from(preflight: PreflightResponse) -> Self {
+impl TryFrom<PreflightResponse> for Headers {
+    type Error = viaduct::ViaductError;
+
+    fn try_from(preflight: PreflightResponse) -> Result<Self, Self::Error> {
         let mut headers = Headers::new();
-        headers
-            .insert("X-Geo-Location", preflight.geo_location)
-            .expect("valid header");
+        headers.insert("X-Geo-Location", preflight.geo_location)?;
         if !preflight.normalized_ua.is_empty() {
-            headers
-                .insert("X-User-Agent", preflight.normalized_ua)
-                .expect("valid header");
+            headers.insert("X-User-Agent", preflight.normalized_ua)?;
         }
-        headers
+        Ok(headers)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn headers_carry_geo_location_and_normalized_ua() {
+        let headers = Headers::try_from(PreflightResponse {
+            geo_location: "US-CA".to_string(),
+            normalized_ua: "Firefox/140.0".to_string(),
+        })
+        .unwrap();
+
+        assert_eq!(headers.get("X-Geo-Location"), Some("US-CA"));
+        assert_eq!(headers.get("X-User-Agent"), Some("Firefox/140.0"));
+    }
+
+    #[test]
+    fn empty_normalized_ua_is_omitted() {
+        let headers = Headers::try_from(PreflightResponse {
+            geo_location: "US-CA".to_string(),
+            normalized_ua: String::new(),
+        })
+        .unwrap();
+
+        assert_eq!(headers.get("X-Geo-Location"), Some("US-CA"));
+        assert_eq!(headers.get("X-User-Agent"), None);
+    }
+
+    #[test]
+    fn non_ascii_geo_location_is_an_error_not_a_panic() {
+        let result = Headers::try_from(PreflightResponse {
+            geo_location: "Zürich".to_string(),
+            normalized_ua: String::new(),
+        });
+
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn header_injection_in_normalized_ua_is_an_error_not_a_panic() {
+        let result = Headers::try_from(PreflightResponse {
+            geo_location: "US-CA".to_string(),
+            normalized_ua: "Firefox/140.0\r\nX-Injected: yes".to_string(),
+        });
+
+        assert!(result.is_err());
     }
 }