mozilla · Almaju · Sep 2, 2026 · Sep 2, 2026 · Sep 2, 2026 · Sep 2, 2026
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
 ### Ads-Client
 
 - Added `blocks: Vec<String>` to `ffi::MozAdsRequestOptions`, `AdsClient::request*_ads`, `MARSClient::fetch_ads`, `mars::AdRequest`, and `mars::AdRequest::try_new`. This is serialized and passed to MARS so that it can remove blocks server-side.
+- 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.
 
 # v156.0 (_2026-08-27_)
 

@@ -216,8 +216,8 @@ fn test_contract_tile_ohttp_prod() {
     viaduct::ohttp::configure_ohttp_channel(
         "ads-client".to_string(),
         viaduct::ohttp::OhttpConfig {
-            relay_url: "https://mozilla-ohttp.fastly-edge.com/".to_string(),
             gateway_host: "prod.ohttp-gateway.prod.webservices.mozgcp.net".to_string(),
+            relay_url: "https://mozilla-ohttp.fastly-edge.com/".to_string(),
         },
     )
     .expect("OHTTP channel configuration should succeed");

@@ -98,18 +98,6 @@ where
         self.client.clear_cache()
     }
 
-    // Shutdown the db connection and drop references to telemetry callbacks.
-    // Should be used only when dropping the ads client, this may be extended to drop more things.
-    pub fn shutdown_client(&mut self) -> Result<(), rusqlite::Error> {
-        // Drop telemetry (within the telemetry wrapper)
-        self.telemetry.shutdown();
-
-        // Shutdown DB
-        self.client.shutdown_db()?;
-
-        Ok(())
-    }
-
     pub fn get_context_id(&self) -> context_id::ApiResult<String> {
         self.context_id_provider.context_id()
     }
@@ -249,6 +237,18 @@ where
             })
     }
 
+    // Shutdown the db connection and drop references to telemetry callbacks.
+    // Should be used only when dropping the ads client, this may be extended to drop more things.
+    pub fn shutdown_client(&mut self) -> Result<(), rusqlite::Error> {
+        // Drop telemetry (within the telemetry wrapper)
+        self.telemetry.shutdown();
+
+        // Shutdown DB
+        self.client.shutdown_db()?;
+
+        Ok(())
+    }
+
     fn request_ads<A>(
         &self,
         placements: Vec<AdPlacementRequest>,

@@ -170,7 +170,7 @@ impl MozAdsClientBuilder {
     }
 }
 
-#[derive(Clone, Copy, Debug, Default, uniffi::Enum, Eq, PartialEq)]
+#[derive(Clone, Copy, Debug, Default, uniffi::Enum, PartialEq, Eq)]
 pub enum MozAdsEnvironment {
     #[default]
     Prod,

@@ -58,12 +58,6 @@ impl MozAdsTelemetryWrapper {
 }
 
 impl Telemetry for MozAdsTelemetryWrapper {
-    // MozAdsTelemetry has hanging uniffi callbacks which need to be explicitly dropped before closing.
-    // This replaces it with a `None` internally, meaning future calls will be noops.
-    fn shutdown(&self) {
-        let _dropped = self.inner.write().take();
-    }
-
     fn record(&self, event: &dyn Any) {
         let Some(inner) = self.inner.read().clone() else {
             return;
@@ -146,6 +140,12 @@ impl Telemetry for MozAdsTelemetryWrapper {
         #[cfg(test)]
         panic!("Unsupported telemetry event type: {:?}", event.type_id());
     }
+
+    // MozAdsTelemetry has hanging uniffi callbacks which need to be explicitly dropped before closing.
+    // This replaces it with a `None` internally, meaning future calls will be noops.
+    fn shutdown(&self) {
+        let _dropped = self.inner.write().take();
+    }
 }
 
 #[cfg(test)]

@@ -60,10 +60,6 @@ impl HttpCache {
         Ok(())
     }
 
-    pub fn shutdown_db(self) -> Result<(), rusqlite::Error> {
-        self.store.close()
-    }
-
     pub fn invalidate_by_hash(&self, request_hash: &RequestHash) -> Result<(), rusqlite::Error> {
         self.store.invalidate_by_hash(request_hash)?;
         Ok(())
@@ -87,17 +83,17 @@ impl HttpCache {
         // Apply the cache policy and collect outcomes
         let (response, mut strategy_outcomes) = match policy {
             CachePolicy::CacheFirst { ttl } => CacheFirst {
+                default_ttl: self.default_ttl,
+                explicit_ttl: *ttl,
                 hash,
                 request,
-                explicit_ttl: *ttl,
-                default_ttl: self.default_ttl,
             }
             .apply(client, &self.store),
             CachePolicy::NetworkFirst { ttl } => NetworkFirst {
+                default_ttl: self.default_ttl,
+                explicit_ttl: *ttl,
                 hash,
                 request,
-                explicit_ttl: *ttl,
-                default_ttl: self.default_ttl,
             }
             .apply(client, &self.store),
         }?;
@@ -115,6 +111,10 @@ impl HttpCache {
 
         Ok((response, outcomes))
     }
+
+    pub fn shutdown_db(self) -> Result<(), rusqlite::Error> {
+        self.store.close()
+    }
 }
 
 #[cfg(test)]
@@ -192,11 +192,11 @@ mod tests {
         let hash = RequestHash::new(&("Get", "https://example.com/test"));
 
         let response = viaduct::Response {
+            body: b"test response".to_vec(),
+            headers: viaduct::Headers::new(),
             request_method: viaduct::Method::Get,
-            url: "https://example.com/test".parse().unwrap(),
             status: 200,
-            headers: viaduct::Headers::new(),
-            body: b"test response".to_vec(),
+            url: "https://example.com/test".parse().unwrap(),
         };
 
         cache
@@ -496,11 +496,11 @@ mod tests {
         let hash2 = RequestHash::new(&("Post", "https://example.com/api2"));
 
         let response = viaduct::Response {
+            body: b"test response".to_vec(),
+            headers: viaduct::Headers::new(),
             request_method: viaduct::Method::Post,
-            url: "https://example.com/test".parse().unwrap(),
             status: 200,
-            headers: viaduct::Headers::new(),
-            body: b"test response".to_vec(),
+            url: "https://example.com/test".parse().unwrap(),
         };
 
         cache

@@ -22,10 +22,10 @@ const MAX_TTL: Duration = Duration::from_secs(60 * 60 * 24 * 7); // 7 days
 
 #[derive(Debug, thiserror::Error)]
 pub enum HttpCacheBuilderError {
-    #[error("Database path cannot be empty")]
-    EmptyDbPath,
     #[error("Database error: {0}")]
     Database(#[from] open_database::Error),
+    #[error("Database path cannot be empty")]
+    EmptyDbPath,
     #[error(
         "Maximum cache size must be between {min_size} and {max_size}, got {size_bytes} bytes"
     )]
@@ -44,67 +44,19 @@ pub enum HttpCacheBuilderError {
 
 pub struct HttpCacheBuilder {
     db_path: PathBuf,
-    max_size: Option<ByteSize>,
     default_ttl: Option<Duration>,
+    max_size: Option<ByteSize>,
 }
 
 impl HttpCacheBuilder {
     pub fn new(db_path: impl Into<PathBuf>) -> Self {
         Self {
             db_path: db_path.into(),
-            max_size: None,
             default_ttl: None,
+            max_size: None,
         }
     }
 
-    pub fn max_size(mut self, max_size: ByteSize) -> Self {
-        self.max_size = Some(max_size);
-        self
-    }
-
-    pub fn default_ttl(mut self, ttl: Duration) -> Self {
-        self.default_ttl = Some(ttl);
-        self
-    }
-
-    fn validate(&self) -> Result<(), HttpCacheBuilderError> {
-        if self.db_path.to_string_lossy().trim().is_empty() {
-            return Err(HttpCacheBuilderError::EmptyDbPath);
-        }
-
-        if let Some(max_size) = self.max_size {
-            if max_size < MIN_CACHE_SIZE || max_size > MAX_CACHE_SIZE {
-                return Err(HttpCacheBuilderError::InvalidMaxSize {
-                    size_bytes: max_size.as_u64(),
-                    min_size: MIN_CACHE_SIZE.to_string(),
-                    max_size: MAX_CACHE_SIZE.to_string(),
-                });
-            }
-        }
-
-        if let Some(ttl) = self.default_ttl {
-            if !(MIN_TTL..=MAX_TTL).contains(&ttl) {
-                return Err(HttpCacheBuilderError::InvalidTtl {
-                    ttl: ttl.as_secs(),
-                    min_ttl: format!("{} seconds", MIN_TTL.as_secs()),
-                    max_ttl: format!("{} seconds", MAX_TTL.as_secs()),
-                });
-            }
-        }
-
-        Ok(())
-    }
-
-    fn open_connection(&self) -> Result<Connection, HttpCacheBuilderError> {
-        let initializer = HttpCacheConnectionInitializer {};
-        let conn = if cfg!(test) {
-            open_database::open_memory_database(&initializer)?
-        } else {
-            open_database::open_database(&self.db_path, &initializer)?
-        };
-        Ok(conn)
-    }
-
     pub fn build(&self) -> Result<HttpCache, HttpCacheBuilderError> {
         self.validate()?;
 
@@ -135,6 +87,54 @@ impl HttpCacheBuilder {
             store,
         })
     }
+
+    pub fn default_ttl(mut self, ttl: Duration) -> Self {
+        self.default_ttl = Some(ttl);
+        self
+    }
+
+    pub fn max_size(mut self, max_size: ByteSize) -> Self {
+        self.max_size = Some(max_size);
+        self
+    }
+
+    fn open_connection(&self) -> Result<Connection, HttpCacheBuilderError> {
+        let initializer = HttpCacheConnectionInitializer {};
+        let conn = if cfg!(test) {
+            open_database::open_memory_database(&initializer)?
+        } else {
+            open_database::open_database(&self.db_path, &initializer)?
+        };
+        Ok(conn)
+    }
+
+    fn validate(&self) -> Result<(), HttpCacheBuilderError> {
+        if self.db_path.to_string_lossy().trim().is_empty() {
+            return Err(HttpCacheBuilderError::EmptyDbPath);
+        }
+
+        if let Some(max_size) = self.max_size {
+            if max_size < MIN_CACHE_SIZE || max_size > MAX_CACHE_SIZE {
+                return Err(HttpCacheBuilderError::InvalidMaxSize {
+                    max_size: MAX_CACHE_SIZE.to_string(),
+                    min_size: MIN_CACHE_SIZE.to_string(),
+                    size_bytes: max_size.as_u64(),
+                });
+            }
+        }
+
+        if let Some(ttl) = self.default_ttl {
+            if !(MIN_TTL..=MAX_TTL).contains(&ttl) {
+                return Err(HttpCacheBuilderError::InvalidTtl {
+                    max_ttl: format!("{} seconds", MAX_TTL.as_secs()),
+                    min_ttl: format!("{} seconds", MIN_TTL.as_secs()),
+                    ttl: ttl.as_secs(),
+                });
+            }
+        }
+
+        Ok(())
+    }
 }
 
 #[cfg(test)]

@@ -5,7 +5,7 @@
 use std::fmt;
 use std::ops;
 
-#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub struct ByteSize(u64);
 
 impl ByteSize {

@@ -47,13 +47,13 @@ impl From<&Response> for CacheControl {
 }
 
 impl CacheControl {
-    pub fn should_cache(&self) -> bool {
-        !self.no_store
-    }
-
     pub fn max_age_duration(&self) -> Option<Duration> {
         self.max_age.map(Duration::from_secs)
     }
+
+    pub fn should_cache(&self) -> bool {
+        !self.no_store
+    }
 }
 
 #[cfg(test)]

@@ -3,17 +3,14 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 pub trait Clock: Send + Sync + 'static {
-    fn now_epoch_seconds(&self) -> i64;
     #[cfg(test)]
     fn advance(&self, secs: i64);
+    fn now_epoch_seconds(&self) -> i64;
 }
 
 pub struct CacheClock;
 
 impl Clock for CacheClock {
-    fn now_epoch_seconds(&self) -> i64 {
-        chrono::Utc::now().timestamp()
-    }
     #[cfg(test)]
     fn advance(&self, _secs: i64) {
         panic!(
@@ -23,6 +20,9 @@ impl Clock for CacheClock {
     "
         )
     }
+    fn now_epoch_seconds(&self) -> i64 {
+        chrono::Utc::now().timestamp()
+    }
 }
 
 #[cfg(test)]
@@ -41,11 +41,11 @@ impl TestClock {
 
 #[cfg(test)]
 impl Clock for TestClock {
-    fn now_epoch_seconds(&self) -> i64 {
-        self.now.load(std::sync::atomic::Ordering::Relaxed)
-    }
     fn advance(&self, secs: i64) {
         self.now
             .fetch_add(secs, std::sync::atomic::Ordering::Relaxed);
     }
+    fn now_epoch_seconds(&self) -> i64 {
+        self.now.load(std::sync::atomic::Ordering::Relaxed)
+    }
 }