`Upgraded::downcast_ref` is unusable with `hyper_util::server::conn::auto`

[nox]

hyper_util::server::conn::auto::Connection<'a, I, S, E> internally wraps I in hyper_util::common::rewind::Rewind<_>, which is a private type, so we cannot ever make use of Upgraded::downcast_ref anymore.

While I could expose Rewind<_> in hyper_util and improve docs in hyper_util::server::conn::auto to signal that one should be downcasting to Rewind<I>, I am filing this in hyper because I thought of a potentially better resolution:

1. Introduce hyper::server::conn::http1::Builder::serve_buffered_connection:
   
   

   impl Builder {
       pub fn serve_buffered_connection<I, S>(&self, buffered: Bytes, io: I, service: S) -> Connection<I, S>
       where
           S: HttpService<IncomingBody>,
           S::Error: Into<Box<dyn StdError + Send + Sync>>,
           S::ResBody: 'static,
           <S::ResBody as Body>::Error: Into<Box<dyn StdError + Send + Sync>>,
           I: Read + Write + Unpin,
       {
           let mut conn = proto::Conn::new_buffered(buffered, io);
           …
       }
   }

2. Use that in hyper_util::server::conn::auto::Builder::serve_connection instead of wrapping I into Rewind<_>.

[nox]

hyper_util::server::conn::auto::Builder itself could also be equipped with new methods serve_buffered_connection and serve_buffered_connection_with_upgrades to accomodate for users doing their own kind of prebuffering (we do such things at work so we also have a type similar to Rewind<_> laying around).

[nox]

Of course, another solution would be to just move hyper_util::server::conn::auto to hyper::server::conn::auto.

[seanmonstar]

I want a solution to the problem, definitely.

• I hope that auto can eventually be moved into hyper from hyper-util, but I do think it needs bake a little more (and know how it can support h3).
• At least from my feelings, I don't love including a method like serve_buffered_connection. It feels like exposing an internal detail that wouldn't otherwise be something we would expose.

I'm not sure exactly how to allow it, though. I kind of find myself wishing for std::any::provide(), which I know got its scope shrunk to only work with Error now. Is there any other mechanism we haven't thought of?

[nox]

At least from my feelings, I don't love including a method like serve_buffered_connection. It feels like exposing an internal detail that wouldn't otherwise be something we would expose.

Tbh "I have a buffer of bytes I read and I need to do something like Rewind<_> to feed it back to Hyper" is a thing that comes up a gazillion times here at work, and it always bothers me a little bit because I know Hyper has its own read buffer internally.

Is there any other mechanism we haven't thought of?

Given that auto is supposed to be put back into hyper at some point, maybe we could just expose Rewind and explain that users need to downgrade to Rewind<S> instead of S? Would that be acceptable as a temporary crutch? It's not really usable for us because of this issue right now.

[nox]

@seanmonstar Can we make some progress about this?

[nox]

I'll make a hyper_util PR with a new module hyper_util::server::conn::auto::upgrade:

pub fn downcast<T: Read + Write + Unpin + 'static>(ext: Upgraded) -> Result<T, Upgraded>;

This way the kludge will be contained to hyper_util.

[seanmonstar]

That sounds like a decent solution, actually!

[nox]

There is still an issue with that though, the return type should be Result<Parts<T>, Upgraded> but hyper_util cannot construct a Parts<T> as it is non_exhaustive.