Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
ChristopherSchultz commented on PR #764: URL: https://github.com/apache/tomcat/pull/764#issuecomment-2491096139 The Servlet 6.2 spec doesn't actually exist yet... at least not in any published form. That reference is speculative, assuming that the work being done here will ultimately be released as Servlet 6.2: https://github.com/jakartaee/servlet/tree/master/spec -- 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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
Chenjp commented on PR #764: URL: https://github.com/apache/tomcat/pull/764#issuecomment-2489895080 @markt-asf @ChristopherSchultz Per HttpServletResponse#sendEarlyHints(), where can I find "Servlet 6.2" spec? ```java /** * Sends a 103 response to the client using the current response headers. This method does not commit the response * and may be called multiple times before the response is committed. The current response headers may include some * headers that have been added automatically by the container. * * This method has no effect if called after the response has been committed. * * @since Servlet 6.2 */ void sendEarlyHints(); ``` -- 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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
markt-asf merged PR #764: URL: https://github.com/apache/tomcat/pull/764 -- 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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] Allow applications to trigger sending of 103 early hints [tomcat]
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-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
