fix: return 502 on upstream URI parse failure instead of dropping query string
Change build_upstream_uri to return Result<Uri, ()> so that URI parse failures are properly handled instead of silently dropping the query string and unwrapping a fallback. On parse failure, log a warning with the malformed URI and return 502 Bad Gateway to the client.
This commit is contained in:
@@ -62,7 +62,23 @@ async fn proxy_handler(
|
|||||||
let upstream_scheme = site.upstream_scheme.clone();
|
let upstream_scheme = site.upstream_scheme.clone();
|
||||||
let upstream = site.upstream.clone();
|
let upstream = site.upstream.clone();
|
||||||
let upstream_addr = format!("{}://{}", upstream_scheme, upstream);
|
let upstream_addr = format!("{}://{}", upstream_scheme, upstream);
|
||||||
let upstream_uri = build_upstream_uri(&upstream_scheme, &upstream, req.uri());
|
let upstream_uri = match build_upstream_uri(&upstream_scheme, &upstream, req.uri()) {
|
||||||
|
Ok(uri) => uri,
|
||||||
|
Err(()) => {
|
||||||
|
log_upstream_error!(&host_owned, &upstream_addr, "malformed upstream URI");
|
||||||
|
let duration_ms = start.elapsed().as_millis() as u64;
|
||||||
|
log_request!(
|
||||||
|
&client_ip,
|
||||||
|
&host_owned,
|
||||||
|
&method,
|
||||||
|
&path,
|
||||||
|
502u16,
|
||||||
|
&upstream,
|
||||||
|
duration_ms
|
||||||
|
);
|
||||||
|
return StatusCode::BAD_GATEWAY.into_response();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
let upstream_req = match build_upstream_request(req, &upstream_uri) {
|
let upstream_req = match build_upstream_request(req, &upstream_uri) {
|
||||||
Ok(r) => r,
|
Ok(r) => r,
|
||||||
@@ -187,17 +203,15 @@ async fn proxy_handler(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn build_upstream_uri(scheme: &str, upstream: &str, original_uri: &Uri) -> Uri {
|
fn build_upstream_uri(scheme: &str, upstream: &str, original_uri: &Uri) -> Result<Uri, ()> {
|
||||||
let path = original_uri.path();
|
let path = original_uri.path();
|
||||||
let query = original_uri
|
let query = original_uri
|
||||||
.query()
|
.query()
|
||||||
.map(|q| format!("?{}", q))
|
.map(|q| format!("?{}", q))
|
||||||
.unwrap_or_default();
|
.unwrap_or_default();
|
||||||
let uri_string = format!("{}://{}{}{}", scheme, upstream, path, query);
|
let uri_string = format!("{}://{}{}{}", scheme, upstream, path, query);
|
||||||
uri_string.parse::<Uri>().unwrap_or_else(|_| {
|
uri_string.parse::<Uri>().map_err(|e| {
|
||||||
format!("{}://{}{}", scheme, upstream, path)
|
warn!(error = %e, uri = %uri_string, "failed to parse upstream URI");
|
||||||
.parse::<Uri>()
|
|
||||||
.unwrap()
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -346,21 +360,21 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn test_build_upstream_uri_with_query() {
|
fn test_build_upstream_uri_with_query() {
|
||||||
let uri: Uri = "/path?foo=bar".parse().unwrap();
|
let uri: Uri = "/path?foo=bar".parse().unwrap();
|
||||||
let result = build_upstream_uri("http", "127.0.0.1:8080", &uri);
|
let result = build_upstream_uri("http", "127.0.0.1:8080", &uri).unwrap();
|
||||||
assert_eq!(result.to_string(), "http://127.0.0.1:8080/path?foo=bar");
|
assert_eq!(result.to_string(), "http://127.0.0.1:8080/path?foo=bar");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_build_upstream_uri_without_query() {
|
fn test_build_upstream_uri_without_query() {
|
||||||
let uri: Uri = "/path".parse().unwrap();
|
let uri: Uri = "/path".parse().unwrap();
|
||||||
let result = build_upstream_uri("http", "127.0.0.1:8080", &uri);
|
let result = build_upstream_uri("http", "127.0.0.1:8080", &uri).unwrap();
|
||||||
assert_eq!(result.to_string(), "http://127.0.0.1:8080/path");
|
assert_eq!(result.to_string(), "http://127.0.0.1:8080/path");
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_build_upstream_uri_https() {
|
fn test_build_upstream_uri_https() {
|
||||||
let uri: Uri = "/secure".parse().unwrap();
|
let uri: Uri = "/secure".parse().unwrap();
|
||||||
let result = build_upstream_uri("https", "upstream.example.com", &uri);
|
let result = build_upstream_uri("https", "upstream.example.com", &uri).unwrap();
|
||||||
assert_eq!(result.to_string(), "https://upstream.example.com/secure");
|
assert_eq!(result.to_string(), "https://upstream.example.com/secure");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user