Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Add HTTP/2 push support to Server #1586

Open
vbrandl opened this issue Jun 28, 2018 · 10 comments
Open

Add HTTP/2 push support to Server #1586

vbrandl opened this issue Jun 28, 2018 · 10 comments
Labels
A-http2 Area: HTTP/2 specific. B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature. E-hard Effort: hard. Likely requires a deeper understanding of how hyper's internals work.

Comments

@vbrandl
Copy link

vbrandl commented Jun 28, 2018

What is the state of HTTP/2 server push in hyper? I couldn't find anything in the documentation. Are there any plans to implement it in the future?
@seanmonstar
Copy link
Member

First step is that the h2 dependency would need Push support: hyperium/h2#291

@seanmonstar seanmonstar added B-upstream Blocked: needs a change in a dependency or the compiler. A-http2 Area: HTTP/2 specific. labels Jun 28, 2018
@seanmonstar seanmonstar added E-hard Effort: hard. Likely requires a deeper understanding of how hyper's internals work. C-feature Category: feature. This is adding a new feature. and removed B-upstream Blocked: needs a change in a dependency or the compiler. labels Dec 13, 2019
@seanmonstar
Copy link
Member

With h2 supporting push promises, I'd like to start proposing an API to support them in hyper, initially from the server side:

  • We'd add a hyper::push module to contain these new pieces.
  • Add a pusher(req: &mut http::Request) -> hyper::Result<Pusher> that can get a Pusher related the request's stream, or an Error explaining why a Pusher wasn't available. Error cases could include that its an HTTP/1 message, that the client disabled server push, and the like.
  • The Pusher would have an async fn push_request(&mut self, req: http::Request<()>) -> hyper::Result<()> method that tries to send the PUSH_PROMISE frame to the client. Error cases could include that the request isn't a legal PUSH_PROMISE, that the clients max concurrent streams limit has been reached, the stream has closed, and the like.
  • Once a push promise has been sent to the client, the same http::Request shall be passed to Service::call, and that future spawned in a new task, to generate the response for the PUSH_PROMISE.

An example using this proposal looks like this:

async fn handle(mut req: Request<Body>) -> Result<Response<Body>, E> {
    match hyper::push::pusher(&mut req) {
        Ok(mut pusher) => {
            let promise = Request::builder()
                .uri("/app.js")
                .body(())
                .unwrap();
            if let Err(e) = pusher.push_request(promise).await {
                eprintln!("push failed: {}", e);
            }
        },
        Err(e) => eprintln!("http2 pusher unavailable: {}", e),
    }

    Ok(Response::new(index_page_stream()))
}

@seanmonstar seanmonstar changed the title State of HTTP/2 push Add HTTP/2 push support to Server Dec 14, 2019
@djc
Copy link
Contributor

djc commented Dec 17, 2019

@stammw would the proposed API make sense for HTTP 3?

@seanmonstar seanmonstar mentioned this issue Dec 20, 2019
Closed
@stammw
Copy link

stammw commented Feb 9, 2020

I was waiting to figure out a first implementation of push streams for quinn-h3 before
answering this, but here is what I can say just by comparing the two specs and
looking at hyper's code:

H2 and H3 share the same semantics on push promises / push streams, except:

  1. H3 draft states that Promises are conveyed on response streams, in any part
    of it: before header, interleaved with DATA frames...

    In RFC 7540, I can't find an explicit statement saying which stream shall be
    used to send the promise. But using the response stream is the only option that
    makes sense to me.

    On this line:

    pusher.push_request(promise).await

    I guess Pusher will hold a reference to the response stream so a
    PUSH_PROMISE frame is sent before the response itself, on the same stream ?

  2. In H3, several requests can share the same promise. A first PUSH_ID is
    created by a request, then if a concurrent request needs to push the same
    resource, it should send a DUPLICATE_PUSH with the same PUSH_ID to avoid
    sending the content multiple times.

    It seems to me that it is possible to achieve that by holding something like
    Map<Request, PUSH_ID> in Service, so Pusher can decide to bind a
    .push_request(x) call to another one that has already started for the same
    resource. Sending only a DUPLICATE_PUSH.

I have a limited knowledge of Hyper for now, so I hope this helps!

@stammw
Copy link

stammw commented Apr 19, 2020

UPDATE: DUPLICATE_PUSH mechanism have been removed from the HTTP/3 spec. So I guess semantics are now identical.

@eggyal
Copy link

eggyal commented Sep 16, 2020

@seanmonstar I'm happy to take a look at this, but I'm not sure about the proposed API. In particular:

  • Add a pusher(req: &mut http::Request) -> hyper::Result<Pusher> that can get a Pusher related the request's stream, or an Error explaining why a Pusher wasn't available. Error cases could include that its an HTTP/1 message, that the client disabled server push, and the like.
  • The Pusher would have an async fn push_request(&mut self, req: http::Request<()>) -> hyper::Result<()> method that tries to send the PUSH_PROMISE frame to the client. Error cases could include that the request isn't a legal PUSH_PROMISE, that the clients max concurrent streams limit has been reached, the stream has closed, and the like.

Clients can disable (and reenable) server push at any point during the life of a connection, by sending updated settings. Therefore this design could create a race hazard, where a Pusher is created (whilst the client supports server push) but the push_request itself fails (because the client has disabled it in the interim).

Would it not be better simply to always permit push attempts (i.e. without any Pusher object) and handle all failure cases, including lack of protocol support or clients that have disabled server push, as a result of that operation?

@seanmonstar
Copy link
Member

Hm, that's a fair point. I think, however, that we'd need to consider the likelihood of different scenarios happening. I believe it would be uncommon for clients to enable and disable push promises mid-connection. Even if they do, it should be very rare for that to happen with each request.

I do think servers may want to know if pushes would even possible be accepted before starting to create Requests. So by checking that first, they can potentially skip looking up other stuff or creating Requests. Especially if it is HTTP/1...

Then again, one option is to allow "pushing" even with HTTP/1.1, and in that case, sending 103 Early Data or Link headers. This could be added later without backward compatibility problems.

@seanmonstar seanmonstar mentioned this issue Feb 8, 2021
Open
@acfoltzer acfoltzer mentioned this issue Mar 14, 2022
Merged
@AlbertMarashi
Copy link

Is this still waiting?

@seanmonstar seanmonstar added the B-rfc Blocked: More comments would be useful in determine next steps. label Aug 31, 2022
@gi0baro gi0baro mentioned this issue Jun 26, 2023
Open
@mladedav
Copy link

Support has been removed from Chrome, Firefox, and even Nginx.

I don't think hyper needs to support this.

@AlbertMarashi
Copy link

Should be closed then

# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
A-http2 Area: HTTP/2 specific. B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature. E-hard Effort: hard. Likely requires a deeper understanding of how hyper's internals work.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@seanmonstar @djc @eggyal @stammw @mladedav @AlbertMarashi @vbrandl