Skip to content

HTTP/1 CONNECT and absolute-form requests fail to parse with http crate >= 1.4.1 (set_raw_path rejects non-origin-form request targets) #909

Description

@torinnd

(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.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions