You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "ChristopherSchultz (via GitHub)" <gi...@apache.org> on 2024/10/09 17:22:27 UTC
[PR] Allow applications to trigger sending of 103 early hints [tomcat]
ChristopherSchultz opened a new pull request, #761:
URL: https://github.com/apache/tomcat/pull/761
Use sendError(103) or setStatus(103).
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "markt-asf (via GitHub)" <gi...@apache.org>.
markt-asf commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2405469155
> I originally wrote this patch just for Tomcat 11 but now that I think about it, I think I should re-write it for Tomcat 12/main so that the behavior is consistent regardless of version.
Can you add a note that this `sendError()` behaviour is deprecated in 12 onwards? I'd quite like to remove it for 13 onwards.
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2404594880
> > Is there now static final for that status code?
>
> Are you talking about https://github.com/apache/tomcat/blob/main/java/jakarta/servlet/http/HttpServletResponse.java#L406 ?
Yes, this one. That's why I am surprised why a literal is used.
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "ChristopherSchultz (via GitHub)" <gi...@apache.org>.
ChristopherSchultz commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2405394259
I will also be adding at least one unit test.
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "ChristopherSchultz (via GitHub)" <gi...@apache.org>.
ChristopherSchultz commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2402893460
I noticed that `Response` does some things that `ResponseFacade` does not. Specifically, it is sensitive to whether or not there is an *include* in progress. However, if the `ResponseFacade` calls `sendEarlyHints`, the `Response` will ultimately ignore the operation if the servlet is being included.
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "isapir (via GitHub)" <gi...@apache.org>.
isapir commented on code in PR #761:
URL: https://github.com/apache/tomcat/pull/761#discussion_r1794578764
##########
java/org/apache/catalina/connector/Response.java:
##########
@@ -1209,7 +1213,11 @@ public void setStatus(int status) {
return;
}
- getCoyoteResponse().setStatus(status);
+ if(103 == status) {
+ sendEarlyHints();
+ } else {
+ getCoyoteResponse().setStatus(status);
Review Comment:
So we don't need to call this method if the status code is 103?
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "ChristopherSchultz (via GitHub)" <gi...@apache.org>.
ChristopherSchultz commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2405802038
I'm not great at github, so I ended up killing this branch and re-creating it. I will have a follow-up PR soon.
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "ChristopherSchultz (via GitHub)" <gi...@apache.org>.
ChristopherSchultz commented on code in PR #761:
URL: https://github.com/apache/tomcat/pull/761#discussion_r1795646509
##########
java/org/apache/catalina/connector/Response.java:
##########
@@ -1069,16 +1069,20 @@ public void sendError(int status, String message) throws IOException {
return;
}
- setError();
+ if(103 == status) {
Review Comment:
Oops I'll fix that. Forgot to run validate.
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2404487095
Is there now static final for that status code?
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "ChristopherSchultz (via GitHub)" <gi...@apache.org>.
ChristopherSchultz closed pull request #761: Allow applications to trigger sending of 103 early hints
URL: https://github.com/apache/tomcat/pull/761
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "rmaucher (via GitHub)" <gi...@apache.org>.
rmaucher commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2404535177
> Is there now static final for that status code?
Are you talking about https://github.com/apache/tomcat/blob/main/java/jakarta/servlet/http/HttpServletResponse.java#L406 ?
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "ChristopherSchultz (via GitHub)" <gi...@apache.org>.
ChristopherSchultz commented on PR #761:
URL: https://github.com/apache/tomcat/pull/761#issuecomment-2405393185
Tomcat 11 is based on Jakarta EE 11 which does not (yet) have this constant.
I originally wrote this patch just for Tomcat 11 but now that I think about it, I think I should re-write it for Tomcat 12/main so that the behavior is consistent regardless of version.
So I will re-base this PR on main which will use the constant in main. The back-ports will require the use of a literal, or a constant defined somewhere other than `HttpServletResponse`.
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "rmaucher (via GitHub)" <gi...@apache.org>.
rmaucher commented on code in PR #761:
URL: https://github.com/apache/tomcat/pull/761#discussion_r1794983398
##########
java/org/apache/catalina/connector/Response.java:
##########
@@ -1209,7 +1213,11 @@ public void setStatus(int status) {
return;
}
- getCoyoteResponse().setStatus(status);
+ if(103 == status) {
+ sendEarlyHints();
+ } else {
+ getCoyoteResponse().setStatus(status);
Review Comment:
The implementations set 103 (except AJP where it does nothing).
--
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] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Igal,
On 10/9/24 23:50, isapir (via GitHub) wrote:
>
> isapir commented on code in PR #761:
> URL: https://github.com/apache/tomcat/pull/761#discussion_r1794575451
>
>
> ##########
> java/org/apache/catalina/connector/Response.java:
> ##########
> @@ -1069,16 +1069,20 @@ public void sendError(int status, String message) throws IOException {
> return;
> }
>
> - setError();
> + if(103 == status) {
>
> Review Comment:
> Are we not putting a space before the parenthesis?
Ooh, I forgot to run validate before commit. I'll update that, as well
as adding at least one unit test.
-chris
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
Posted by "isapir (via GitHub)" <gi...@apache.org>.
isapir commented on code in PR #761:
URL: https://github.com/apache/tomcat/pull/761#discussion_r1794575451
##########
java/org/apache/catalina/connector/Response.java:
##########
@@ -1069,16 +1069,20 @@ public void sendError(int status, String message) throws IOException {
return;
}
- setError();
+ if(103 == status) {
Review Comment:
Are we not putting a space before the parenthesis?
--
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