Address PR review: use millisecond timestamps, remove unnecessary clones

Author: ChristianPavilonis

crates/common/src/integrations/prebid.rs

@@ -262,7 +262,6 @@ impl IntegrationAttributeRewriter for PrebidIntegration {
     }
 }
 
-
 fn transform_prebid_response(
     response: &mut Json,
     request_host: &str,
@@ -620,24 +619,22 @@ impl AuctionProvider for PrebidAuctionProvider {
         log::info!("Prebid: requesting bids for {} slots", request.slots.len());
 
         // Create signer and compute signature if request signing is enabled
-        let signer_with_signature =
-            if let Some(request_signing_config) = &context.settings.request_signing {
-                if request_signing_config.enabled {
-                    let request_info = RequestInfo::from_request(context.request);
-                    let signer = RequestSigner::from_config()?;
-                    let params = SigningParams::new(
-                        request.id.clone(),
-                        request_info.host,
-                        request_info.scheme,
-                    );
-                    let signature = signer.sign_request(&params)?;
-                    Some((signer, signature, params))
-                } else {
-                    None
-                }
+        let signer_with_signature = if let Some(request_signing_config) =
+            &context.settings.request_signing
+        {
+            if request_signing_config.enabled {
+                let request_info = RequestInfo::from_request(context.request);
+                let signer = RequestSigner::from_config()?;
+                let params =
+                    SigningParams::new(request.id.clone(), request_info.host, request_info.scheme);
+                let signature = signer.sign_request(&params)?;
+                Some((signer, signature, params))
             } else {
                 None
-            };
+            }
+        } else {
+            None
+        };
 
         // Convert to OpenRTB with all enrichments
         let openrtb = self.to_openrtb(

crates/common/src/openrtb.rs

@@ -124,7 +124,7 @@ pub struct TrustedServerExt {
     pub request_host: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub request_scheme: Option<String>,
-    /// Unix timestamp for replay protection
+    /// Unix timestamp in milliseconds for replay protection
     #[serde(skip_serializing_if = "Option::is_none")]
     pub ts: Option<u64>,
 }

crates/common/src/request_signing/signing.rs

@@ -58,7 +58,7 @@ pub struct SigningParams {
 }
 
 impl SigningParams {
-    /// Creates a new `SigningParams` with the current timestamp
+    /// Creates a new `SigningParams` with the current timestamp in milliseconds
     #[must_use]
     pub fn new(request_id: String, request_host: String, request_scheme: String) -> Self {
         Self {
@@ -67,7 +67,7 @@ impl SigningParams {
             request_scheme,
             timestamp: std::time::SystemTime::now()
                 .duration_since(std::time::UNIX_EPOCH)
-                .map(|d| d.as_secs())
+                .map(|d| d.as_millis() as u64)
                 .unwrap_or(0),
         }
     }
@@ -313,13 +313,13 @@ mod tests {
         assert_eq!(params.request_id, "req-123");
         assert_eq!(params.request_host, "example.com");
         assert_eq!(params.request_scheme, "https");
-        // Timestamp should be recent (within last minute)
-        let now = std::time::SystemTime::now()
+        // Timestamp should be recent (within last minute), in milliseconds
+        let now_ms = std::time::SystemTime::now()
             .duration_since(std::time::UNIX_EPOCH)
             .unwrap()
-            .as_secs();
-        assert!(params.timestamp <= now);
-        assert!(params.timestamp >= now - 60);
+            .as_millis() as u64;
+        assert!(params.timestamp <= now_ms);
+        assert!(params.timestamp >= now_ms - 60_000);
     }
 
     #[test]
@@ -361,7 +361,10 @@ mod tests {
         let sig1 = signer.sign_request(&params1).unwrap();
         let sig2 = signer.sign_request(&params2).unwrap();
 
-        assert_ne!(sig1, sig2, "Different hosts should produce different signatures");
+        assert_ne!(
+            sig1, sig2,
+            "Different hosts should produce different signatures"
+        );
     }
 
     #[test]