Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions opentelemetry-exporter-geneva/geneva-uploader/src/client.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! High-level GenevaClient for user code. Wraps config_service and ingestion_service.

use crate::common::{build_geneva_headers, validate_user_agent_prefix};
use crate::config_service::client::{AuthMethod, GenevaConfigClient, GenevaConfigClientConfig};
use crate::ingestion_service::uploader::{GenevaUploader, GenevaUploaderConfig};
use crate::payload_encoder::lz4_chunked_compression::lz4_chunked_compression;
Expand All @@ -23,6 +24,17 @@ pub struct GenevaClientConfig {
pub role_instance: String,
/// Maximum number of concurrent uploads. If None, defaults to number of CPU cores.
pub max_concurrent_uploads: Option<usize>,
/// User agent prefix for the application. Will be formatted as "<prefix> (GenevaUploader/0.1)".
/// If None, defaults to "GenevaUploader/0.1".
///
/// The prefix must contain only ASCII printable characters, be non-empty (after trimming),
/// and not exceed 200 characters in length.
///
/// Examples:
/// - None: "GenevaUploader/0.1"
/// - Some("MyApp/2.1.0"): "MyApp/2.1.0 (GenevaUploader/0.1)"
/// - Some("ProductionService-1.0"): "ProductionService-1.0 (GenevaUploader/0.1)"
pub user_agent_prefix: Option<&'static str>,
// Add event name/version here if constant, or per-upload if you want them per call.
}

Expand All @@ -38,7 +50,17 @@ pub struct GenevaClient {
impl GenevaClient {
/// Construct a new client with minimal configuration. Fetches and caches ingestion info as needed.
pub async fn new(cfg: GenevaClientConfig) -> Result<Self, String> {
// Build config client config
// Validate user agent prefix once and build headers once for both services
// This avoids duplicate validation and header building in config and ingestion services
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a duplicate validation since build_geneva_headers method also calls validate_user_agent_prefix method.

if let Some(prefix) = cfg.user_agent_prefix {
validate_user_agent_prefix(prefix)
.map_err(|e| format!("Invalid user agent prefix: {e}"))?;
}

let static_headers = build_geneva_headers(cfg.user_agent_prefix)
.map_err(|e| format!("Failed to build Geneva headers: {e}"))?;

// Build config client config with pre-built headers
let config_client_config = GenevaConfigClientConfig {
endpoint: cfg.endpoint,
environment: cfg.environment.clone(),
Expand All @@ -47,6 +69,7 @@ impl GenevaClient {
region: cfg.region,
config_major_version: cfg.config_major_version,
auth_method: cfg.auth_method,
static_headers: static_headers.clone(),
};
let config_client = Arc::new(
GenevaConfigClient::new(config_client_config)
Expand All @@ -67,12 +90,13 @@ impl GenevaClient {
cfg.namespace, config_version, cfg.tenant, cfg.role_name, cfg.role_instance,
);

// Uploader config
// Uploader config with pre-built headers
let uploader_config = GenevaUploaderConfig {
namespace: cfg.namespace.clone(),
source_identity,
environment: cfg.environment,
config_version: config_version.clone(),
static_headers: static_headers.clone(),
};

let uploader = GenevaUploader::from_config_client(config_client, uploader_config)
Expand Down
235 changes: 235 additions & 0 deletions opentelemetry-exporter-geneva/geneva-uploader/src/common.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
//! Common utilities and validation functions shared across the Geneva uploader crate.
use reqwest::header::{HeaderMap, HeaderValue, ACCEPT, USER_AGENT};
use thiserror::Error;

/// Common validation errors
#[derive(Debug, Error)]
pub(crate) enum ValidationError {
#[error("Invalid user agent prefix: {0}")]
InvalidUserAgentPrefix(String),
}

pub(crate) type Result<T> = std::result::Result<T, ValidationError>;

// Validates a user agent prefix for HTTP header compliance
// Validation Rules:
// - Must contain only ASCII printable characters (0x20-0x7E)
// - Must not contain control characters (especially \r, \n, \0)
// - Must not exceed 200 characters in length
// - Must not be empty or only whitespace
pub(crate) fn validate_user_agent_prefix(prefix: &str) -> Result<()> {
if prefix.trim().is_empty() {
return Err(ValidationError::InvalidUserAgentPrefix(
"User agent prefix cannot be empty or only whitespace".to_string(),
));
}

if prefix.len() > 200 {
return Err(ValidationError::InvalidUserAgentPrefix(format!(
"User agent prefix too long: {len} characters (max 200)",
len = prefix.len()
)));
}

// Check for invalid characters
for (i, ch) in prefix.char_indices() {
match ch {
// Control characters that would break HTTP headers
'\r' | '\n' | '\0' => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think had initially suggested checking for control characters in a comment, but I realized they would anyway be outside of the ASCII range so we would never allow control characters as long as we only accept ASCII.

And the ASCII check is already done by HeaderValue::from_str() method, so we can probably get rid of this whole method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I am good to remove the validation altogether, and let the reqwest crate handle it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems HeaderValue::from_str() allows non-ascii chars https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=1e264800416082c69cbb17c474caab02

This led to some new learnings. The documentation for HeaderValue::from_str() method says that it doesn't allow non-ASCII characters so I would have expected the test cases you shared to fail. I then found this is in the documentation for HeaderValue struct:

In practice, HTTP header field values are usually valid ASCII. However, the HTTP spec allows for a header value to contain opaque bytes as well. In this case, the header field value is not able to be represented as a string.

To handle this, the HeaderValue is useable as a type and can be compared with strings and implements Debug. A to_str fn is provided that returns an Err if the header value contains non visible ascii characters.

This means that we need to check if HeaderValue::from_str(str).is_ok() && HeaderValue::from_str(str).unwrap().to_str().is_ok() to rule out the non-ASCII inputs.

Check this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=9802b30e262c93e92e71fd92fcb543dd

return Err(ValidationError::InvalidUserAgentPrefix(format!(
"Invalid control character at position {i}: {ch:?}"
)));
}
// Non-ASCII or non-printable characters
ch if !ch.is_ascii() || (ch as u8) < 0x20 || (ch as u8) > 0x7E => {
return Err(ValidationError::InvalidUserAgentPrefix(format!(
"Invalid character at position {i}: {ch:?} (must be ASCII printable)"
)));
}
_ => {} // Valid character
}
}

Ok(())
}

// Builds a standardized User-Agent header for Geneva services
// TODO: Update the user agent format based on whether custom config will come first or later
// Current format:
// - If prefix is None or empty: "GenevaUploader/0.1"
// - If prefix is provided: "{prefix} (GenevaUploader/0.1)"
pub(crate) fn build_user_agent_header(user_agent_prefix: Option<&str>) -> Result<HeaderValue> {
let prefix = user_agent_prefix.unwrap_or("");

// Validate the prefix if provided
if !prefix.is_empty() {
validate_user_agent_prefix(prefix)?;
}

let user_agent = if prefix.is_empty() {
"GenevaUploader/0.1".to_string()
} else {
format!("{prefix} (GenevaUploader/0.1)")
};

HeaderValue::from_str(&user_agent).map_err(|e| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this method does the ASCII characters validation check anyway so we could get avoid checking that ourselves.

ValidationError::InvalidUserAgentPrefix(format!("Failed to create User-Agent header: {e}"))
})
}

// Builds a complete set of HTTP headers for Geneva services
// Returns HTTP headers including User-Agent and Accept
pub(crate) fn build_geneva_headers(user_agent_prefix: Option<&str>) -> Result<HeaderMap> {
let mut headers = HeaderMap::new();

let user_agent = build_user_agent_header(user_agent_prefix)?;
headers.insert(USER_AGENT, user_agent);
headers.insert(ACCEPT, HeaderValue::from_static("application/json"));

Ok(headers)
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_validate_user_agent_prefix_valid() {
assert!(validate_user_agent_prefix("MyApp/1.0").is_ok());
assert!(validate_user_agent_prefix("Production-Service-2.1.0").is_ok());
assert!(validate_user_agent_prefix("TestApp_v3").is_ok());
assert!(validate_user_agent_prefix("App-Name.1.2.3").is_ok());
assert!(validate_user_agent_prefix("Simple123").is_ok());
}

#[test]
fn test_validate_user_agent_prefix_empty() {
assert!(validate_user_agent_prefix("").is_err());
assert!(validate_user_agent_prefix(" ").is_err());
assert!(validate_user_agent_prefix("\t\n").is_err());

if let Err(e) = validate_user_agent_prefix("") {
assert!(e.to_string().contains("cannot be empty"));
}
}

#[test]
fn test_validate_user_agent_prefix_too_long() {
let long_prefix = "a".repeat(201);
let result = validate_user_agent_prefix(&long_prefix);
assert!(result.is_err());

if let Err(e) = result {
assert!(e.to_string().contains("too long"));
assert!(e.to_string().contains("201 characters"));
}

// Test exactly at the limit should be OK
let max_length_prefix = "a".repeat(200);
assert!(validate_user_agent_prefix(&max_length_prefix).is_ok());
}

#[test]
fn test_validate_user_agent_prefix_invalid_chars() {
// Test control characters
assert!(validate_user_agent_prefix("App\nName").is_err());
assert!(validate_user_agent_prefix("App\rName").is_err());
assert!(validate_user_agent_prefix("App\0Name").is_err());
assert!(validate_user_agent_prefix("App\tName").is_err());

// Test non-ASCII characters
assert!(validate_user_agent_prefix("App🚀Name").is_err());
assert!(validate_user_agent_prefix("Appé").is_err());
assert!(validate_user_agent_prefix("App中文").is_err());

// Test non-printable ASCII - construct strings with actual control characters
let unit_separator = format!("App{}Name", '\u{001F}');
let del_char = format!("App{}Name", '\u{007F}');
assert!(validate_user_agent_prefix(&unit_separator).is_err()); // Unit separator (0x1F)
assert!(validate_user_agent_prefix(&del_char).is_err()); // DEL character (0x7F)

// Verify error messages contain position information
if let Err(e) = validate_user_agent_prefix("App\nName") {
assert!(e.to_string().contains("position 3"));
assert!(e.to_string().contains("control character"));
}
}

#[test]
fn test_character_validation_edge_cases() {
// Test ASCII printable range boundaries
assert!(validate_user_agent_prefix(" ").is_err()); // Space only should be trimmed to empty
assert!(validate_user_agent_prefix("App Space").is_ok()); // Space in middle is OK
assert!(validate_user_agent_prefix("~").is_ok()); // Last printable ASCII (0x7E)
assert!(validate_user_agent_prefix("!").is_ok()); // First printable ASCII after space (0x21)

// Test that spaces at the beginning and end are allowed (they're ASCII printable)
assert!(validate_user_agent_prefix(" ValidApp ").is_ok()); // Leading/trailing spaces are valid ASCII printable chars
// But strings that trim to empty should fail
assert!(validate_user_agent_prefix(" ").is_err()); // Only spaces should fail
}

#[test]
fn test_build_user_agent_header_without_prefix() {
let header = build_user_agent_header(None).unwrap();
assert_eq!(header.to_str().unwrap(), "GenevaUploader/0.1");
}

#[test]
fn test_build_user_agent_header_with_empty_prefix() {
let header = build_user_agent_header(Some("")).unwrap();
assert_eq!(header.to_str().unwrap(), "GenevaUploader/0.1");
}

#[test]
fn test_build_user_agent_header_with_valid_prefix() {
let header = build_user_agent_header(Some("MyApp/2.1.0")).unwrap();
assert_eq!(header.to_str().unwrap(), "MyApp/2.1.0 (GenevaUploader/0.1)");
}

#[test]
fn test_build_user_agent_header_with_invalid_prefix() {
let result = build_user_agent_header(Some("Invalid\nPrefix"));
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("Invalid user agent prefix"));
}

#[test]
fn test_build_geneva_headers_complete() {
let headers = build_geneva_headers(Some("TestApp/1.0")).unwrap();

let user_agent = headers.get(USER_AGENT).unwrap();
assert_eq!(
user_agent.to_str().unwrap(),
"TestApp/1.0 (GenevaUploader/0.1)"
);

let accept = headers.get(ACCEPT).unwrap();
assert_eq!(accept.to_str().unwrap(), "application/json");
}

#[test]
fn test_build_geneva_headers_without_prefix() {
let headers = build_geneva_headers(None).unwrap();

let user_agent = headers.get(USER_AGENT).unwrap();
assert_eq!(user_agent.to_str().unwrap(), "GenevaUploader/0.1");

let accept = headers.get(ACCEPT).unwrap();
assert_eq!(accept.to_str().unwrap(), "application/json");
}

#[test]
fn test_build_geneva_headers_with_invalid_prefix() {
let result = build_geneva_headers(Some("Invalid\rPrefix"));
assert!(result.is_err());
assert!(result
.unwrap_err()
.to_string()
.contains("Invalid user agent prefix"));
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
// Geneva Config Client with TLS (PKCS#12) and TODO: Managed Identity support

use base64::{engine::general_purpose, Engine as _};
use reqwest::{
header::{HeaderMap, HeaderValue, ACCEPT, USER_AGENT},
Client,
};
use reqwest::{header::HeaderMap, Client};
use serde::Deserialize;
use std::time::Duration;
use thiserror::Error;
Expand Down Expand Up @@ -128,7 +125,8 @@ pub(crate) struct GenevaConfigClientConfig {
pub(crate) namespace: String,
pub(crate) region: String,
pub(crate) config_major_version: u32,
pub(crate) auth_method: AuthMethod, // agent_identity and agent_version are hardcoded for now
pub(crate) auth_method: AuthMethod,
pub(crate) static_headers: HeaderMap,
}

#[allow(dead_code)]
Expand Down Expand Up @@ -260,9 +258,6 @@ impl GenevaConfigClient {
}

let agent_identity = "GenevaUploader";
let agent_version = "0.1";
let static_headers = Self::build_static_headers(agent_identity, agent_version);

let identity = format!("Tenant=Default/Role=GcsClient/RoleInstance={agent_identity}");

let encoded_identity = general_purpose::STANDARD.encode(&identity);
Expand All @@ -283,15 +278,16 @@ impl GenevaConfigClient {
).map_err(|e| GenevaConfigClientError::InternalError(format!("Failed to write URL: {e}")))?;

let http_client = client_builder.build()?;
let static_headers = config.static_headers.clone();

Ok(Self {
static_headers,
config,
http_client,
cached_data: RwLock::new(None),
precomputed_url_prefix: pre_url,
agent_identity: agent_identity.to_string(), // TODO make this configurable
agent_version: "1.0".to_string(), // TODO make this configurable
static_headers,
})
}

Expand All @@ -302,14 +298,6 @@ impl GenevaConfigClient {
.map(|dt| dt.with_timezone(&Utc))
}

fn build_static_headers(agent_identity: &str, agent_version: &str) -> HeaderMap {
let mut headers = HeaderMap::new();
let user_agent = format!("{agent_identity}-{agent_version}");
headers.insert(USER_AGENT, HeaderValue::from_str(&user_agent).unwrap());
headers.insert(ACCEPT, HeaderValue::from_static("application/json"));
headers
}

/// Retrieves ingestion gateway information from the Geneva Config Service.
///
/// # HTTP API Details
Expand Down
Loading
Loading