Skip to content

HTTP CONNECT proxy tunnel has lax response parsing and loses early data #4095

Open
hyperium/hyper-util
#320

Description

@arjan-bal

Version

hyper-util 0.1.20

Platform

Linux 6.18.14

Summary

While evaluating hyper-util's client::legacy::connect::proxy::tunnel for use with gRPC routing over HTTP CONNECT proxies, I identified two limitations in the current handshake implementation.

1. Lax Response Parsing

It heuristically matches the start and end of the expected response, potentially accepting malformed responses.

This can be fixed by using httparse to properly parse the response.

2. No Buffering for Early Data

It assumes the read buffer contains no data from the target server immediately after the proxy's response. This assumption holds for protocols where the client always speaks first (e.g., TLS) after the tunnel is established. However, since gRPC implementations must work with any security protocol, they must handle cases where the server sends data immediately.

Fixing this would require an API change, as the returned I/O stream would need to incorporate a buffer for the peeked data, similar to the private Rewind struct.

Although the handshaking code is simple enough to be re-implemented in gRPC, I wanted to check if these fixes can be upstreamed to allow us to use hyper-util.

Code Sample

Expected Behavior

  • The proxy response must be correctly parsed.
  • The data from the target server must be retained.

Actual Behavior

  • A malformed proxy response may be accepted
  • The data from the target server may be lost.

Additional Context

No response

Activity

  1. added
    C-bugCategory: bug. Something is wrong. This is bad!
    on Jun 3, 2026
  2. seanmonstar commented on Jun 8, 2026

    @seanmonstar
    Member

    True, it could be better. We could rewrite it to wrap some Service instead, that could use hyper::upgrade::on and normal request, and that would allow it to go over HTTP/1 or HTTP/2, even.

  3. Catwoman08 commented on Aug 17, 2026

    @Catwoman08
    Contributor

    @arjan-bal Thank you for your report. Would you like to contribute a PR?

  4. arjan-bal commented on Aug 18, 2026

    @arjan-bal
    Author

    Since adding Rewind would introduce a breaking API change (or a new API?) for hyper, I ended up implementing the HTTP CONNECT handshake directly in grpc-rust:
    https://github.com/grpc/grpc-rust/blob/2968bea2c4855568aadd8d74086e2531dfa2c7b5/grpc/src/client/transport/http_connect/mod.rs#L49-L136

    It uses httparse to parse the CONNECT response and a local Rewind adapter to buffer any extra data received from the destination server. I don't plan to submit a PR for hyper at this time.

  5. Catwoman08 commented on Aug 20, 2026

    @Catwoman08
    Contributor

    Thank you for responding. We are still interested in a PR for this issue if someone wants to take a stab at it.

  6. heitzlki commented on Sep 8, 2026

    @heitzlki

    Hi, I'd like to take a stab at this. From what I can tell hyper-util#315 fixed the parsing, but early data still gets dropped. tunnel() just throws away anything after the header end.

    One way to fix it is a rewind wrapper around the returned conn, but that changes Tunnel's response type, and I'm guessing that's why hyper-util#313 went nowhere. So before I write code: would you take that type change, should it be a new API like @arjan-bal suggested, or do you want the Service + hyper::upgrade::on rewrite @seanmonstar mentioned?

  7. seanmonstar commented on Sep 8, 2026

    @seanmonstar
    Member

    Hm, yea, I hadn't noticed why that PR was opened and closed immediately. But looking at it, the change in return type is a breaking change. So probably wouldn't be as easy to land.

    The alternative way I mentioned is a much bigger change. If we did it, it'd probably need to be named something new, either clarifying why it's different or just indicating it would be the better way in an eventual breaking change.

  8. heitzlki commented on Sep 9, 2026

    @heitzlki

    Makes sense. If it needs a new name anyway, the upgrade-based version seems like the better thing to build than a second hand-rolled one.

    Rough idea before I write code: same shape as Tunnel (wraps a connector, takes the proxy URI and auth headers), but it does a real CONNECT request over http1 and returns the upgraded IO, so early data survives via the upgrade path. The h1 connection future resolves once the upgrade fires, so it can be driven inside the connector future itself, no spawned task needed. HTTP/2 could come later behind the same type. Any preference on the name? Was thinking ConnectProxy or HttpConnect.

  9. added a commit that references this issue on Oct 13, 2026
    c644755
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-clientArea: client.C-bugCategory: bug. Something is wrong. This is bad!E-pr-welcomeEffort: a pull request is welcome.K-hyper-utilCrate: hyper-util

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions