net: include the error's cause chain in network error messages
reqwest and wreq only say "error sending request for url" at the top level; the actual cause (expired certificate, wrong host, handshake failure, DNS failure, refused connection) is in the error's source() chain, which the default client dropped and the stealth client printed as a Debug dump of the internal structs, including the BoringSSL build path. Add error_chain(), which appends each source's message once, and use it for the send and body-read errors of both clients.
This commit is contained in:
@@ -824,7 +824,7 @@ async fn read_reqwest_body_limited(
|
||||
.min(limit);
|
||||
let mut body = Vec::with_capacity(capacity);
|
||||
while let Some(chunk) = response.chunk().await.map_err(|error| {
|
||||
ObscuraNetError::Network(format!("Failed to read body: {}", error))
|
||||
ObscuraNetError::Network(format!("Failed to read body: {}", error_chain(&error)))
|
||||
})? {
|
||||
if chunk.len() > limit.saturating_sub(body.len()) {
|
||||
return Err(response_too_large(url, limit));
|
||||
@@ -1575,7 +1575,7 @@ impl ObscuraHttpClient {
|
||||
|
||||
let in_flight = InFlightGuard::new(&self.in_flight);
|
||||
let resp = req_builder.send().await.map_err(|e| {
|
||||
ObscuraNetError::Network(format!("{}: {}", current_url, e))
|
||||
ObscuraNetError::Network(format!("{}: {}", current_url, error_chain(&e)))
|
||||
})?;
|
||||
|
||||
let status = resp.status();
|
||||
@@ -1681,6 +1681,25 @@ impl Default for ObscuraHttpClient {
|
||||
}
|
||||
}
|
||||
|
||||
/// `error` followed by the message of each error in its `source()` chain, joined
|
||||
/// with ": ". reqwest and wreq only say "error sending request" at the top level;
|
||||
/// the cause (expired certificate, handshake failure, refused connection, DNS)
|
||||
/// is further down the chain.
|
||||
pub(crate) fn error_chain(error: &dyn std::error::Error) -> String {
|
||||
let mut text = error.to_string();
|
||||
let mut source = error.source();
|
||||
while let Some(cause) = source {
|
||||
let cause_text = cause.to_string();
|
||||
// Some layers already include their source's message in their own.
|
||||
if !text.contains(&cause_text) {
|
||||
text.push_str(": ");
|
||||
text.push_str(&cause_text);
|
||||
}
|
||||
source = cause.source();
|
||||
}
|
||||
text
|
||||
}
|
||||
|
||||
#[derive(Debug, thiserror::Error)]
|
||||
pub enum ObscuraNetError {
|
||||
#[error("Network error: {0}")]
|
||||
@@ -2826,7 +2845,38 @@ mod ssrf_tests {
|
||||
let client =
|
||||
ObscuraHttpClient::with_full_options(Arc::new(CookieJar::new()), None, true);
|
||||
let url = Url::parse(&format!("https://127.0.0.1:{port}/")).unwrap();
|
||||
assert!(client.fetch(&url).await.is_err(), "unknown CA must be rejected");
|
||||
let error = client.fetch(&url).await.expect_err("unknown CA must be rejected");
|
||||
// The message must carry the TLS cause, not only "error sending request".
|
||||
let message = error.to_string();
|
||||
assert!(message.contains("invalid peer certificate"), "{message}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_chain_appends_each_source_once() {
|
||||
use std::fmt;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct Layer(&'static str, Option<Box<Layer>>);
|
||||
impl fmt::Display for Layer {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(self.0)
|
||||
}
|
||||
}
|
||||
impl std::error::Error for Layer {
|
||||
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
|
||||
self.1.as_deref().map(|e| e as _)
|
||||
}
|
||||
}
|
||||
|
||||
let leaf = Layer("certificate expired", None);
|
||||
// The middle layer already quotes the leaf, so the leaf is not repeated.
|
||||
let middle = Layer("tls: certificate expired", Some(Box::new(leaf)));
|
||||
let top = Layer("error sending request", Some(Box::new(middle)));
|
||||
assert_eq!(
|
||||
super::error_chain(&top),
|
||||
"error sending request: tls: certificate expired"
|
||||
);
|
||||
assert_eq!(super::error_chain(&Layer("alone", None)), "alone");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -16,7 +16,7 @@ use url::Url;
|
||||
|
||||
#[cfg(feature = "stealth")]
|
||||
use crate::client::{
|
||||
cors_required, env_allows_private_network, fetch_file_url, is_forbidden_ip,
|
||||
cors_required, env_allows_private_network, error_chain, fetch_file_url, is_forbidden_ip,
|
||||
redirect_taints_origin, request_fetch_site, request_referrer, response_too_large,
|
||||
same_site_context, serialized_request_origin, validate_cors_response, validate_request_mode,
|
||||
validate_url, CallbackRegistry, InFlightGuard, ObscuraNetError, RequestInfo, RequestMode,
|
||||
@@ -155,7 +155,7 @@ async fn read_wreq_body_limited(
|
||||
let mut body = Vec::with_capacity(capacity);
|
||||
while let Some(chunk) = stream.next().await {
|
||||
let chunk = chunk.map_err(|error| {
|
||||
ObscuraNetError::Network(format!("Failed to read body: {}", error))
|
||||
ObscuraNetError::Network(format!("Failed to read body: {}", error_chain(&error)))
|
||||
})?;
|
||||
if chunk.len() > limit.saturating_sub(body.len()) {
|
||||
return Err(response_too_large(url, limit));
|
||||
@@ -404,12 +404,7 @@ impl StealthHttpClient {
|
||||
let resp = send_get_with_connection_reset_retry(req, ¤t_url)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
ObscuraNetError::Network(format!(
|
||||
"{}: {} (source: {:?})",
|
||||
current_url,
|
||||
e,
|
||||
e.source()
|
||||
))
|
||||
ObscuraNetError::Network(format!("{}: {}", current_url, error_chain(&e)))
|
||||
})?;
|
||||
|
||||
let status = resp.status();
|
||||
@@ -561,7 +556,7 @@ impl StealthHttpClient {
|
||||
|
||||
let in_flight = InFlightGuard::new(&self.in_flight);
|
||||
let resp = req.send().await.map_err(|e| {
|
||||
ObscuraNetError::Network(format!("{}: {}", url, e))
|
||||
ObscuraNetError::Network(format!("{}: {}", url, error_chain(&e)))
|
||||
})?;
|
||||
|
||||
let status = resp.status();
|
||||
@@ -871,6 +866,23 @@ mod tests {
|
||||
assert_eq!(resp.text(), "folded");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn stealth_network_error_names_the_cause() {
|
||||
// A port nobody listens on: the message must say the connection was
|
||||
// refused instead of only "error sending request", and must not be a
|
||||
// Debug dump of the error structs.
|
||||
let port = std::net::TcpListener::bind("127.0.0.1:0")
|
||||
.unwrap()
|
||||
.local_addr()
|
||||
.unwrap()
|
||||
.port();
|
||||
let client = StealthHttpClient::with_proxy(Arc::new(CookieJar::new()), None, true);
|
||||
let url = Url::parse(&format!("http://127.0.0.1:{port}/")).unwrap();
|
||||
let message = client.fetch(&url).await.expect_err("nothing listens").to_string();
|
||||
assert!(message.to_lowercase().contains("refused"), "{message}");
|
||||
assert!(!message.contains("source: Some("), "{message}");
|
||||
}
|
||||
|
||||
/// Serve one `Content-Encoding: gzip` response on an ephemeral port.
|
||||
async fn gzip_fixture() -> u16 {
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
|
||||
|
||||
Reference in New Issue
Block a user