diff --git a/object_store/src/aws/credential.rs b/object_store/src/aws/credential.rs index c13f8aafc7f3..63cb57160ce2 100644 --- a/object_store/src/aws/credential.rs +++ b/object_store/src/aws/credential.rs @@ -610,6 +610,7 @@ async fn web_identity( ]) .retryable(retry_config) .idempotent(true) + .sensitive(true) .send() .await? .bytes() diff --git a/object_store/src/client/retry.rs b/object_store/src/client/retry.rs index 5dfdd55b6540..5df4ce059c24 100644 --- a/object_store/src/client/retry.rs +++ b/object_store/src/client/retry.rs @@ -174,6 +174,7 @@ pub struct RetryableRequest { retry_timeout: Duration, backoff: Backoff, + sensitive: bool, idempotent: Option, payload: Option, } @@ -190,6 +191,14 @@ impl RetryableRequest { } } + /// Set whether this request contains sensitive data + /// + /// This will avoid printing out the URL in error messages + #[allow(unused)] + pub fn sensitive(self, sensitive: bool) -> Self { + Self { sensitive, ..self } + } + /// Provide a [`PutPayload`] pub fn payload(self, payload: Option) -> Self { Self { payload, ..self } @@ -206,6 +215,11 @@ impl RetryableRequest { .idempotent .unwrap_or_else(|| self.request.method().is_safe()); + let sanitize_err = move |e: reqwest::Error| match self.sensitive { + true => e.without_url(), + false => e, + }; + loop { let mut request = self .request @@ -238,6 +252,7 @@ impl RetryableRequest { }; } Err(e) => { + let e = sanitize_err(e); let status = r.status(); if retries == max_retries || now.elapsed() > retry_timeout @@ -280,6 +295,8 @@ impl RetryableRequest { } }, Err(e) => { + let e = sanitize_err(e); + let mut do_retry = false; if e.is_connect() || e.is_body() @@ -365,6 +382,7 @@ impl RetryExt for reqwest::RequestBuilder { backoff: Backoff::new(&config.backoff), idempotent: None, payload: None, + sensitive: false, } } @@ -565,6 +583,48 @@ mod tests { "{e}" ); + let url = format!("{}/SENSITIVE", mock.url()); + for _ in 0..=retry.max_retries { + mock.push( + Response::builder() + .status(StatusCode::BAD_GATEWAY) + .body("ignored".to_string()) + .unwrap(), + ); + } + let res = client.request(Method::GET, url).send_retry(&retry).await; + let err = res.unwrap_err().to_string(); + assert!(err.contains("SENSITIVE"), "{err}"); + + let url = format!("{}/SENSITIVE", mock.url()); + for _ in 0..=retry.max_retries { + mock.push( + Response::builder() + .status(StatusCode::BAD_GATEWAY) + .body("ignored".to_string()) + .unwrap(), + ); + } + + // Sensitive requests should strip URL from error + let req = client + .request(Method::GET, &url) + .retryable(&retry) + .sensitive(true); + let err = req.send().await.unwrap_err().to_string(); + assert!(!err.contains("SENSITIVE"), "{err}"); + + for _ in 0..=retry.max_retries { + mock.push_fn(|_| panic!()); + } + + let req = client + .request(Method::GET, &url) + .retryable(&retry) + .sensitive(true); + let err = req.send().await.unwrap_err().to_string(); + assert!(!err.contains("SENSITIVE"), "{err}"); + // Shutdown mock.shutdown().await }