Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hyper-util auto version detection breaks the ability to use Upgraded::downcast #3587

Open
RyanAD opened this issue Feb 29, 2024 · 2 comments
Labels
C-bug Category: bug. Something is wrong. This is bad! K-hyper-util Crate: hyper-util

Comments

@RyanAD
Copy link

RyanAD commented Feb 29, 2024

Version
hyper = 1.2.0
hyper-util = 0.1.3

Description

When using: hyper_util::server::conn::auto::Builder::new(..).serve_connection_with_upgrades the io stream is wrapped with Rewind which is private and therefore cannot be used to call Upgraded::downcast.

To reproduce the issue I'v modified the upgrades example here: https://github.com/RyanAD/hyper-broken-upgrade

The shortened version is that handing a connection with:

hyper_util::server::conn::auto::Builder::new(TokioExecutor::new())
                    .serve_connection_with_upgrades(io, service_fn(server_upgrade)).await;

...

// downcast to the underlying TcpStream
let parts = upgraded
    .downcast::<TokioIo<TcpStream>>()
    .map_err(|_| "server: Unable to downcast Upgraded")?;

let mut conn: TcpStream = parts.io.into_inner();

Will fail, because the stream is actually wrapped in Rewind which is not visible externally and can't be used to call downcast.

Without using hyper_util::server::conn::auto::Builder::new it works as expected. This also prevents downcasting Upgraded when using axum::serve, but that can be worked around by setting up the handling manually without using auto::Builder

@RyanAD RyanAD added the C-bug Category: bug. Something is wrong. This is bad! label Feb 29, 2024
@seanmonstar seanmonstar added the K-hyper-util Crate: hyper-util label Mar 1, 2024
@seanmonstar
Copy link
Member

Yea good point. Before, we were able to work around this explicitly since both types were in the same library. Now that they are separate, it will be trickier to do so. 🤔

@conradludgate
Copy link

bikeshedding/stopgap: hyper_util::server::conn::auto::upgrade::downcast::<T>(upgraded) (that's a horrible path but whatever).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug. Something is wrong. This is bad! K-hyper-util Crate: hyper-util
Projects
None yet
Development

No branches or pull requests

3 participants