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

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

commit 450c5c4ede604f99ebcc6070fe86e4e53c6bc897
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-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to