(Caveat emptor: This report was drafted with LLM assistance, as Rust is not a language I'm very familiar with. I applied enough critical thought that I'm happy to put my name to it, but I invite above-average scrutiny and welcome any feedback!)
Describe the bug
RequestHeader::set_raw_path in pingora-http builds the URI via Uri::builder().path_and_query(target) for every request-target form. As of http 1.4.1, PathAndQuery rejects (correctly, per RFC 9112 §3.2) any input that doesn't begin with / (origin-form) or equal * (asterisk-form). That breaks the other two request-target forms:
- Authority-form (
CONNECT host:port): rejected, so every CONNECT request fails to parse.
- Absolute-form (
GET http://host/path): rejected, so requests carrying a full URL in the request line fail too. (Before 1.4.1 these "worked" but left uri.path() returning the entire URL instead of the path component.)
The impact depends on deployment style. HTTP/1 explicit forward proxies lose every request, since CONNECT and absolute-form are the only two forms they receive. Reverse proxies lose only absolute-form requests, but RFC 9112 §3.2.2 requires all servers to accept absolute-form, so conformant clients that send it now get 400s. HTTP/2 is unaffected (that path builds RequestHeader from http::request::Parts and never calls set_raw_path).
The breakage is lockfile-dependent and silent: pingora-http declares http = "1", so a routine cargo update (or any fresh resolution) introduces the failure with no compile-time signal.
I suspect this is what motivated the 0.8.1 change that gated test_connect_proxying_disallowed_h1 behind the patched_http1 feature ("Authority-form CONNECT request targets require patched HTTP/1 parsing until general request-target form support is available"). If so, the gating was working around this bug: the failure reproduces with stock crates.io dependencies and can be fixed in set_raw_path alone, after which that test passes without the patched parser stack.
Pingora info
Pingora version: Identified on 0.8.0, reproducible on 0.8.1 and confirmed present on main
Rust version: rustc 1.92.0
Operating system version: Linux
Steps to reproduce
With http >= 1.4.1 in the lockfile (the current default resolution):
use pingora_http::RequestHeader;
fn main() {
// What the downstream session does internally when parsing an
// inbound CONNECT request line:
let connect = RequestHeader::build("CONNECT", b"example.com:443", None);
println!("{connect:?}"); // Err: InvalidHTTPHeader "invalid uri example.com:443"
// Absolute-form, i.e. every non-CONNECT request an explicit
// forward proxy receives:
let absolute = RequestHeader::build("GET", b"http://example.com/index.html", None);
println!("{absolute:?}"); // Err with http >= 1.4.1
}
Equivalently end-to-end: run any ProxyHttp service, point a client at it as an HTTP proxy (curl -x http://proxy:port https://example.com/ for CONNECT, curl -x http://proxy:port http://example.com/ for absolute-form), and observe the 400. The failing parse happens inside the downstream session before any user trait method runs, so it can't be worked around in application code.
Expected results
Request targets parse according to their RFC 9112 form: CONNECT example.com:443 yields a URI with authority() == "example.com:443" and raw_path() round-tripping the original wire bytes; absolute-form yields the correct scheme/authority/path split (uri.path() == "/index.html").
Observed results
InvalidHTTPHeader ("invalid uri ...") from set_raw_path for both forms; in a running proxy, affected requests are rejected with a 400 before reaching application code (all requests for an HTTP/1 forward proxy, absolute-form requests for a reverse proxy).
Additional context
- The
http 1.4.1 changelog entry is "Fix PathAndQuery::from_static() and from_shared() to reject inputs that do not start with /". Uri::builder().path_and_query() goes through the same validation. (http 1.4.2, June 8, relaxed the builder for * with scheme and authority; it doesn't change this bug.)
- Current workaround: pinning
http < 1.4.1, which has to be rediscovered by every affected user after an unrelated cargo update, and pins against an upstream correctness fix.
- Suggested fix: in
set_raw_path, parse authority-form (method is CONNECT) and absolute-form (http:// / https:// prefix, case-insensitive) targets with str::parse::<Uri>() instead of path_and_query(). When the parsed URI has no path-and-query (authority-form, or absolute-form without a path), keep the original target bytes in the existing raw_path_fallback field so raw_path() stays total and serialization preserves the exact wire target. The CONNECT branch relies on the method being set before the path, which build_no_case already guarantees (worth a doc note, since both setters are public). set_uri needs the same totality treatment, since it can already produce a URI without a path-and-query today, which makes raw_path() panic. I've drafted this fix and can put up a PR if there's interest. It would also let test_connect_proxying_disallowed_h1 run without the patched_http1 feature.
- Two intentional behavior changes worth calling out. First, absolute-form targets with a path re-serialize origin-form through
raw_path() (per RFC 9112 §3.2.2, the correct form when forwarding to an origin server), where pre-1.4.1 behavior passed the full URL through byte-for-byte. Second, uri.path() changes from the whole URL to the path component, which could affect routing logic relying on the old (arguably wrong) value. Both alter what existing users pinned below 1.4.1 would see on upgrade, so they should have explicit tests rather than be silently included.
- Known limitations of the sketch: scheme detection covers http and https only (other schemes in absolute-form remain rejected, as today), and the non-UTF-8 lossy branch is unchanged, so it still fails on stock dependencies for targets that don't start with
/ (the existing patched_http1-gated unit tests cover that territory).
- Related: the same
raw_path_fallback mechanism would let callers control the serialized request target independently of the parsed URI. The concrete case is proxy chaining: when forwarding to a next-hop proxy rather than an origin server, the request line must stay absolute-form on the wire (RFC 9112 §3.2.2), but with this fix an absolute-form target parses into scheme/authority/path, so raw_path() serializes origin-form, which is right for origin servers and wrong for upstream proxies. There's currently no way to express "parse this as a URI, but send these exact target bytes" from outside the crate, since raw_path_fallback is private and only set internally. A small public setter (or equivalent serialization hook) would cover it. Since that's an API-design question rather than a bug, I'd propose it separately and keep any fix PR here scoped to the parsing bug.
(Caveat emptor: This report was drafted with LLM assistance, as Rust is not a language I'm very familiar with. I applied enough critical thought that I'm happy to put my name to it, but I invite above-average scrutiny and welcome any feedback!)
Describe the bug
RequestHeader::set_raw_pathin pingora-http builds the URI viaUri::builder().path_and_query(target)for every request-target form. As ofhttp1.4.1,PathAndQueryrejects (correctly, per RFC 9112 §3.2) any input that doesn't begin with/(origin-form) or equal*(asterisk-form). That breaks the other two request-target forms:CONNECT host:port): rejected, so every CONNECT request fails to parse.GET http://host/path): rejected, so requests carrying a full URL in the request line fail too. (Before 1.4.1 these "worked" but lefturi.path()returning the entire URL instead of the path component.)The impact depends on deployment style. HTTP/1 explicit forward proxies lose every request, since CONNECT and absolute-form are the only two forms they receive. Reverse proxies lose only absolute-form requests, but RFC 9112 §3.2.2 requires all servers to accept absolute-form, so conformant clients that send it now get 400s. HTTP/2 is unaffected (that path builds
RequestHeaderfromhttp::request::Partsand never callsset_raw_path).The breakage is lockfile-dependent and silent: pingora-http declares
http = "1", so a routinecargo update(or any fresh resolution) introduces the failure with no compile-time signal.I suspect this is what motivated the 0.8.1 change that gated
test_connect_proxying_disallowed_h1behind thepatched_http1feature ("Authority-form CONNECT request targets require patched HTTP/1 parsing until general request-target form support is available"). If so, the gating was working around this bug: the failure reproduces with stock crates.io dependencies and can be fixed inset_raw_pathalone, after which that test passes without the patched parser stack.Pingora info
Pingora version: Identified on 0.8.0, reproducible on 0.8.1 and confirmed present on main
Rust version: rustc 1.92.0
Operating system version: Linux
Steps to reproduce
With
http >= 1.4.1in the lockfile (the current default resolution):Equivalently end-to-end: run any
ProxyHttpservice, point a client at it as an HTTP proxy (curl -x http://proxy:port https://example.com/for CONNECT,curl -x http://proxy:port http://example.com/for absolute-form), and observe the 400. The failing parse happens inside the downstream session before any user trait method runs, so it can't be worked around in application code.Expected results
Request targets parse according to their RFC 9112 form:
CONNECT example.com:443yields a URI withauthority() == "example.com:443"andraw_path()round-tripping the original wire bytes; absolute-form yields the correct scheme/authority/path split (uri.path() == "/index.html").Observed results
InvalidHTTPHeader("invalid uri ...") fromset_raw_pathfor both forms; in a running proxy, affected requests are rejected with a 400 before reaching application code (all requests for an HTTP/1 forward proxy, absolute-form requests for a reverse proxy).Additional context
http1.4.1 changelog entry is "FixPathAndQuery::from_static()andfrom_shared()to reject inputs that do not start with/".Uri::builder().path_and_query()goes through the same validation. (http1.4.2, June 8, relaxed the builder for*with scheme and authority; it doesn't change this bug.)http < 1.4.1, which has to be rediscovered by every affected user after an unrelatedcargo update, and pins against an upstream correctness fix.set_raw_path, parse authority-form (method is CONNECT) and absolute-form (http:///https://prefix, case-insensitive) targets withstr::parse::<Uri>()instead ofpath_and_query(). When the parsed URI has no path-and-query (authority-form, or absolute-form without a path), keep the original target bytes in the existingraw_path_fallbackfield soraw_path()stays total and serialization preserves the exact wire target. The CONNECT branch relies on the method being set before the path, whichbuild_no_casealready guarantees (worth a doc note, since both setters are public).set_urineeds the same totality treatment, since it can already produce a URI without a path-and-query today, which makesraw_path()panic. I've drafted this fix and can put up a PR if there's interest. It would also lettest_connect_proxying_disallowed_h1run without thepatched_http1feature.raw_path()(per RFC 9112 §3.2.2, the correct form when forwarding to an origin server), where pre-1.4.1 behavior passed the full URL through byte-for-byte. Second,uri.path()changes from the whole URL to the path component, which could affect routing logic relying on the old (arguably wrong) value. Both alter what existing users pinned below 1.4.1 would see on upgrade, so they should have explicit tests rather than be silently included./(the existingpatched_http1-gated unit tests cover that territory).raw_path_fallbackmechanism would let callers control the serialized request target independently of the parsed URI. The concrete case is proxy chaining: when forwarding to a next-hop proxy rather than an origin server, the request line must stay absolute-form on the wire (RFC 9112 §3.2.2), but with this fix an absolute-form target parses into scheme/authority/path, soraw_path()serializes origin-form, which is right for origin servers and wrong for upstream proxies. There's currently no way to express "parse this as a URI, but send these exact target bytes" from outside the crate, sinceraw_path_fallbackis private and only set internally. A small public setter (or equivalent serialization hook) would cover it. Since that's an API-design question rather than a bug, I'd propose it separately and keep any fix PR here scoped to the parsing bug.