This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2a0b5a1cd0a1dc8221c35ca3560883eeca14fb3b Author: opencode <[email protected]> AuthorDate: Mon Oct 19 13:53:30 2026 +0200 Fall back when c:when or c:otherwise has no choose parent The When plugin only stepped aside when its direct parent had no tag plugin context at all. Any other plugin-enabled direct parent, such as the default-registered c:forEach, passed the check, and the plugin emitted its opening if statement inside the parent's generated construct, leaving the brace chain unbalanced until the javac of the whole page failed with a syntax error at an unrelated line. The Otherwise plugin generated its "} else {" without any parent check at all, with the same result. The JSTL TLD validator does not reject this nesting, so the page reached code generation. Mark the Choose plugin context and have When and Otherwise fall back to the standard tag handlers unless their direct parent carries the mark. The standard handlers perform the same direct-parent check as the reference implementation and report the illegal nesting with a clean runtime JspTagException instead of failing the translation of the entire page. Add tests covering the misplaced tags and a test pinning that valid nesting still uses the plugins. Found by code review. --- .../apache/jasper/tagplugins/jstl/core/Choose.java | 4 ++ .../jasper/tagplugins/jstl/core/Otherwise.java | 9 ++++ .../apache/jasper/tagplugins/jstl/core/When.java | 12 ++++++ .../jasper/tagplugins/jstl/core/TestChoose.java | 35 ++++++++------- .../jasper/tagplugins/jstl/core/TestOtherwise.java | 47 ++++++++++++++++++++ .../jasper/tagplugins/jstl/core/TestWhen.java | 50 ++++++++++++++++++++++ test/webapp/jstl/choose-basic.jsp | 24 +++++++++++ test/webapp/jstl/otherwise-outside-choose.jsp | 22 ++++++++++ test/webapp/jstl/when-outside-choose.jsp | 22 ++++++++++ webapps/docs/changelog.xml | 10 +++++ 10 files changed, 217 insertions(+), 18 deletions(-) diff --git a/java/org/apache/jasper/tagplugins/jstl/core/Choose.java b/java/org/apache/jasper/tagplugins/jstl/core/Choose.java index 60e5d6e9bd..bfbfd758a5 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/Choose.java +++ b/java/org/apache/jasper/tagplugins/jstl/core/Choose.java @@ -33,9 +33,13 @@ public final class Choose implements TagPlugin { @Override public void doTag(TagPluginContext ctxt) { + // Mark this context so that the <c:when> and <c:otherwise> tags in + // the body know that they are directly nested in a <c:choose>. + // Not much to do here, much of the work will be done in the // containing tags, <c:when> and <c:otherwise>. + ctxt.setPluginAttribute("inChoose", "true"); ctxt.generateBody(); // See comments in When.java for the reason "}" is generated here. ctxt.generateJavaSource("}"); diff --git a/java/org/apache/jasper/tagplugins/jstl/core/Otherwise.java b/java/org/apache/jasper/tagplugins/jstl/core/Otherwise.java index 0e1217de4c..8d143b1c69 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/Otherwise.java +++ b/java/org/apache/jasper/tagplugins/jstl/core/Otherwise.java @@ -33,6 +33,15 @@ public final class Otherwise implements TagPlugin { @Override public void doTag(TagPluginContext ctxt) { + // See When.java. An otherwise outside a plugin-enabled choose must + // not generate a stray "} else {". + TagPluginContext parentContext = ctxt.getParentContext(); + if (parentContext == null || + !"true".equals(parentContext.getPluginAttribute("inChoose"))) { + ctxt.dontUseTagPlugin(); + return; + } + // See When.java for the reason why "}" is needed at the beginning and // not at the end. ctxt.generateJavaSource("} else {"); diff --git a/java/org/apache/jasper/tagplugins/jstl/core/When.java b/java/org/apache/jasper/tagplugins/jstl/core/When.java index e8766fd609..bd84624c29 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/When.java +++ b/java/org/apache/jasper/tagplugins/jstl/core/When.java @@ -39,6 +39,18 @@ public final class When implements TagPlugin { return; } + /* + * The plugin-enabled parent is not a <c:choose>, for example a + * <c:forEach>, which the TLD validator does not reject. The generated + * if/else chain would be unbalanced and the whole page would fail to + * translate. Fall back to the standard tag handler, which reports the + * illegal nesting at runtime. + */ + if (!"true".equals(parentContext.getPluginAttribute("inChoose"))) { + ctxt.dontUseTagPlugin(); + return; + } + if ("true".equals(parentContext.getPluginAttribute("hasBeenHere"))) { ctxt.generateJavaSource("} else if("); // See comment below for the reason we generate the extra "}" here. diff --git a/java/org/apache/jasper/tagplugins/jstl/core/Choose.java b/test/org/apache/jasper/tagplugins/jstl/core/TestChoose.java similarity index 56% copy from java/org/apache/jasper/tagplugins/jstl/core/Choose.java copy to test/org/apache/jasper/tagplugins/jstl/core/TestChoose.java index 60e5d6e9bd..548e963d87 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/Choose.java +++ b/test/org/apache/jasper/tagplugins/jstl/core/TestChoose.java @@ -16,28 +16,27 @@ */ package org.apache.jasper.tagplugins.jstl.core; -import org.apache.jasper.compiler.tagplugin.TagPlugin; -import org.apache.jasper.compiler.tagplugin.TagPluginContext; +import jakarta.servlet.http.HttpServletResponse; -/** - * Tag plugin for the JSTL <c:choose> tag. - */ -public final class Choose implements TagPlugin { +import org.junit.Assert; +import org.junit.Test; - /** - * Creates a new Choose tag plugin. - */ - public Choose() { - } +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestChoose extends AbstractTestTag { + + @Test + public void testChooseWhenOtherwise() throws Exception { + ByteChunk res = new ByteChunk(); - @Override - public void doTag(TagPluginContext ctxt) { + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/choose-basic.jsp", res, null); - // Not much to do here, much of the work will be done in the - // containing tags, <c:when> and <c:otherwise>. + Assert.assertEquals(HttpServletResponse.SC_OK, rc); - ctxt.generateBody(); - // See comments in When.java for the reason "}" is generated here. - ctxt.generateJavaSource("}"); + String body = res.toString(); + Assert.assertTrue(body.contains("other-1")); + Assert.assertTrue(body.contains("two")); + Assert.assertTrue(body.contains("three")); } } diff --git a/test/org/apache/jasper/tagplugins/jstl/core/TestOtherwise.java b/test/org/apache/jasper/tagplugins/jstl/core/TestOtherwise.java new file mode 100644 index 0000000000..783ab93df2 --- /dev/null +++ b/test/org/apache/jasper/tagplugins/jstl/core/TestOtherwise.java @@ -0,0 +1,47 @@ +/* + * 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. + */ +package org.apache.jasper.tagplugins.jstl.core; + +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestOtherwise extends AbstractTestTag { + + /* + * A <c:otherwise> whose direct parent is not a <c:choose> must not + * generate a stray "} else {", which would fail the translation of the + * whole page. See TestWhen for the equivalent <c:when> case. + */ + @Test + public void testOutsideChoose() throws Exception { + ByteChunk res = new ByteChunk(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/otherwise-outside-choose.jsp", res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + + String body = res.toString(); + Assert.assertTrue(body, body.contains("JspTagException")); + Assert.assertTrue(body, body.contains("style tag without")); + Assert.assertTrue(body, body.contains("as its direct parent")); + } +} diff --git a/test/org/apache/jasper/tagplugins/jstl/core/TestWhen.java b/test/org/apache/jasper/tagplugins/jstl/core/TestWhen.java new file mode 100644 index 0000000000..b64694fc3a --- /dev/null +++ b/test/org/apache/jasper/tagplugins/jstl/core/TestWhen.java @@ -0,0 +1,50 @@ +/* + * 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. + */ +package org.apache.jasper.tagplugins.jstl.core; + +import jakarta.servlet.http.HttpServletResponse; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.tomcat.util.buf.ByteChunk; + +public class TestWhen extends AbstractTestTag { + + /* + * A <c:when> whose direct parent is not a <c:choose> (but is another + * plugin-enabled tag) must not generate an unbalanced if statement, which + * would fail the translation of the whole page. The plugin must step + * aside so that the standard tag handler reports the illegal nesting at + * runtime, as the reference implementation does. The TLD validator does + * not reject this nesting. + */ + @Test + public void testOutsideChoose() throws Exception { + ByteChunk res = new ByteChunk(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/when-outside-choose.jsp", res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + + String body = res.toString(); + Assert.assertTrue(body, body.contains("JspTagException")); + Assert.assertTrue(body, body.contains("style tag without")); + Assert.assertTrue(body, body.contains("as its direct parent")); + } +} diff --git a/test/webapp/jstl/choose-basic.jsp b/test/webapp/jstl/choose-basic.jsp new file mode 100644 index 0000000000..fd046a128c --- /dev/null +++ b/test/webapp/jstl/choose-basic.jsp @@ -0,0 +1,24 @@ +<%-- + 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. +--%> +<%@ page session="false" contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +<%@ taglib uri="http://java.sun.com/jsp/jstl/core"; prefix="c" %> +<c:forEach items="${[1,2,3]}" var="n"><c:choose> + <c:when test="${n == 2}">two </c:when> + <c:when test="${n == 3}">three </c:when> + <c:otherwise>other-${n} </c:otherwise> +</c:choose></c:forEach> diff --git a/test/webapp/jstl/otherwise-outside-choose.jsp b/test/webapp/jstl/otherwise-outside-choose.jsp new file mode 100644 index 0000000000..fb0913e949 --- /dev/null +++ b/test/webapp/jstl/otherwise-outside-choose.jsp @@ -0,0 +1,22 @@ +<%-- + 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. +--%> +<%@ page session="false" contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +<%@ taglib uri="http://java.sun.com/jsp/jstl/core"; prefix="c" %> +<c:catch var="err"><c:forEach items="${['a','b']}" var="x"><c:otherwise + >O-${x}</c:otherwise></c:forEach></c:catch> +<p>error=${err}</p> diff --git a/test/webapp/jstl/when-outside-choose.jsp b/test/webapp/jstl/when-outside-choose.jsp new file mode 100644 index 0000000000..f17cfa9d41 --- /dev/null +++ b/test/webapp/jstl/when-outside-choose.jsp @@ -0,0 +1,22 @@ +<%-- + 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. +--%> +<%@ page session="false" contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +<%@ taglib uri="http://java.sun.com/jsp/jstl/core"; prefix="c" %> +<c:catch var="err"><c:forEach items="${['a','b']}" var="x"><c:when + test="${true}">X-${x}</c:when></c:forEach></c:catch> +<p>error=${err}</p> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index a07fe17e74..e5de6744ce 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -429,6 +429,16 @@ rejected and EL writes to such properties could be silently dropped when running in a GraalVM native image. (remm) </fix> + <fix> + The tag plugins for <code>c:when</code> and <code>c:otherwise</code> + now fall back to the standard tag handlers when the direct parent tag + is not a plugin enabled <code>c:choose</code>. Previously, they + generated unbalanced Java code for such misplaced tags, for example a + <code>c:when</code> nested directly in a <code>c:forEach</code>, which + failed the translation of the entire page instead of reporting the + illegal nesting at runtime as the reference implementation does. + (remm) + </fix> </changelog> </subsection> <subsection name="WebSocket"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
