diff --git a/bin_tests/tests/crashtracker_bin_test.rs b/bin_tests/tests/crashtracker_bin_test.rs index 491d082c3..fbd9c8317 100644 --- a/bin_tests/tests/crashtracker_bin_test.rs +++ b/bin_tests/tests/crashtracker_bin_test.rs @@ -226,12 +226,13 @@ fn setup_test_fixtures<'a>(crates: &[&'a ArtifactsBuild]) -> TestFixtures<'a> { let artifacts = build_artifacts(crates).unwrap(); let tmpdir = tempfile::TempDir::new().unwrap(); + let dirpath = tmpdir.path(); TestFixtures { - crash_profile_path: extend_path(tmpdir.path(), "crash"), - crash_telemetry_path: extend_path(tmpdir.path(), "crash.telemetry"), - stdout_path: extend_path(tmpdir.path(), "out.stdout"), - stderr_path: extend_path(tmpdir.path(), "out.stderr"), - unix_socket_path: extend_path(tmpdir.path(), "crashtracker.socket"), + crash_profile_path: extend_path(dirpath, "crash"), + crash_telemetry_path: extend_path(dirpath, "crash.telemetry"), + stdout_path: extend_path(dirpath, "out.stdout"), + stderr_path: extend_path(dirpath, "out.stderr"), + unix_socket_path: extend_path(dirpath, "crashtracker.socket"), artifacts, tmpdir, diff --git a/crashtracker/src/crash_info.rs b/crashtracker/src/crash_info.rs index 92ce76f50..fa28a7b43 100644 --- a/crashtracker/src/crash_info.rs +++ b/crashtracker/src/crash_info.rs @@ -9,6 +9,7 @@ use chrono::{DateTime, Utc}; use ddcommon::tag::Tag; use serde::{Deserialize, Serialize}; use std::io::BufRead; +use std::path::Path; use std::{collections::HashMap, fs::File, io::BufReader}; use uuid::Uuid; @@ -241,10 +242,11 @@ impl CrashInfo { /// Emit the CrashInfo as structured json in file `path`. /// SIGNAL SAFETY: /// I believe but have not verified this is signal safe. - pub fn to_file(&self, path: &str) -> anyhow::Result<()> { - let file = File::create(path).with_context(|| format!("Failed to create {path}"))?; + pub fn to_file(&self, path: &Path) -> anyhow::Result<()> { + let file = + File::create(path).with_context(|| format!("Failed to create {}", path.display()))?; serde_json::to_writer_pretty(file, self) - .with_context(|| format!("Failed to write json to {path}"))?; + .with_context(|| format!("Failed to write json to {}", path.display()))?; Ok(()) } @@ -252,13 +254,9 @@ impl CrashInfo { // If we're debugging to a file, dump the actual crashinfo into a json if let Some(endpoint) = &config.endpoint { if Some("file") == endpoint.url.scheme_str() { - self.to_file( - endpoint - .url - .path_and_query() - .ok_or_else(|| anyhow::format_err!("empty path for upload to file"))? - .as_str(), - )?; + let path = ddcommon::decode_uri_path_in_authority(&endpoint.url) + .context("crash output file was not correctly formatted")?; + self.to_file(&path)?; } } self.upload_to_telemetry(config) diff --git a/crashtracker/src/telemetry.rs b/crashtracker/src/telemetry.rs index f791b4440..31d998bbf 100644 --- a/crashtracker/src/telemetry.rs +++ b/crashtracker/src/telemetry.rs @@ -6,7 +6,7 @@ use std::fmt::Write; use std::time::{self, SystemTime}; use super::{CrashInfo, CrashtrackerConfiguration, CrashtrackerMetadata, StackFrame}; -use anyhow::Ok; +use anyhow::{Context, Ok}; use ddtelemetry::{ build_host, data::{self, Application, LogLevel}, @@ -70,7 +70,9 @@ impl TelemetryCrashUploader { // ignore result because what are we going to do? let _ = if endpoint.url.scheme_str() == Some("file") { - cfg.set_host_from_url(&format!("file://{}.telemetry", endpoint.url.path())) + let path = ddcommon::decode_uri_path_in_authority(&endpoint.url) + .context("file path is not valid")?; + cfg.set_host_from_url(&format!("file://{}.telemetry", path.display())) } else { cfg.set_endpoint(endpoint.clone()) }; diff --git a/ddcommon-ffi/src/endpoint.rs b/ddcommon-ffi/src/endpoint.rs index 13955a9d8..c6a15375d 100644 --- a/ddcommon-ffi/src/endpoint.rs +++ b/ddcommon-ffi/src/endpoint.rs @@ -51,3 +51,28 @@ pub extern "C" fn ddog_endpoint_from_api_key_and_site( #[no_mangle] pub extern "C" fn ddog_endpoint_drop(_: Box) {} + +#[cfg(test)] +mod tests { + use crate::CharSlice; + + use super::ddog_endpoint_from_url; + + #[test] + fn test_ddog_endpoint_from_url() { + let cases = [ + ("", false), + ("http:// /hey", false), + ("file://", false), + ("http://localhost:8383/hello", true), + ("file:/// file / with/weird chars 🤡", true), + ("file://./", true), + ("unix://./", true), + ]; + + for (input, expected) in cases { + let actual = ddog_endpoint_from_url(CharSlice::from(input)).is_some(); + assert_eq!(actual, expected); + } + } +} diff --git a/ddcommon/src/lib.rs b/ddcommon/src/lib.rs index 51b52a30a..14ce9b567 100644 --- a/ddcommon/src/lib.rs +++ b/ddcommon/src/lib.rs @@ -1,7 +1,7 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use std::{borrow::Cow, ops::Deref, str::FromStr}; +use std::{borrow::Cow, ops::Deref, path::PathBuf, str::FromStr}; use hyper::{ header::HeaderValue, @@ -95,17 +95,7 @@ pub fn parse_uri(uri: &str) -> anyhow::Result { } else if let Some(path) = uri.strip_prefix("windows:") { encode_uri_path_in_authority("windows", path) } else if let Some(path) = uri.strip_prefix("file://") { - let mut parts = uri::Parts::default(); - parts.scheme = uri::Scheme::from_str("file").ok(); - parts.authority = Some(uri::Authority::from_static("localhost")); - - // TODO: handle edge cases like improperly escaped url strings - // - // this is eventually user configurable field - // anything we can do to ensure invalid input becomes valid - will improve usability - parts.path_and_query = uri::PathAndQuery::from_str(path).ok(); - - Ok(hyper::Uri::from_parts(parts)?) + encode_uri_path_in_authority("file", path) } else { Ok(hyper::Uri::from_str(uri)?) } @@ -122,6 +112,26 @@ fn encode_uri_path_in_authority(scheme: &str, path: &str) -> anyhow::Result anyhow::Result { + let path = hex::decode( + uri.authority() + .ok_or_else(|| anyhow::anyhow!("missing uri authority"))? + .as_str(), + )?; + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + Ok(PathBuf::from(std::ffi::OsString::from_vec(path))) + } + #[cfg(not(unix))] + { + match String::from_utf8(path) { + Ok(s) => Ok(PathBuf::from(s.as_str())), + _ => Err(anyhow::anyhow!("file uri should be utf-8")), + } + } +} + impl Endpoint { /// Return a request builder with the following headers: /// - User agent diff --git a/ddtelemetry/src/config.rs b/ddtelemetry/src/config.rs index 9748aa0fb..984120f46 100644 --- a/ddtelemetry/src/config.rs +++ b/ddtelemetry/src/config.rs @@ -267,6 +267,8 @@ impl Config { #[cfg(test)] mod tests { + use std::path::Path; + #[cfg(unix)] use ddcommon::connector::uds; @@ -388,6 +390,10 @@ mod tests { ("file:///absolute/path", "/absolute/path"), ("file://./relative/path", "./relative/path"), ("file://relative/path", "relative/path"), + ( + "file://c://temp//with space\\foo.json", + "c://temp//with space\\foo.json", + ), ]; for (input, expected) in cases { @@ -405,15 +411,8 @@ mod tests { .to_string() ); assert_eq!( - expected, - cfg.clone() - .endpoint - .unwrap() - .url - .into_parts() - .path_and_query - .unwrap() - .as_str() + Path::new(expected), + ddcommon::decode_uri_path_in_authority(&cfg.endpoint.unwrap().url).unwrap(), ); } } diff --git a/ddtelemetry/src/worker/http_client.rs b/ddtelemetry/src/worker/http_client.rs index 0e9024cef..0e353294a 100644 --- a/ddtelemetry/src/worker/http_client.rs +++ b/ddtelemetry/src/worker/http_client.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use ddcommon::HttpRequestBuilder; -use http::uri::Parts; use http::{Request, Response}; use hyper::Body; use std::{ @@ -41,21 +40,18 @@ pub fn request_builder(c: &Config) -> anyhow::Result { } pub fn from_config(c: &Config) -> Box { - if let Some(Parts { - scheme: Some(scheme), - path_and_query: Some(path), - .. - }) = c.endpoint.as_ref().map(|e| e.url.clone().into_parts()) - { - if scheme.as_str() == "file" { + match &c.endpoint { + Some(e) if e.url.scheme_str() == Some("file") => { + let file_path = ddcommon::decode_uri_path_in_authority(&e.url) + .expect("file urls should always have been encoded in authority"); return Box::new(MockClient { file: Arc::new(Mutex::new(Box::new( - File::create(path.path()).expect("Couldn't open mock client file"), + File::create(file_path).expect("Couldn't open mock client file"), ))), }); } - } - + Some(_) | None => {} + }; Box::new(HyperClient { inner: hyper::Client::builder() .pool_idle_timeout(std::time::Duration::from_secs(30))