Skip to content

Commit d764b57

Browse files
markt-asfmarkt-asf
committed
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.
1 parent 8cb18a6 commit d764b57

6 files changed

Lines changed: 206 additions & 9 deletions

File tree

‎java/org/apache/jasper/compiler/Generator.java‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2088,6 +2088,10 @@ private void generateCustomStart(Node.CustomTag n, TagHandlerInfo handlerInfo, S
20882088
out.print(".get(");
20892089
out.print(tagHandlerClassName);
20902090
out.println(".class);");
2091+
out.printin("boolean ");
2092+
// Used to track if the tag is re-used successfully
2093+
out.print(tagHandlerVar);
2094+
out.println("_reused = false;");
20912095
} else {
20922096
writeNewInstance(tagHandlerVar, tagHandlerClass);
20932097
}
@@ -2279,22 +2283,32 @@ private void generateCustomEnd(Node.CustomTag n, String tagHandlerVar, String ta
22792283
out.printil("}");
22802284
}
22812285

2286+
// Print tag reuse
2287+
if (usePooling(n)) {
2288+
out.printin(n.getTagHandlerPoolName());
2289+
out.print(".reuse(");
2290+
out.print(tagHandlerVar);
2291+
out.println(");");
2292+
// This code will be skipped if an exception occurs which will prevent this instance from being re-used.
2293+
out.printin(tagHandlerVar);
2294+
out.println("_reused = true;");
2295+
}
2296+
22822297
// Ensure clean-up takes place
2298+
// Use JspRuntimeLibrary to minimise code in _jspService()
22832299
out.popIndent();
22842300
out.printil("} finally {");
22852301
out.pushIndent();
2286-
2302+
out.printin("org.apache.jasper.runtime.JspRuntimeLibrary.releaseTag(");
2303+
out.print(tagHandlerVar);
2304+
out.print(", _jsp_getInstanceManager()");
22872305
if (usePooling(n)) {
2288-
// Print tag reuse
2289-
out.printin(n.getTagHandlerPoolName());
2290-
out.print(".reuse(");
2306+
// The tag will only be released for reuse if no exceptions occurred during use.
2307+
out.print(", ");
22912308
out.print(tagHandlerVar);
2292-
out.println(");");
2309+
out.println("_reused);");
22932310
} else {
2294-
// Clean-up
2295-
out.printin("org.apache.jasper.runtime.JspRuntimeLibrary.releaseTag(");
2296-
out.print(tagHandlerVar);
2297-
out.println(", _jsp_getInstanceManager());");
2311+
out.print(");");
22982312
}
22992313
out.popIndent();
23002314
out.printil("}");

‎java/org/apache/jasper/runtime/JspRuntimeLibrary.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,21 @@ public static JspWriter startBufferedBody(PageContext pageContext, BodyTag tag)
12191219
}
12201220

12211221

1222+
/**
1223+
* Releases a tag and destroys its instance it it has not been re-used.
1224+
*
1225+
* @param tag The tag to release
1226+
* @param instanceManager The instance manager
1227+
* @param reused Has the tag successfully been re-used
1228+
*/
1229+
public static void releaseTag(Tag tag, InstanceManager instanceManager, boolean reused) {
1230+
// Caller ensures pool is non-null if reuse is true
1231+
if (!reused) {
1232+
releaseTag(tag, instanceManager);
1233+
}
1234+
}
1235+
1236+
12221237
/**
12231238
* Releases a tag and destroys its instance.
12241239
*

‎test/org/apache/jasper/compiler/TestGenerator.java‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,18 @@
2323
import java.io.File;
2424
import java.io.IOException;
2525
import java.lang.reflect.Field;
26+
import java.nio.channels.IllegalSelectorException;
2627
import java.nio.charset.CodingErrorAction;
2728
import java.util.ArrayList;
2829
import java.util.Date;
30+
import java.util.HashSet;
2931
import java.util.List;
3032
import java.util.Map;
3133
import java.util.Scanner;
34+
import java.util.Set;
35+
import java.util.concurrent.atomic.AtomicInteger;
3236

37+
import jakarta.servlet.ServletRequest;
3338
import jakarta.servlet.http.HttpServletResponse;
3439
import jakarta.servlet.jsp.JspException;
3540
import jakarta.servlet.jsp.PageContext;
@@ -738,6 +743,81 @@ public void setAttribute06(Object attribute06) {
738743
}
739744
}
740745

746+
public static class TesterExceptionTag extends TagSupport {
747+
748+
private static final long serialVersionUID = 1L;
749+
750+
private static AtomicInteger counter = new AtomicInteger(0);
751+
752+
private static Set<Integer> released = new HashSet<>();
753+
754+
private final int index;
755+
756+
private String state = "";
757+
758+
public static void resetForNewTest() {
759+
counter.set(0);
760+
released.clear();
761+
}
762+
763+
public static boolean isReleased(int index) {
764+
return released.contains(Integer.valueOf(index));
765+
}
766+
767+
public TesterExceptionTag() {
768+
index = counter.incrementAndGet();
769+
}
770+
771+
@Override
772+
public int doStartTag() throws JspException {
773+
ServletRequest request = pageContext.getRequest();
774+
boolean throwOnStart = Boolean.parseBoolean(request.getParameter("throwOnStart"));
775+
if (throwOnStart) {
776+
state += "lastStartFailed";
777+
throw new IllegalSelectorException();
778+
} else {
779+
try {
780+
if (!state.isBlank()) {
781+
pageContext.getOut().print(state);
782+
pageContext.getOut().print("-");
783+
}
784+
pageContext.getOut().println("start-" + index);
785+
} catch (IOException ioe) {
786+
throw new JspException(ioe);
787+
}
788+
}
789+
return super.doStartTag();
790+
}
791+
792+
@Override
793+
public int doEndTag() throws JspException {
794+
ServletRequest request = pageContext.getRequest();
795+
boolean throwOnEnd = Boolean.parseBoolean(request.getParameter("throwOnEnd"));
796+
if (throwOnEnd) {
797+
state += "lastEndFailed";
798+
throw new IllegalSelectorException();
799+
} else {
800+
try {
801+
if (!state.isBlank()) {
802+
pageContext.getOut().print(state);
803+
pageContext.getOut().print("-");
804+
}
805+
pageContext.getOut().println("end-" + index);
806+
} catch (IOException ioe) {
807+
throw new JspException(ioe);
808+
}
809+
}
810+
return super.doEndTag();
811+
}
812+
813+
@Override
814+
public void release() {
815+
released.add(Integer.valueOf(index));
816+
super.release();
817+
}
818+
}
819+
820+
741821
@Test
742822
public void testLambdaScriptlets() throws Exception {
743823
doTestJsp("lambda.jsp");
@@ -1181,4 +1261,62 @@ public void testNonstandardRemoves() throws Exception {
11811261
+ "application value=null", body.toString());
11821262
body.recycle();
11831263
}
1264+
1265+
@Test
1266+
public void testTagReuseException01() throws Exception {
1267+
TesterExceptionTag.resetForNewTest();
1268+
// No exceptions. Check re-use.
1269+
doTestTagReuseException(null, "start-1\nend-1\n", HttpServletResponse.SC_OK);
1270+
Assert.assertFalse(TesterExceptionTag.isReleased(1));
1271+
doTestTagReuseException(null, "start-1\nend-1\n", HttpServletResponse.SC_OK);
1272+
Assert.assertFalse(TesterExceptionTag.isReleased(1));
1273+
}
1274+
1275+
@Test
1276+
public void testTagReuseException02() throws Exception {
1277+
TesterExceptionTag.resetForNewTest();
1278+
// Exception on doStartTag
1279+
doTestTagReuseException(null, "start-1\nend-1\n", HttpServletResponse.SC_OK);
1280+
Assert.assertFalse(TesterExceptionTag.isReleased(1));
1281+
doTestTagReuseException("throwOnStart=true", null, HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
1282+
Assert.assertTrue(TesterExceptionTag.isReleased(1));
1283+
// Should use new instance
1284+
doTestTagReuseException(null, "start-2\nend-2\n", HttpServletResponse.SC_OK);
1285+
Assert.assertFalse(TesterExceptionTag.isReleased(2));
1286+
doTestTagReuseException(null, "start-2\nend-2\n", HttpServletResponse.SC_OK);
1287+
Assert.assertFalse(TesterExceptionTag.isReleased(2));
1288+
}
1289+
1290+
@Test
1291+
public void testTagReuseException03() throws Exception {
1292+
TesterExceptionTag.resetForNewTest();
1293+
// Exception on doEndTag
1294+
doTestTagReuseException(null, "start-1\nend-1\n", HttpServletResponse.SC_OK);
1295+
Assert.assertFalse(TesterExceptionTag.isReleased(1));
1296+
doTestTagReuseException("throwOnEnd=true", null, HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
1297+
Assert.assertTrue(TesterExceptionTag.isReleased(1));
1298+
// Should use new instance
1299+
doTestTagReuseException(null, "start-2\nend-2\n", HttpServletResponse.SC_OK);
1300+
Assert.assertFalse(TesterExceptionTag.isReleased(2));
1301+
doTestTagReuseException(null, "start-2\nend-2\n", HttpServletResponse.SC_OK);
1302+
Assert.assertFalse(TesterExceptionTag.isReleased(2));
1303+
}
1304+
1305+
private void doTestTagReuseException(String queryString, String expectedBody, int expectedStatus) throws Exception {
1306+
if (!getTomcatInstance().getServer().getState().isAvailable()) {
1307+
getTomcatInstanceTestWebapp(false, true);
1308+
}
1309+
1310+
ByteChunk body = new ByteChunk();
1311+
String uri = "/test/jsp/generator/reuse-exception.jsp";
1312+
if (queryString != null) {
1313+
uri += "?" + queryString;
1314+
}
1315+
int rc = getUrl("http://localhost:" + getPort() + uri, body, null);
1316+
1317+
Assert.assertEquals(expectedStatus, rc);
1318+
if (expectedBody != null) {
1319+
Assert.assertEquals("Wrong body", expectedBody, body.toString());
1320+
}
1321+
}
11841322
}

‎test/webapp/WEB-INF/bugs.tld‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,11 @@
188188
<type>jakarta.el.MethodExpression</type>
189189
</attribute>
190190
</tag>
191+
<tag>
192+
<name>TesterExceptionTag</name>
193+
<tag-class>org.apache.jasper.compiler.TestGenerator$TesterExceptionTag</tag-class>
194+
<body-content>JSP</body-content>
195+
</tag>
191196
<function>
192197
<name>bug49555</name>
193198
<function-class>org.apache.el.TesterFunctions$Inner$Class</function-class>

‎test/webapp/jsp/generator/reuse-exception.jsp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<%--
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
--%><%@
17+
taglib uri="http://tomcat.apache.org/bugs" prefix="bugs"
18+
%><bugs:TesterExceptionTag/>

‎webapps/docs/changelog.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,13 @@
304304
(markt)
305305
</add>
306306
<!-- Entries for backport and removal before 12.0.0-M1 below this line -->
307+
<fix>
308+
<bug>70120</bug>: The fix for <bug>69699</bug> (itself a fix for a
309+
regression in the fix for <bug>69333</bug>) was incomplete and tags that
310+
threw exceptions in <code>doStartTag()</code> and
311+
<code>doEndTag()</code> were incorrectly re-used. This fix prevents tags
312+
from being re-used if such an exception occurs. (markt)
313+
</fix>
307314
</changelog>
308315
</subsection>
309316
<subsection name="Cluster">

0 commit comments

Comments
 (0)