fix(sidecar): don't double-encode file:// telemetry endpoints

datadog-sidecar/src/service/sidecar_server.rs

@@ -724,13 +724,14 @@ impl SidecarInterface for ConnectionSidecarHandler {
                 &config.endpoint,
             );
             cfg.set_endpoint(TelemetryEndpoint {
-                url: Some(endpoint.url.to_string()),
                 api_key: endpoint.api_key.as_deref().map(str::to_owned),
                 test_token: endpoint.test_token.as_deref().map(str::to_owned),
                 timeout_ms: endpoint.timeout_ms,
                 use_system_resolver: endpoint.use_system_resolver,
+                ..Default::default()
             })
             .ok();
+            cfg.set_endpoint_uri(endpoint.url).ok();
             cfg.telemetry_heartbeat_interval = config.telemetry_heartbeat_interval;
             cfg.telemetry_extended_heartbeat_interval =
                 config.telemetry_extended_heartbeat_interval;

libdd-common/src/lib.rs

@@ -286,6 +286,20 @@ where
     builder.build().map_err(Error::custom)
 }
 
+/// Converts a human-facing URL string into the internal [`http::Uri`]
+/// representation.
+///
+/// NOTE: the name is misleading. For `http`/`https` this is an ordinary parse,
+/// but for the `file`/`unix`/`windows` schemes it *encodes* the path into the
+/// URI authority (see `encode_uri_path_in_authority`), so it is a
+/// URL-string-to-`Uri` *constructor*, not a pure parser.
+///
+/// WARNING: this is NOT idempotent for those three schemes. The `Uri` it
+/// returns stringifies back to the encoded form (`file://<hex>/`), and feeding
+/// that string in again re-encodes it, double-encoding the path. Only ever call
+/// this on an original URL string — never on the `.to_string()` of a `Uri` that
+/// already came out of here.
+///
 /// TODO: we should properly handle malformed urls
 /// * For windows and unix schemes:
 ///     * For compatibility reasons with existing implementation this parser stores the encoded path

libdd-crashtracker/src/crash_info/telemetry.rs

@@ -198,23 +198,25 @@ impl TelemetryCrashUploader {
             // But do we want to support direct submission to the intake?
 
             // ignore result because what are we going to do?
-            let telemetry_endpoint = if endpoint.url.scheme_str() == Some("file") {
+            if endpoint.url.scheme_str() == Some("file") {
+                // Write telemetry alongside the crash file. Rebuild from the
+                // decoded path so the file path is re-encoded exactly once.
                 let path = libdd_common::decode_uri_path_in_authority(&endpoint.url)
                     .context("file path is not valid")?;
-                libdd_telemetry::config::TelemetryEndpoint {
+                let _ = cfg.set_endpoint(libdd_telemetry::config::TelemetryEndpoint {
                     url: Some(format!("file://{}.telemetry", path.display())),
                     ..Default::default()
-                }
+                });
             } else {
-                libdd_telemetry::config::TelemetryEndpoint {
-                    url: Some(endpoint.url.to_string()),
+                let _ = cfg.set_endpoint(libdd_telemetry::config::TelemetryEndpoint {
                     api_key: endpoint.api_key.as_deref().map(str::to_owned),
                     test_token: endpoint.test_token.as_deref().map(str::to_owned),
                     timeout_ms: endpoint.timeout_ms,
                     use_system_resolver: endpoint.use_system_resolver,
-                }
-            };
-            let _ = cfg.set_endpoint(telemetry_endpoint);
+                    ..Default::default()
+                });
+                let _ = cfg.set_endpoint_uri(endpoint.url.clone());
+            }
         }
 
         parse_tags!(

libdd-telemetry/src/config.rs

@@ -268,8 +268,9 @@ impl Config {
 
     /// Applies a [`TelemetryEndpoint`] patch to the endpoint, then rewrites the
     /// path to the telemetry path appropriate for the resulting scheme and API
-    /// key. This is the single entry point for endpoint configuration so the URL
-    /// path invariant always holds.
+    /// key. This accepts original, human-facing URL strings; callers that
+    /// already hold a parsed [`Uri`] must use [`Config::set_endpoint_uri`] to
+    /// avoid re-parsing encoded `file`, `unix`, or `windows` URIs.
     pub fn set_endpoint(&mut self, endpoint: TelemetryEndpoint) -> anyhow::Result<()> {
         // Parse the URL before touching `self.endpoint` so a parse error leaves
         // the existing endpoint untouched.
@@ -280,8 +281,6 @@ impl Config {
             inner.url = url;
         }
 
-        // Move the owned Strings into the `Cow<'static, str>` fields — `Cow::from(String)`
-        // yields `Cow::Owned`, so no copy happens.
         if let Some(api_key) = endpoint.api_key {
             inner.api_key = Some(Cow::from(api_key));
         }
@@ -299,6 +298,17 @@ impl Config {
         self.apply_telemetry_path()
     }
 
+    /// Sets the endpoint URL from an already-parsed [`Uri`].
+    ///
+    /// This is the non-stringifying counterpart to the `url` field in
+    /// [`TelemetryEndpoint`]. It is intended for integrations that already hold
+    /// a parsed URI and therefore must not pass its string representation back
+    /// through [`parse_uri`]. Other endpoint properties remain unchanged.
+    pub fn set_endpoint_uri(&mut self, uri: Uri) -> anyhow::Result<()> {
+        self.endpoint.get_or_insert_with(Endpoint::default).url = uri;
+        self.apply_telemetry_path()
+    }
+
     /// Sets (or, with `None`, clears) the `X-Datadog-Test-Session-Token` header
     /// sent with requests. Unlike [`Config::set_endpoint`], `None` clears the
     /// token rather than leaving it unchanged, and an absent endpoint is left
@@ -501,6 +511,20 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_config_set_parsed_file_uri_does_not_reencode_path() {
+        let mut cfg = Config::default();
+        let uri = libdd_common::parse_uri("file:///absolute/path").unwrap();
+
+        cfg.set_endpoint_uri(uri).unwrap();
+
+        let endpoint = cfg.endpoint().unwrap();
+        assert_eq!(
+            Path::new("/absolute/path"),
+            libdd_common::decode_uri_path_in_authority(&endpoint.url).unwrap()
+        );
+    }
+
     #[test]
     #[cfg(unix)]
     fn test_config_set_url_unix_socket() {