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

Support Http Response 103 (Early Hints) #8057

Closed
joakime opened this issue May 25, 2022 · 5 comments · Fixed by #8058
Closed

Support Http Response 103 (Early Hints) #8057

joakime opened this issue May 25, 2022 · 5 comments · Fixed by #8058
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement

Comments

@joakime
Copy link
Contributor

joakime commented May 25, 2022

Target Jetty version(s)
10+

Enhancement Description

What would it take to have Jetty support response code 103 (Early Hints)?

@sbordet
Copy link
Contributor

sbordet commented May 25, 2022

I wonder how a 103 is any different from a HTTP/2 PUSH_PROMISE, which would carry exactly the same information.

The only difference I can see is that PUSH_PROMISE may send the response content before the client has a chance to cancel it, but typically bandwidth is not a problem.

Server push has been removed from HTTP/3, and a 103 may possibly break clients that are unaware of it.

Furthermore, there is no API on the server to send a 103, which is similar to 100 Continue as in the server sends 2 (or more) responses for one request (yes you can have multiple 103 followed by a 200).
We send the 100 implicitly when the application calls getInputStream(), but in the Servlet realm there is no API.

We need to consider whether to add a "send informational response" API in Jetty 12.

@gregw
Copy link
Contributor

gregw commented May 25, 2022

I think a 103 is very similar to our current support of 102 PROCESSING. We currently allow a sendError(102) to be done that sends a partial response, but without any of the current header fields from the response. I think all that is needed is that we duplicate the 102 handling, but use a temporary MetaData.Response to contain the headers as they currently are.

However, I'm unsure if 102 or 103 are handled completely by http>=2

We also need to consider if we want optional or mandatory Expect headers.

@joakime
Copy link
Contributor Author

joakime commented May 25, 2022

Chrome 95 was the first version to support 103 Early Hints.

https://chromium.googlesource.com/chromium/src/+/master/docs/early-hints.md

Fastly opened support in Nov 2020.

Cloudfare opened support in Sep 2021

gregw added a commit that referenced this issue May 26, 2022
Added test harness for intermediary responses.
@gregw gregw added the Sponsored This issue affects a user with a commercial support agreement label May 26, 2022
@gregw gregw linked a pull request May 26, 2022 that will close this issue
@gregw
Copy link
Contributor

gregw commented May 26, 2022

I've opened a branch for this and started by adding a better test for 100 and 102 responses. These work for all protocols.
I'll implement 103 support later today.

gregw added a commit that referenced this issue May 26, 2022
Implemented 103 early hint, but is failing for HTTP>=2
gregw added a commit that referenced this issue May 26, 2022
Fixed H2
Attempted fix H3
gregw added a commit that referenced this issue May 26, 2022
Fixed H2
Attempted fix H3
gregw added a commit that referenced this issue May 26, 2022
fixed checkstyle
gregw added a commit that referenced this issue May 26, 2022
updates from review
gregw added a commit that referenced this issue May 30, 2022
Added test harness for intermediary responses.
gregw added a commit that referenced this issue May 30, 2022
Implemented 103 early hint, but is failing for HTTP>=2
gregw added a commit that referenced this issue May 30, 2022
Fixed H2
Attempted fix H3
gregw added a commit that referenced this issue May 30, 2022
Fixed H2
Attempted fix H3
gregw added a commit that referenced this issue May 30, 2022
fixed checkstyle
gregw added a commit that referenced this issue May 30, 2022
updates from review
gregw added a commit that referenced this issue May 30, 2022
updates from review
gregw added a commit that referenced this issue May 30, 2022
Found (but not fixed) flake.
gregw added a commit that referenced this issue May 30, 2022
Fixed flake. @sbordet please review this commit!
sbordet added a commit that referenced this issue May 30, 2022
Improved implementation on the client-side.
Introduced HttpStatus.isInterim() for 1XX codes that are not 101.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
gregw added a commit that referenced this issue Jun 1, 2022
Co-authored-by: Ludovic Orban <lorban@bitronix.be>
Co-authored-by: Simone Bordet <simone.bordet@gmail.com>
@gregw
Copy link
Contributor

gregw commented Jun 1, 2022

@joakime can you pass this on to the user

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants