feat(conn/auto): allow serving connection with upgrade after config

[martinetd]

Hello!
Thanks for this crate, I've been using it with axum since converting to hyper 1.0.

However I had been using it with default values, and after https://seanmonstar.com/blog/hyper-http2-continuation-flood/ I wanted to set max_header_list_size as suggested, and realized I couldn't call .serve_connection_with_upgrades with that (I've seen serve_connection().with_upgrades() in another part of the code but that doesn't seem to make sense with this one that returns a future Result<()> -- and not using with_upgrades means I cannot create websockets anymore so that's not an option either)

Happy to rework it otherwise, or if that's easier let someone else implement something different (or just tell me I could use xyz in my code!) and drop this.

Cheers,

---

When using hyper_util::server::conn::auto::Builder, if one wants to set http1 or http2 options one has to specialize the struct into Http1Builder or Http2Builder with .http1()/.http2() methods.

However, once the struct has been specialized there is no way to go back to the inner Builder to call serve_connection_with_upgrades(): one would need to either add make inner public, add an inner() method to get it back, or add a stub that just calls the method on inner like we have with serve_connection().

Since we already had one for serve_connection(), add an indentical one for serve_connection_with_upgrades()
Note that it does not make sense for serve_connection_with_upgrades() to be called without the http feature (I think?), so it has only been implemented for http1 if the http2 feature is set.

[seanmonstar]

However, once the struct has been specialized there is no way to go back to the inner Builder to call serve_connection_with_upgrades()

Yea, I noticed that too. One potential solution I put forth was #93, but could just be a method.

I also was thinking that if we add a mirror http1_max_header_bytes or something, then the main builder could allow a method that configures for both: Builder::new().max_header_bytes(1024 * 16) would set both HTTP/1 and 2.

[martinetd]

Yea, I noticed that too. One potential solution I put forth was #93, but could just be a method.

Oh, hadn't seen this (nor #98 ...); I'm happy with any solution as long as long as it's usable. I don't consider myself to be good enough at rust to weight the pros and cons here but if I understand it right the deref implem would allow to just use any of the http1/http2/auto methods without thinking about it as long as there's no conflicts, so it sounds neat but I'm a not sure about extensibility long term so explicit is probably just as good.
I agree going back to auto from http1/http2 probably makes more sense than wrapping all the auto calls (even if there's only these two), so unless you see a problem with #93 I'd be happy to drop this in favor of it. Is there a reason it's been there for 3 months?

(self-centered note: We have a monthly release schedule so ideally if there could be a release with this by the 22 or so it'd be awesome; if you think this requires discussion and will take more time I'll whip up a local override for this month and we can discuss this more leisurely)

I also was thinking that if we add a mirror http1_max_header_bytes or something, then the main builder could allow a method that configures for both: Builder::new().max_header_bytes(1024 * 16) would set both HTTP/1 and 2.

Such shortcuts would also be welcome imo -- sharing anything that's not http1/2 specific at the auto level definitely makes sense.

Thanks!

[martinetd]

Thanks!

(and I have no idea what github is doing but it overwrote the author I had set in my branch from work email to personal email, ugh. Oh well, so much for attributions...)