You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/05/18 17:30:27 UTC

[tomcat] branch 9.0.x updated (d139685518 -> 0353f057bc)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


    from d139685518 Fix backport
     new 2d1c1deeed Improve error message if a required --add-opens option is missing
     new 0353f057bc Further improvements to HEAD tests

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 java/org/apache/tomcat/util/compat/Jre9Compat.java |  6 +-
 .../servlet/http/HttpServletDoHeadBaseTest.java    | 76 ++++++++++++++++++----
 webapps/docs/changelog.xml                         |  4 ++
 3 files changed, 73 insertions(+), 13 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/02: Further improvements to HEAD tests

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 0353f057bc1e00eb99fcb1a543de5820952350c6
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 18 18:13:51 2022 +0100

    Further improvements to HEAD tests
    
    Try and maintain the original intention of the tests while working
    around the issues created by the encoder changes in Java 19+
---
 .../servlet/http/HttpServletDoHeadBaseTest.java    | 76 ++++++++++++++++++----
 1 file changed, 64 insertions(+), 12 deletions(-)

diff --git a/test/javax/servlet/http/HttpServletDoHeadBaseTest.java b/test/javax/servlet/http/HttpServletDoHeadBaseTest.java
index 1317502c93..6bdb12d134 100644
--- a/test/javax/servlet/http/HttpServletDoHeadBaseTest.java
+++ b/test/javax/servlet/http/HttpServletDoHeadBaseTest.java
@@ -19,6 +19,8 @@ package javax.servlet.http;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintWriter;
+import java.lang.reflect.Field;
+import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.List;
@@ -30,6 +32,9 @@ import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runners.Parameterized.Parameter;
 
+import org.apache.catalina.connector.OutputBuffer;
+import org.apache.catalina.connector.Response;
+import org.apache.catalina.connector.ResponseFacade;
 import org.apache.catalina.core.StandardContext;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
@@ -137,9 +142,12 @@ public class HttpServletDoHeadBaseTest extends TomcatBaseTest {
         @Override
         protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
 
+            int originalBufferSize = resp.getBufferSize();
             int adjustedBufferSize = bufferSize;
+            boolean resetBufferSize = false;
 
-            if (useWriter && JreCompat.isJre19Available()) {
+            if (JreCompat.isJre19Available() && "HEAD".equals(req.getMethod()) && useWriter &&
+                    resetType != ResetType.NONE) {
                 /*
                  * Using legacy (non-legacy isn't available until Servlet 6.0 /
                  * Tomcat 10.1.x) HEAD handling with a Writer on Java 19+.
@@ -149,31 +157,48 @@ public class HttpServletDoHeadBaseTest extends TomcatBaseTest {
                  * characters that can be written before the response is
                  * committed needs to be the same.
                  *
-                 * Internally, Tomcat the response buffer is at least 8192
+                 * Internally, the Tomcat response buffer defaults to 8192
                  * bytes. When a Writer is used, an additional 8192 byte buffer
-                 * is used for char -> byte.
+                 * is used for char->byte.
                  *
-                 * With Java <19, the char -> byte buffer is created as a 8192
+                 * With Java <19, the char->byte buffer used by HttpServlet
+                 * processing HEAD requests in legacy mode is created as a 8192
                  * byte buffer which is consistent with the buffer Tomcat uses
                  * internally.
                  *
-                 * With Java 19+, the char -> byte buffer is created as a 512
-                 * byte buffer. Need to adjust the size of the response buffer
-                 * to account for the smaller char -> byte buffer.
+                 * With Java 19+, the char->byte buffer used by HttpServlet
+                 * processing HEAD requests in legacy mode is created as a 512
+                 * byte buffer.
+                 *
+                 * If the response isn't reset, none of this matters as it is
+                 * just the size of the response buffer and the size of the
+                 * response that determines if chunked encoding is used.
+                 * However, if the response is reset then things get interesting
+                 * as the amount of response data that can be written before
+                 * committing the response is the combination of the char->byte
+                 * buffer and the response buffer. Because the char->byte buffer
+                 * size in legacy mode varies with Java version, the size of the
+                 * response buffer needs to be adjusted to compensate so that
+                 * both GET and HEAD can write the same amount of data before
+                 * committing the response. To make matters worse, to obtain the
+                 * correct behaviour the response buffer size needs to be reset
+                 * back to 8192 after the reset.
                  *
                  * This is why the legacy mode is problematic and the new
                  * approach of the container handling HEAD is better.
                  */
 
-                // Response buffer is always no smaller than 8192 bytes
-                if (adjustedBufferSize < 8192) {
-                    adjustedBufferSize = 8192;
+                // Response buffer is always no smaller than originalBufferSize
+                if (adjustedBufferSize < originalBufferSize) {
+                    adjustedBufferSize = originalBufferSize;
                 }
 
                 // Adjust for smaller initial char -> byte buffer in Java 19+
-                // 8192 initial size in Java <19
+                // originalBufferSize initial size in Java <19
                 // 512 initial size in Java 19+
-                adjustedBufferSize = adjustedBufferSize + 8192 - 512;
+                adjustedBufferSize = adjustedBufferSize + originalBufferSize - 512;
+
+                resetBufferSize = true;
             }
 
             resp.setBufferSize(adjustedBufferSize);
@@ -204,10 +229,16 @@ public class HttpServletDoHeadBaseTest extends TomcatBaseTest {
                     }
                     case BUFFER: {
                         resp.resetBuffer();
+                        if (resetBufferSize) {
+                            resetResponseBuffer(resp, originalBufferSize);
+                        }
                         break;
                     }
                     case FULL: {
                         resp.reset();
+                        if (resetBufferSize) {
+                            resetResponseBuffer(resp, originalBufferSize);
+                        }
                         break;
                     }
                 }
@@ -231,6 +262,27 @@ public class HttpServletDoHeadBaseTest extends TomcatBaseTest {
                 os.write(msg.getBytes(StandardCharsets.UTF_8));
             }
         }
+
+        private void resetResponseBuffer(HttpServletResponse resp, int size) throws ServletException {
+            // This bypasses various checks but that is OK in this case.
+            try {
+                ResponseFacade responseFacade = (ResponseFacade) ((HttpServletResponseWrapper) resp).getResponse();
+
+                Field responseField = ResponseFacade.class.getDeclaredField("response");
+                responseField.setAccessible(true);
+                Response response = (Response) responseField.get(responseFacade);
+
+                Field outputBufferField = Response.class.getDeclaredField("outputBuffer");
+                outputBufferField.setAccessible(true);
+                OutputBuffer outputBuffer = (OutputBuffer) outputBufferField.get(response);
+
+                Field byteBufferField = OutputBuffer.class.getDeclaredField("bb");
+                byteBufferField.setAccessible(true);
+                byteBufferField.set(outputBuffer, ByteBuffer.allocate(Math.max(size, bufferSize)));
+            } catch (NoSuchFieldException | IllegalArgumentException | IllegalAccessException e) {
+                throw new ServletException(e);
+            }
+        }
     }
 
 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/02: Improve error message if a required --add-opens option is missing

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 2d1c1deeed5dbca097e3b1dee7fb928dac6df813
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed May 18 17:08:08 2022 +0100

    Improve error message if a required --add-opens option is missing
---
 java/org/apache/tomcat/util/compat/Jre9Compat.java | 6 +++++-
 webapps/docs/changelog.xml                         | 4 ++++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/java/org/apache/tomcat/util/compat/Jre9Compat.java b/java/org/apache/tomcat/util/compat/Jre9Compat.java
index 87a3c385c0..86fba5eb30 100644
--- a/java/org/apache/tomcat/util/compat/Jre9Compat.java
+++ b/java/org/apache/tomcat/util/compat/Jre9Compat.java
@@ -253,7 +253,11 @@ class Jre9Compat extends JreCompat {
     public String getModuleName(Class<?> type) {
         try {
             Object module = getModuleMethod.invoke(type);
-            return (String) getNameMethod.invoke(module);
+            String moduleName = (String) getNameMethod.invoke(module);
+            if (moduleName == null) {
+                moduleName = "ALL-UNNAMED";
+            }
+            return moduleName;
         } catch (ReflectiveOperationException e) {
             return "ERROR";
         }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 1781bc212f..df5dc3da15 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -111,6 +111,10 @@
         Update the memory leak protection code to support stopping application
         created executor threads when running on Java 19 and later. (markt)
       </fix>
+      <fix>
+        Improve the error message if a required <code>--add-opens</code> option
+        is missing. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org