This is an automated email from the ASF dual-hosted git repository.
markt-asf 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 d764b57475 Fix BZ 70120. Don't re-use failed tags.
d764b57475 is described below
commit d764b5747590bbe2cf2e62c0460fd3deec646ed3
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Jun 23 17:29:42 2026 +0100
Fix BZ 70120. Don't re-use failed tags.
The original fix for BZ 69333 was over eager and caused a regression.
The fix for the regression (BZ 69399) was incomplete. This commit
completes the regression fix.
---
java/org/apache/jasper/compiler/Generator.java | 32 +++--
.../apache/jasper/runtime/JspRuntimeLibrary.java | 15 +++
test/org/apache/jasper/compiler/TestGenerator.java | 138 +++++++++++++++++++++
test/webapp/WEB-INF/bugs.tld | 5 +
test/webapp/jsp/generator/reuse-exception.jsp | 18 +++
webapps/docs/changelog.xml | 7 ++
6 files changed, 206 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/jasper/compiler/Generator.java
b/java/org/apache/jasper/compiler/Generator.java
index 26593e3dfa..446ac51f39 100644
--- a/java/org/apache/jasper/compiler/Generator.java
+++ b/java/org/apache/jasper/compiler/Generator.java
@@ -2088,6 +2088,10 @@ class Generator {
out.print(".get(");
out.print(tagHandlerClassName);
out.println(".class);");
+ out.printin("boolean ");
+ // Used to track if the tag is re-used successfully
+ out.print(tagHandlerVar);
+ out.println("_reused = false;");
} else {
writeNewInstance(tagHandlerVar, tagHandlerClass);
}
@@ -2279,22 +2283,32 @@ class Generator {
out.printil("}");
}
+ // Print tag reuse
+ if (usePooling(n)) {
+ out.printin(n.getTagHandlerPoolName());
+ out.print(".reuse(");
+ out.print(tagHandlerVar);
+ out.println(");");
+ // This code will be skipped if an exception occurs which will
prevent this instance from being re-used.
+ out.printin(tagHandlerVar);
+ out.println("_reused = true;");
+ }
+
// Ensure clean-up takes place
+ // Use JspRuntimeLibrary to minimise code in _jspService()
out.popIndent();
out.printil("} finally {");
out.pushIndent();
-
+
out.printin("org.apache.jasper.runtime.JspRuntimeLibrary.releaseTag(");
+ out.print(tagHandlerVar);
+ out.print(", _jsp_getInstanceManager()");
if (usePooling(n)) {
- // Print tag reuse
- out.printin(n.getTagHandlerPoolName());
- out.print(".reuse(");
+ // The tag will only be released for reuse if no exceptions
occurred during use.
+ out.print(", ");
out.print(tagHandlerVar);
- out.println(");");
+ out.println("_reused);");
} else {
- // Clean-up
-
out.printin("org.apache.jasper.runtime.JspRuntimeLibrary.releaseTag(");
- out.print(tagHandlerVar);
- out.println(", _jsp_getInstanceManager());");
+ out.print(");");
}
out.popIndent();
out.printil("}");
diff --git a/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
b/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
index 170344312b..63ea44efe7 100644
--- a/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
+++ b/java/org/apache/jasper/runtime/JspRuntimeLibrary.java
@@ -1219,6 +1219,21 @@ public class JspRuntimeLibrary {
}
+ /**
+ * Releases a tag and destroys its instance it it has not been re-used.
+ *
+ * @param tag The tag to release
+ * @param instanceManager The instance manager
+ * @param reused Has the tag successfully been re-used
+ */
+ public static void releaseTag(Tag tag, InstanceManager instanceManager,
boolean reused) {
+ // Caller ensures pool is non-null if reuse is true
+ if (!reused) {
+ releaseTag(tag, instanceManager);
+ }
+ }
+
+
/**
* Releases a tag and destroys its instance.
*
diff --git a/test/org/apache/jasper/compiler/TestGenerator.java
b/test/org/apache/jasper/compiler/TestGenerator.java
index 41bbdca59f..de8e470fc1 100644
--- a/test/org/apache/jasper/compiler/TestGenerator.java
+++ b/test/org/apache/jasper/compiler/TestGenerator.java
@@ -23,13 +23,18 @@ import java.beans.PropertyEditorSupport;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
+import java.nio.channels.IllegalSelectorException;
import java.nio.charset.CodingErrorAction;
import java.util.ArrayList;
import java.util.Date;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Scanner;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
+import jakarta.servlet.ServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.servlet.jsp.JspException;
import jakarta.servlet.jsp.PageContext;
@@ -738,6 +743,81 @@ public class TestGenerator extends TomcatBaseTest {
}
}
+ public static class TesterExceptionTag extends TagSupport {
+
+ private static final long serialVersionUID = 1L;
+
+ private static AtomicInteger counter = new AtomicInteger(0);
+
+ private static Set<Integer> released = new HashSet<>();
+
+ private final int index;
+
+ private String state = "";
+
+ public static void resetForNewTest() {
+ counter.set(0);
+ released.clear();
+ }
+
+ public static boolean isReleased(int index) {
+ return released.contains(Integer.valueOf(index));
+ }
+
+ public TesterExceptionTag() {
+ index = counter.incrementAndGet();
+ }
+
+ @Override
+ public int doStartTag() throws JspException {
+ ServletRequest request = pageContext.getRequest();
+ boolean throwOnStart =
Boolean.parseBoolean(request.getParameter("throwOnStart"));
+ if (throwOnStart) {
+ state += "lastStartFailed";
+ throw new IllegalSelectorException();
+ } else {
+ try {
+ if (!state.isBlank()) {
+ pageContext.getOut().print(state);
+ pageContext.getOut().print("-");
+ }
+ pageContext.getOut().println("start-" + index);
+ } catch (IOException ioe) {
+ throw new JspException(ioe);
+ }
+ }
+ return super.doStartTag();
+ }
+
+ @Override
+ public int doEndTag() throws JspException {
+ ServletRequest request = pageContext.getRequest();
+ boolean throwOnEnd =
Boolean.parseBoolean(request.getParameter("throwOnEnd"));
+ if (throwOnEnd) {
+ state += "lastEndFailed";
+ throw new IllegalSelectorException();
+ } else {
+ try {
+ if (!state.isBlank()) {
+ pageContext.getOut().print(state);
+ pageContext.getOut().print("-");
+ }
+ pageContext.getOut().println("end-" + index);
+ } catch (IOException ioe) {
+ throw new JspException(ioe);
+ }
+ }
+ return super.doEndTag();
+ }
+
+ @Override
+ public void release() {
+ released.add(Integer.valueOf(index));
+ super.release();
+ }
+ }
+
+
@Test
public void testLambdaScriptlets() throws Exception {
doTestJsp("lambda.jsp");
@@ -1181,4 +1261,62 @@ public class TestGenerator extends TomcatBaseTest {
+ "application value=null", body.toString());
body.recycle();
}
+
+ @Test
+ public void testTagReuseException01() throws Exception {
+ TesterExceptionTag.resetForNewTest();
+ // No exceptions. Check re-use.
+ doTestTagReuseException(null, "start-1\nend-1\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(1));
+ doTestTagReuseException(null, "start-1\nend-1\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(1));
+ }
+
+ @Test
+ public void testTagReuseException02() throws Exception {
+ TesterExceptionTag.resetForNewTest();
+ // Exception on doStartTag
+ doTestTagReuseException(null, "start-1\nend-1\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(1));
+ doTestTagReuseException("throwOnStart=true", null,
HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ Assert.assertTrue(TesterExceptionTag.isReleased(1));
+ // Should use new instance
+ doTestTagReuseException(null, "start-2\nend-2\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(2));
+ doTestTagReuseException(null, "start-2\nend-2\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(2));
+ }
+
+ @Test
+ public void testTagReuseException03() throws Exception {
+ TesterExceptionTag.resetForNewTest();
+ // Exception on doEndTag
+ doTestTagReuseException(null, "start-1\nend-1\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(1));
+ doTestTagReuseException("throwOnEnd=true", null,
HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ Assert.assertTrue(TesterExceptionTag.isReleased(1));
+ // Should use new instance
+ doTestTagReuseException(null, "start-2\nend-2\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(2));
+ doTestTagReuseException(null, "start-2\nend-2\n",
HttpServletResponse.SC_OK);
+ Assert.assertFalse(TesterExceptionTag.isReleased(2));
+ }
+
+ private void doTestTagReuseException(String queryString, String
expectedBody, int expectedStatus) throws Exception {
+ if (!getTomcatInstance().getServer().getState().isAvailable()) {
+ getTomcatInstanceTestWebapp(false, true);
+ }
+
+ ByteChunk body = new ByteChunk();
+ String uri = "/test/jsp/generator/reuse-exception.jsp";
+ if (queryString != null) {
+ uri += "?" + queryString;
+ }
+ int rc = getUrl("http://localhost:"; + getPort() + uri, body, null);
+
+ Assert.assertEquals(expectedStatus, rc);
+ if (expectedBody != null) {
+ Assert.assertEquals("Wrong body", expectedBody, body.toString());
+ }
+ }
}
diff --git a/test/webapp/WEB-INF/bugs.tld b/test/webapp/WEB-INF/bugs.tld
index 99b5f18f8c..a9be9e1f41 100644
--- a/test/webapp/WEB-INF/bugs.tld
+++ b/test/webapp/WEB-INF/bugs.tld
@@ -188,6 +188,11 @@
<type>jakarta.el.MethodExpression</type>
</attribute>
</tag>
+ <tag>
+ <name>TesterExceptionTag</name>
+
<tag-class>org.apache.jasper.compiler.TestGenerator$TesterExceptionTag</tag-class>
+ <body-content>JSP</body-content>
+ </tag>
<function>
<name>bug49555</name>
<function-class>org.apache.el.TesterFunctions$Inner$Class</function-class>
diff --git a/test/webapp/jsp/generator/reuse-exception.jsp
b/test/webapp/jsp/generator/reuse-exception.jsp
new file mode 100644
index 0000000000..eaaf67ace6
--- /dev/null
+++ b/test/webapp/jsp/generator/reuse-exception.jsp
@@ -0,0 +1,18 @@
+<%--
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+--%><%@
+taglib uri="http://tomcat.apache.org/bugs"; prefix="bugs"
+%><bugs:TesterExceptionTag/>
\ No newline at end of file
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8c0734370b..dbcc9c2216 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -304,6 +304,13 @@
(markt)
</add>
<!-- Entries for backport and removal before 12.0.0-M1 below this line
-->
+ <fix>
+ <bug>70120</bug>: The fix for <bug>69699</bug> (itself a fix for a
+ regression in the fix for <bug>69333</bug>) was incomplete and tags
that
+ threw exceptions in <code>doStartTag()</code> and
+ <code>doEndTag()</code> were incorrectly re-used. This fix prevents
tags
+ from being re-used if such an exception occurs. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Cluster">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
