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:18:49 UTC
[tomcat] branch main updated (22ceccc5a3 -> 83199ccf21)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 22ceccc5a3 Fix HEAD tests when running on Java 19+
new 43022dd099 Improve error message if a required --add-opens option is missing
new 83199ccf21 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:
.../catalina/loader/WebappClassLoaderBase.java | 15 ++++-
.../servlet/http/HttpServletDoHeadBaseTest.java | 75 ++++++++++++++++++----
webapps/docs/changelog.xml | 4 ++
3 files changed, 79 insertions(+), 15 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 main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 83199ccf21506a97f1fd1f62a11284cab0ca08ec
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 | 75 ++++++++++++++++++----
1 file changed, 63 insertions(+), 12 deletions(-)
diff --git a/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java b/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java
index c967b10bcd..cd1d0de0dd 100644
--- a/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java
+++ b/test/jakarta/servlet/http/HttpServletDoHeadBaseTest.java
@@ -19,6 +19,7 @@ package jakarta.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;
@@ -33,6 +34,9 @@ import org.junit.runners.Parameterized.Parameter;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.Wrapper;
+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.coyote.http2.Http2TestBase;
@@ -217,10 +221,13 @@ public class HttpServletDoHeadBaseTest extends Http2TestBase {
@Override
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ int originalBufferSize = resp.getBufferSize();
int adjustedBufferSize = bufferSize;
+ boolean resetBufferSize = false;
if (Boolean.parseBoolean(getServletConfig().getInitParameter(LEGACY_DO_HEAD)) &&
- useWriter && JreCompat.isJre19Available()) {
+ JreCompat.isJre19Available() && "HEAD".equals(req.getMethod()) && useWriter &&
+ resetType != ResetType.NONE) {
/*
* Using legacy HEAD handling with a Writer on Java 19+.
* HttpServlet wraps the response. The test is sensitive to
@@ -229,31 +236,48 @@ public class HttpServletDoHeadBaseTest extends Http2TestBase {
* 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);
@@ -284,10 +308,16 @@ public class HttpServletDoHeadBaseTest extends Http2TestBase {
}
case BUFFER: {
resp.resetBuffer();
+ if (resetBufferSize) {
+ resetResponseBuffer(resp, originalBufferSize);
+ }
break;
}
case FULL: {
resp.reset();
+ if (resetBufferSize) {
+ resetResponseBuffer(resp, originalBufferSize);
+ }
break;
}
}
@@ -311,6 +341,27 @@ public class HttpServletDoHeadBaseTest extends Http2TestBase {
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 main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 43022dd099019a254944874b5984c20135c8b65e
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
---
.../org/apache/catalina/loader/WebappClassLoaderBase.java | 15 ++++++++++++---
webapps/docs/changelog.xml | 4 ++++
2 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/loader/WebappClassLoaderBase.java b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
index 3376713d6f..b87cc72abe 100644
--- a/java/org/apache/catalina/loader/WebappClassLoaderBase.java
+++ b/java/org/apache/catalina/loader/WebappClassLoaderBase.java
@@ -2025,7 +2025,7 @@ public abstract class WebappClassLoaderBase extends URLClassLoader
}
} catch (InaccessibleObjectException e) {
// Must be running on without the necessary command line options.
- log.warn(sm.getString("webappClassLoader.addExportsThreadLocal", this.getClass().getModule().getName()));
+ log.warn(sm.getString("webappClassLoader.addExportsThreadLocal", getCurrentModuleName()));
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
log.warn(sm.getString(
@@ -2283,7 +2283,7 @@ public abstract class WebappClassLoaderBase extends URLClassLoader
getContextName()), e);
} catch (InaccessibleObjectException e) {
// Must be running on without the necessary command line options.
- log.warn(sm.getString("webappClassLoader.addExportsRmi", this.getClass().getModule().getName()));
+ log.warn(sm.getString("webappClassLoader.addExportsRmi", getCurrentModuleName()));
}
}
@@ -2298,11 +2298,20 @@ public abstract class WebappClassLoaderBase extends URLClassLoader
"webappClassLoader.clearObjectStreamClassCachesFail", getContextName()), e);
} catch (InaccessibleObjectException e) {
// Must be running on without the necessary command line options.
- log.warn(sm.getString("webappClassLoader.addExportsJavaIo", this.getClass().getModule().getName()));
+ log.warn(sm.getString("webappClassLoader.addExportsJavaIo", getCurrentModuleName()));
}
}
+ private String getCurrentModuleName() {
+ String moduleName = this.getClass().getModule().getName();
+ if (moduleName == null) {
+ moduleName = "ALL-UNNAMED";
+ }
+ return moduleName;
+ }
+
+
private void clearCache(Class<?> target, String mapName)
throws ReflectiveOperationException, SecurityException, ClassCastException {
Field f = target.getDeclaredField(mapName);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ca1521f30f..2b22817b79 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