You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "Chenjp (via GitHub)" <gi...@apache.org> on 2024/11/22 09:42:10 UTC
[PR] enhanced http11 early hints behavior [tomcat]
Chenjp opened a new pull request, #785:
URL: https://github.com/apache/tomcat/pull/785
twin PR of #784 .
1. automatically flush when earlyHints called, to enable the possibility of server-think-time between the Early Hints headers and the final response. Speeding up page loads is the main goal of Early hints feature, and the "server think-time" is the key.
2. unset early-hints relative headers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] enhanced http11 early hints behavior [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on code in PR #785:
URL: https://github.com/apache/tomcat/pull/785#discussion_r1853737908
##########
java/org/apache/coyote/http11/Http11Processor.java:
##########
@@ -1250,8 +1255,19 @@ protected final void ack(ContinueResponseTiming continueResponseTiming) {
@Override
protected void earlyHints() throws IOException {
- writeHeaders(103, response.getMimeHeaders());
+ MimeHeaders earlyHintsHeaders = new MimeHeaders();
+ earlyHintsHeaders.duplicate(response.getMimeHeaders());
+ earlyHintsHeaders.filter(HTTP_EARLY_HINTS_HEADERS);
+ writeHeaders(HttpServletResponse.SC_EARLY_HINTS, earlyHintsHeaders);
+ // Once early hints emitted, clean up early hints relative headers.
+ // The final response is responsible for resetting Link/CSP/etc related headers
+ for (String header : HTTP_EARLY_HINTS_HEADERS) {
+ response.getMimeHeaders().removeHeader(header);
+ }
Review Comment:
No need to duplicate MimeHeaders. No need to remove headers after sending 103.
##########
java/org/apache/coyote/http11/Http11Processor.java:
##########
@@ -1250,8 +1255,19 @@ protected final void ack(ContinueResponseTiming continueResponseTiming) {
@Override
protected void earlyHints() throws IOException {
- writeHeaders(103, response.getMimeHeaders());
+ MimeHeaders earlyHintsHeaders = new MimeHeaders();
+ earlyHintsHeaders.duplicate(response.getMimeHeaders());
+ earlyHintsHeaders.filter(HTTP_EARLY_HINTS_HEADERS);
+ writeHeaders(HttpServletResponse.SC_EARLY_HINTS, earlyHintsHeaders);
+ // Once early hints emitted, clean up early hints relative headers.
+ // The final response is responsible for resetting Link/CSP/etc related headers
+ for (String header : HTTP_EARLY_HINTS_HEADERS) {
+ response.getMimeHeaders().removeHeader(header);
+ }
outputBuffer.writeHeaders();
+ // to take advantage of the Early Hints feature, the server-think-time is needed between the Early Hints
+ // headers and the final response. Therefore, we need emit early hints status and headers via flush.
+ outputBuffer.flush();
Review Comment:
No need to flush.
##########
java/org/apache/coyote/http11/Http11Processor.java:
##########
@@ -1250,8 +1255,19 @@ protected final void ack(ContinueResponseTiming continueResponseTiming) {
@Override
protected void earlyHints() throws IOException {
- writeHeaders(103, response.getMimeHeaders());
+ MimeHeaders earlyHintsHeaders = new MimeHeaders();
+ earlyHintsHeaders.duplicate(response.getMimeHeaders());
+ earlyHintsHeaders.filter(HTTP_EARLY_HINTS_HEADERS);
+ writeHeaders(HttpServletResponse.SC_EARLY_HINTS, earlyHintsHeaders);
Review Comment:
I do support the use of a constant here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Http11Processor minor change - magic int 103 to HttpServletResponse.SC_EARLY_HINTS [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on PR #785:
URL: https://github.com/apache/tomcat/pull/785#issuecomment-2494595319
I've committed a change that replaces both uses of the 103 literal with the constant from the Servlet API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Http11Processor minor change - magic int 103 to HttpServletResponse.SC_EARLY_HINTS [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf closed pull request #785: Http11Processor minor change - magic int 103 to HttpServletResponse.SC_EARLY_HINTS
URL: https://github.com/apache/tomcat/pull/785
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org