hyper icon indicating copy to clipboard operation
hyper copied to clipboard

Prepare forwards-compatibile Body trait

Open seanmonstar opened this issue 2 years ago • 10 comments

We want to explore a trait that can be forwards-compatible in frame types for a body stream.

Steps:

  • [x] Accept a proposal
  • [ ] Implement in the http-body crate.

seanmonstar avatar May 20 '22 17:05 seanmonstar

Proposal

Problem

The Body trait provides methods to allow basically 2 frame types to be passed along: data, and trailers. There's a video overview, but I'll summarize: in order to be Flexible, we want to allow Bodys to transmit frame types that hyper doesn't even know about. We can't do that with per-frame-type methods.

Frames

We want a Frame like thing that users can poll (and send). It would be convenient if it behaved like an enum, where they could match on the variants, but there's a forwards-compatibility trap. Consider:

match f {
    Frame::Data(buf) => {},
    Frame::Trailers(map) => {},
    Frame::Unknown(raw) => {},
    // #[non_exhaustive]
    _ => {},
}

If we were to start to "understand" a frame type that used to fall into the Unknown variant, and created Frame::Priority(pri), then user code that updated would suddenly no longer be handling the frame.

So, we don't want to expose an enum directly. Something like the dyn match concept would be great, but it doesn't exist yet.

It'd be possible to inspect the frame using a bunch ifs. But Rust programmers like the feel of match, so a macro could do it.

Proposed Design

pub trait Body {
    type Data: Buf;
    fn poll_frame(..) -> Result<Option<Frame<Self::Data>>>;
}

pub struct Frame<T>(Kind<T>);

enum Kind<T> {
    // The first two variants are "inlined" since they are undoubtedly
    // the most common. This saves us from having to allocate a
    // boxed trait object for them.
    Data(T),
    Trailers(HeaderMap),
    Unknown(Box<dyn Frameish>),
}

/// The name is just YOLO for now.
pub trait Frameish {
    fn frame_kind(&self) -> u64;
    // Just a byte slice? The DATA type couldn't necessarily fulfill this...
    fn payload(&self) -> &[u8];
}

Usage

Matching

while let Some(fr) = body.frame().await? {
    // the macro is optional, but seems helpful
    match_dyn! { fr;
        frame::Data(buf) => {
            
        },
        frame::Trailers(trailers) => {
            
        },
        other => {
            if other.frame_kind() == 0xF3 {
                eprintln!("my custom frame: {:?}", other.payload());
            } else {
                // i dunno?
            }
        }
        
    }
}

Buffered

The Buffered util type provides methods to await specific frame types. It can do this by doing the dyn-match internally, and if it doesn't match the expected type, put the frame back into a slot to poll for next time.

let mut buffered = body.buffered();

// Buffered can poll for specific frame types
while let Some(data) = buffered.data().await? {
    
}

// DATA frames are done. What stopped them? A trailers frame? EOS?
if let Some(trailers) = buffered.trailers().await? {
    
}
// or maybe some other EOS-ish frame
if let Some(fr) = buffered.frame().await? {
    // ``\_(0_0)_/``
}

Sending

Assume a channel where the receiver is impl Body.

let (tx, rx) = hyper_util::body::channel();
// give away the rx

tx.send_frame(Frame::data(buf)).await?;
tx.send_frame(Frame::trailers(map)).await?;
tx.send_frame(Frame::custom(0xF3, b"yolo")).await?;

Remaining Questions

How does dyn_match! convert from to frame::Data?

Just a quich sketch:

impl<T> Frame<T> {
    pub fn is_data(&self) -> bool;
    pub fn into_data(self) -> Option<frame::Data<T>>;
    
    pub fn is_trailers(&self) -> bool;
    pub fn into_trailers(self) -> Option<frame::Trailers>;
    
    pub fn frame_kind(&self) -> u64;
}

Do we seal the Frameish trait?

Should users implement the type for their own new frame kinds?

Or should they just use Frame::custom(id, payload)?

seanmonstar avatar Aug 17 '22 20:08 seanmonstar

cc @acfoltzer @davidpdrsn @nox @olix0r

You all either maintain proxies that could need this, or previously done a lot with http-body trait.

seanmonstar avatar Aug 17 '22 20:08 seanmonstar

Also @LucioFranco and @rcoh.

seanmonstar avatar Aug 19 '22 20:08 seanmonstar

I'm going to move forward with this, unless anyone wants to comment.

seanmonstar avatar Aug 24 '22 17:08 seanmonstar

Oh sorry missed this, What would exist in http-body vs what would exist in hyper? Would all the frame stuff go into the body crate? I think this makes sense though I am curious to see how it works in practice.

LucioFranco avatar Aug 24 '22 20:08 LucioFranco

Yes pretty much everything proposed here is actually for http-body.

seanmonstar avatar Aug 24 '22 22:08 seanmonstar

HTTP 2 and 3 have slightly different meanings on their frame type ids. The body may need to provide information about its underlying connection protocol.

HyeonuPark avatar Aug 26 '22 02:08 HyeonuPark

That's a good point. HTTP/3 tried to reuse the same IDs for equivalent frames, but there may be subtle differences, and it could be worth including a version() method to a frame. Or, perhaps it should be up to the user to set the version() on the corresponding Request/Response`. Either way, doesn't need to be determined immediately.


I'm moving forward with the proposal.

seanmonstar avatar Sep 08 '22 01:09 seanmonstar

Hello, @seanmonstar! @acfoltzer and I work for the same company. This is great progress and we are looking forward to seeing how custom HTTP/2 frames will be handled in hyper. At the moment we have a patched h2 crate that has special handling for our custom frame, which works fine, but it is always nice to see standard interfaces for things.

A few things spring to mind related to the plan:

In addition to frame_kind and payload, I think that it would be useful for Frameish to have an accessor for flags. In our internal HTTP/2-based protocol we use the flags field in the frame header of our custom frames for distinguishing between different payload formats.

What do you think would be the best way to represent custom frames exchanged before the headers frame? In our internal HTTP/2-based protocol a custom frame with a stream id can precede the headers frame. In that case, rather than the headers frame creating the new stream, it is the custom frame that creates it. We use the extensions/extensions_mut interface on the http crate's Request and Response as a super convenient way to retrieve the custom frame(s) from the request and to attach it to the response.

We also use custom settings, though this is getting further and further away from the topic of the Body trait.

jhatala avatar Sep 09 '22 01:09 jhatala

it would be useful for Frameish to have an accessor for flags

Hm, yea, flags would be good to support too. Would you like to push forward on exploring the design for custom frames? I've got a deadline for a hyper 1.0 release candidate soon, and so my main priority is getting the types in place, such as Body::poll_frame and an extensible Frame, and then allow the custom parts to be added once ready.

What do you think would be the best way to represent custom frames exchanged before the headers frame?

The design doesn't accommodate that at all, since I've been assuming what it says in the spec is that it's illegal:

Receiving any frame other than HEADERS or PRIORITY on a stream in this state MUST be treated as a connection error (Section 5.4.1) of type PROTOCOL_ERROR.

I... think at that point I'd probably customize at the codec level, like the h2 crate. (I'd love to support what you're doing generally, as long as it keeps our goal of making hyper a spec-compliant HTTP library.)

seanmonstar avatar Sep 09 '22 16:09 seanmonstar