Re: Future of JNI in Tomcat
On Tue, Dec 10, 2024 at 1:01 PM Rainer Jung wrote: > > Am 12.09.24 um 16:15 schrieb Rémy Maucherat: > > This JEP has the potential to have a significant impact with Tomcat's > > JNI use starting with Java 26. > > https://openjdk.org/jeps/471 > > > > Unsafe.invokeCleaner will be removed, which will effectively prevent > > using the direct ByteBuffers that are needed for tomcat-native. The > > solution is to use a memory segment from FFM, then call > > MemorySegment.asByteBuffer, which creates a direct ByteBuffer with a > > controllable lifecycle. So using JNI would require FFM and using the > > full FFM code instead should make more sense. > > Recent Java 24 EA now issues: > > WARNING: A terminally deprecated method in sun.misc.Unsafe has been called > WARNING: sun.misc.Unsafe::invokeCleaner has been called by > org.apache.tomcat.util.buf.ByteBufferUtils (file:/.../lib/tomcat-util.jar) > WARNING: Please consider reporting this to the maintainers of class > org.apache.tomcat.util.buf.ByteBufferUtils > WARNING: sun.misc.Unsafe::invokeCleaner will be removed in a future release Ok, so: - Don't enable direct buffers on the connectors. This is not the default so it should be fine. Eventually that feature would have to be removed. Maybe as soon as Tomcat 12. - If using Java 24+, use FFM instead of tomcat-native (since tomcat-native needs some direct byte buffers). This seems to work. It would of course be a lot more comfortable if FFM was in Java 21. Rémy - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
Chenjp commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1879094689 ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -726,10 +727,79 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp) throws */ protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { +String ifMatchHeader = request.getHeader("If-Match"); +String ifUnmodifiedSinceHeader = request.getHeader("If-Unmodified-Since"); +String ifNoneMatchHeader = request.getHeader("If-None-Match"); +String ifModifiedSinceHeader = request.getHeader("If-Modified-Since"); +String ifRangeHeader = request.getHeader("If-Range"); +String rangeHeader = request.getHeader("Range"); -return checkIfMatch(request, response, resource) && checkIfModifiedSince(request, response, resource) && -checkIfNoneMatch(request, response, resource) && checkIfUnmodifiedSince(request, response, resource); - +// RFC9110 #13.3.2 defines preconditions evaluation order +int next = 1; +while (next < 6) { Review Comment: it does follow rfc spec strictly. -- 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
(tomcat) branch main updated: Add javadoc about the warning
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/main by this push: new 23f2d1a864 Add javadoc about the warning 23f2d1a864 is described below commit 23f2d1a864ade6a6df64a5b6f44643f982ca9052 Author: remm AuthorDate: Tue Dec 10 13:32:49 2024 +0100 Add javadoc about the warning --- java/org/apache/tomcat/util/buf/ByteBufferUtils.java | 4 1 file changed, 4 insertions(+) diff --git a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java index 552ce94c3a..6ae744db2a 100644 --- a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java +++ b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java @@ -94,6 +94,10 @@ public class ByteBufferUtils { return out; } +/** + * Clean specified direct buffer. This will cause an unavoidable warning on Java 24 and newer. + * @param buf the buffer to clean + */ public static void cleanDirectBuffer(ByteBuffer buf) { if (invokeCleanerMethod != null) { try { - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 10.1.x updated: Add javadoc about the warning
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/10.1.x by this push: new 05d5ad90c0 Add javadoc about the warning 05d5ad90c0 is described below commit 05d5ad90c053962b2423837e7e5276aec1a5f655 Author: remm AuthorDate: Tue Dec 10 13:32:49 2024 +0100 Add javadoc about the warning --- java/org/apache/tomcat/util/buf/ByteBufferUtils.java | 4 1 file changed, 4 insertions(+) diff --git a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java index 552ce94c3a..6ae744db2a 100644 --- a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java +++ b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java @@ -94,6 +94,10 @@ public class ByteBufferUtils { return out; } +/** + * Clean specified direct buffer. This will cause an unavoidable warning on Java 24 and newer. + * @param buf the buffer to clean + */ public static void cleanDirectBuffer(ByteBuffer buf) { if (invokeCleanerMethod != null) { try { - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 11.0.x updated: Add javadoc about the warning
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/11.0.x by this push: new ed62a24e73 Add javadoc about the warning ed62a24e73 is described below commit ed62a24e73332323512d0d670f5926dee3cf5eeb Author: remm AuthorDate: Tue Dec 10 13:32:49 2024 +0100 Add javadoc about the warning --- java/org/apache/tomcat/util/buf/ByteBufferUtils.java | 4 1 file changed, 4 insertions(+) diff --git a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java index 552ce94c3a..6ae744db2a 100644 --- a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java +++ b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java @@ -94,6 +94,10 @@ public class ByteBufferUtils { return out; } +/** + * Clean specified direct buffer. This will cause an unavoidable warning on Java 24 and newer. + * @param buf the buffer to clean + */ public static void cleanDirectBuffer(ByteBuffer buf) { if (invokeCleanerMethod != null) { try { - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat) branch 9.0.x updated: Add javadoc about the warning
This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git The following commit(s) were added to refs/heads/9.0.x by this push: new 972e33b95f Add javadoc about the warning 972e33b95f is described below commit 972e33b95f1de447084b86e0f1076e43506857bd Author: remm AuthorDate: Tue Dec 10 13:32:49 2024 +0100 Add javadoc about the warning --- java/org/apache/tomcat/util/buf/ByteBufferUtils.java | 4 1 file changed, 4 insertions(+) diff --git a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java index 4593fabbad..e082fb36a4 100644 --- a/java/org/apache/tomcat/util/buf/ByteBufferUtils.java +++ b/java/org/apache/tomcat/util/buf/ByteBufferUtils.java @@ -116,6 +116,10 @@ public class ByteBufferUtils { return out; } +/** + * Clean specified direct buffer. This will cause an unavoidable warning on Java 24 and newer. + * @param buf the buffer to clean + */ public static void cleanDirectBuffer(ByteBuffer buf) { if (cleanMethod != null) { try { - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
rmaucher commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1878573149 ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -726,10 +727,79 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp) throws */ protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { +String ifMatchHeader = request.getHeader("If-Match"); +String ifUnmodifiedSinceHeader = request.getHeader("If-Unmodified-Since"); +String ifNoneMatchHeader = request.getHeader("If-None-Match"); +String ifModifiedSinceHeader = request.getHeader("If-Modified-Since"); +String ifRangeHeader = request.getHeader("If-Range"); +String rangeHeader = request.getHeader("Range"); -return checkIfMatch(request, response, resource) && checkIfModifiedSince(request, response, resource) && -checkIfNoneMatch(request, response, resource) && checkIfUnmodifiedSince(request, response, resource); - +// RFC9110 #13.3.2 defines preconditions evaluation order +int next = 1; +while (next < 6) { Review Comment: I have the impression it works, but the whole thing looks weird. ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -2092,36 +2139,51 @@ protected boolean checkSendfile(HttpServletRequest request, HttpServletResponse protected boolean checkIfMatch(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { -String headerValue = request.getHeader("If-Match"); -if (headerValue != null) { +String resourceETag = generateETag(resource); +if (resourceETag == null) { +// if a current representation for the target resource is not present +response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED); +return false; +} -boolean conditionSatisfied; +int headerCount = Collections.list(request.getHeaders("If-Match")).size(); Review Comment: This check should be simplified with !headerValues.hasMoreElements() ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -2176,15 +2240,40 @@ protected boolean checkIfModifiedSince(HttpServletRequest request, HttpServletRe protected boolean checkIfNoneMatch(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { -String headerValue = request.getHeader("If-None-Match"); -if (headerValue != null) { +String resourceETag = generateETag(resource); +int headerCount = Collections.list(request.getHeaders("If-None-Match")).size(); Review Comment: Same. -- 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
Buildbot failure in on tomcat-9.0.x
Build status: BUILD FAILED: failed compile (failure) Worker used: bb_worker2_ubuntu URL: https://ci2.apache.org/#builders/37/builds/1211 Blamelist: remm Build Text: failed compile (failure) Status Detected: new failure Build Source Stamp: [branch 9.0.x] 972e33b95f1de447084b86e0f1076e43506857bd Steps: worker_preparation: 0 git: 0 shell: 0 shell_1: 0 shell_2: 0 shell_3: 0 shell_4: 0 shell_5: 0 compile: 1 shell_6: 0 shell_7: 0 shell_8: 0 shell_9: 0 Rsync docs to nightlies.apache.org: 0 shell_10: 0 Rsync RAT to nightlies.apache.org: 0 compile_1: 2 shell_11: 0 Rsync Logs to nightlies.apache.org: 0 -- ASF Buildbot - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
rmaucher commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1877654377 ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -718,18 +719,89 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp) throws * @param request The servlet request we are processing * @param response The servlet response we are creating * @param resource The resource + * @param resourceETag The resource etag + * @param resourceLastModified The last modification time in milliseconds of the resource, -1 if not available. * * @return true if the resource meets all the specified conditions, and false if any of * the conditions is not satisfied, in which case request processing is stopped * * @throws IOException an IO error occurred */ -protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource) -throws IOException { - -return checkIfMatch(request, response, resource) && checkIfModifiedSince(request, response, resource) && -checkIfNoneMatch(request, response, resource) && checkIfUnmodifiedSince(request, response, resource); +protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource, +String resourceETag, long resourceLastModified) throws IOException { Review Comment: -1 for the API change -- 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
[PR] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
Chenjp opened a new pull request, #796: URL: https://github.com/apache/tomcat/pull/796 1. preconditions evaluation implementation comply spec, as rfc 9110 section 13.2.2. defined. 2. fix BZ69492; 3. since resource etag and lastmodified are used often during checkIfXXX, then make them parameterized. -- 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] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
Chenjp commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1877684200 ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -718,18 +719,89 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp) throws * @param request The servlet request we are processing * @param response The servlet response we are creating * @param resource The resource + * @param resourceETag The resource etag + * @param resourceLastModified The last modification time in milliseconds of the resource, -1 if not available. * * @return true if the resource meets all the specified conditions, and false if any of * the conditions is not satisfied, in which case request processing is stopped * * @throws IOException an IO error occurred */ -protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource) -throws IOException { - -return checkIfMatch(request, response, resource) && checkIfModifiedSince(request, response, resource) && -checkIfNoneMatch(request, response, resource) && checkIfUnmodifiedSince(request, response, resource); +protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource, +String resourceETag, long resourceLastModified) throws IOException { Review Comment: Okay, agree -- 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] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
rmaucher commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1877944306 ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -718,18 +719,89 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp) throws * @param request The servlet request we are processing * @param response The servlet response we are creating * @param resource The resource + * @param resourceETag The resource etag + * @param resourceLastModified The last modification time in milliseconds of the resource, -1 if not available. * * @return true if the resource meets all the specified conditions, and false if any of * the conditions is not satisfied, in which case request processing is stopped * * @throws IOException an IO error occurred */ -protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource) -throws IOException { - -return checkIfMatch(request, response, resource) && checkIfModifiedSince(request, response, resource) && -checkIfNoneMatch(request, response, resource) && checkIfUnmodifiedSince(request, response, resource); +protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource, +String resourceETag, long resourceLastModified) throws IOException { Review Comment: Good. -- 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] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
Chenjp commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1877937586 ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -718,18 +719,89 @@ protected void doDelete(HttpServletRequest req, HttpServletResponse resp) throws * @param request The servlet request we are processing * @param response The servlet response we are creating * @param resource The resource + * @param resourceETag The resource etag + * @param resourceLastModified The last modification time in milliseconds of the resource, -1 if not available. * * @return true if the resource meets all the specified conditions, and false if any of * the conditions is not satisfied, in which case request processing is stopped * * @throws IOException an IO error occurred */ -protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource) -throws IOException { - -return checkIfMatch(request, response, resource) && checkIfModifiedSince(request, response, resource) && -checkIfNoneMatch(request, response, resource) && checkIfUnmodifiedSince(request, response, resource); +protected boolean checkIfHeaders(HttpServletRequest request, HttpServletResponse response, WebResource resource, +String resourceETag, long resourceLastModified) throws IOException { Review Comment: > -1 for the API change revert api change. -- 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: Future of JNI in Tomcat
Am 12.09.24 um 16:15 schrieb Rémy Maucherat: This JEP has the potential to have a significant impact with Tomcat's JNI use starting with Java 26. https://openjdk.org/jeps/471 Unsafe.invokeCleaner will be removed, which will effectively prevent using the direct ByteBuffers that are needed for tomcat-native. The solution is to use a memory segment from FFM, then call MemorySegment.asByteBuffer, which creates a direct ByteBuffer with a controllable lifecycle. So using JNI would require FFM and using the full FFM code instead should make more sense. Recent Java 24 EA now issues: WARNING: A terminally deprecated method in sun.misc.Unsafe has been called WARNING: sun.misc.Unsafe::invokeCleaner has been called by org.apache.tomcat.util.buf.ByteBufferUtils (file:/.../lib/tomcat-util.jar) WARNING: Please consider reporting this to the maintainers of class org.apache.tomcat.util.buf.ByteBufferUtils WARNING: sun.misc.Unsafe::invokeCleaner will be removed in a future release Best regards, Rainer - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [PR] fix BZ69492, Conditional requests comply rfc9110 [tomcat]
Chenjp commented on code in PR #796: URL: https://github.com/apache/tomcat/pull/796#discussion_r1879213975 ## java/org/apache/catalina/servlets/DefaultServlet.java: ## @@ -2092,36 +2139,51 @@ protected boolean checkSendfile(HttpServletRequest request, HttpServletResponse protected boolean checkIfMatch(HttpServletRequest request, HttpServletResponse response, WebResource resource) throws IOException { -String headerValue = request.getHeader("If-Match"); -if (headerValue != null) { +String resourceETag = generateETag(resource); +if (resourceETag == null) { +// if a current representation for the target resource is not present +response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED); +return false; +} -boolean conditionSatisfied; +int headerCount = Collections.list(request.getHeaders("If-Match")).size(); Review Comment: It is useful later: ```java if (hasAsteriskValue && headerCount > 1) { ``` @rmaucher another replacement would be: ```java List headerValues = Collections.list(request.getHeaders("If-Match")); int headerCount = headerValues.size(); if (headerCount == 0) { return true; } boolean conditionSatisfied = false; boolean hasAsteriskValue = false;// check existence of special header value '*' for (String headerValue:headerValues) { if(conditionSatisfied) { break; } if ("*".equals(headerValue)) { hasAsteriskValue = true; conditionSatisfied = true; } else { // RFC 7232 requires strong comparison for If-Match headers Boolean matched = EntityTag.compareEntityTag(new StringReader(headerValue), false, resourceETag); if (matched == null) { if (debug > 10) { log("DefaultServlet.checkIfMatch: Invalid header value [" + headerValue + "]"); } response.sendError(HttpServletResponse.SC_BAD_REQUEST); return false; } else { conditionSatisfied = matched.booleanValue(); } } } if (hasAsteriskValue && headerCount > 1) { // Note that an If-Match header field with a list value containing "*" and other values (including other // instances of "*") is syntactically invalid (therefore not allowed to be generated) and furthermore is // unlikely to be interoperable. response.sendError(HttpServletResponse.SC_BAD_REQUEST); return false; } if (!conditionSatisfied) { response.sendError(HttpServletResponse.SC_PRECONDITION_FAILED); return false; } ``` -- 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