Skip to content

RequestHeader offers DerefMut access to its Parts that allow violation of internal consistency #917

Description

@fiadliel

Describe the bug

pingora_request::RequestHeader offers a DerefMut that gives mutable access to http::request::Parts. From here, you can modify various parts of the request (accidentally!) that violate the consistency of the request data structures. Footguns should not be this easy to trigger; so I think this trait impl should be removed.

Pingora info

Please include the following information about your environment:

Pingora version: 0.8.1
Rust version: cargo 1.98.0-nightly (fe63976b2 2026-06-11)
Operating system version: MacOS arm64 26.5.1

Steps to reproduce

Adding the below to the quickstart on the website:

    async fn upstream_request_filter(
        &self,
        _session: &mut Session,
        upstream_request: &mut RequestHeader,
        _ctx: &mut Self::CTX,
    ) -> Result<()> {
        upstream_request.headers.remove("user-agent").unwrap();
        Ok(())
    }

This panics as long as there is a header following user-agent in the request:

Request:

> curl -v http://localhost:6188
* Host localhost:6188 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:6188...
* connect to ::1 port 6188 from ::1 port 56336 failed: Connection refused
*   Trying 127.0.0.1:6188...
* Connected to localhost (127.0.0.1) port 6188
> GET / HTTP/1.1
> Host: localhost:6188
> User-Agent: curl/8.7.1
> Accept: */*
>
* Request completely sent off
* Empty reply from server
* Closing connection
curl: (52) Empty reply from server

Server (stdout/stderr):

upstream peer is: Backend { addr: Inet(1.1.1.1:443), weight: 1, ext: {} }

thread 'Pingora HTTP Proxy Service' (24044531) panicked at /Users/gcoady/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/pingora-http-0.8.1/src/lib.rs:755:17:
assertion `left == right` failed: header iter mismatch user-agent, accept
  left: "user-agent"
 right: "accept"

Expected results

The header user-agent was removed, but the request completed successfully. headers was accessible and seemed at first glance like a completely viable and supported part of the API.

Observed results

Panic, as above.

Additional context

Pingora has remove_header that implements the appropriate bookkeeping. However, DerefMut here is a footgun that should be removed.

Metadata

Metadata

Assignees

Labels

AcceptedThis change is accepted by us and merged to our internal repoWIPWe are working on this feature internallybugSomething 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