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 9b1f916e3fcdc66c1491a53c876d97336479b04c Author: opencode <[email protected]> AuthorDate: Mon Oct 19 13:35:11 2026 +0200 Validate begin, end and step in the ForEach and ForTokens tag plugins The reference implementation validates these three attributes in their setters, throwing a JspTagException with the message "'begin' < 0", "'end' < 0" or "'step' <= 0" when the corresponding attribute is specified. The tag plugins generated no validation at all: a numeric c:forEach with step="0" generated a for loop with a zero increment, hanging the request thread indefinitely, a negative step looped forever, negative begin and end values were silently accepted, and c:forTokens with step="0" failed mid-loop with an ArithmeticException instead of a tag error. Evaluate the loop control attributes once, as the reference does, and emit the equivalent throw checks for the specified attributes. In numeric mode this also corrects two related deviations: end and step were previously re-evaluated on every iteration, and an unspecified begin or end produced uncompilable generated code, since no Java source is emitted for an absent attribute. These now default to 0 and -1, matching the reference and producing zero iterations when end is absent. Add tests using c:catch to pin the exception type and message of the generated checks. Found by code review. --- .../jasper/tagplugins/jstl/core/ForEach.java | 58 +++++++++++++++++++--- .../jasper/tagplugins/jstl/core/ForTokens.java | 19 ++++--- .../jasper/tagplugins/jstl/core/TestForEach.java | 37 ++++++++++++++ .../jasper/tagplugins/jstl/core/TestForTokens.java | 58 ++++++++++++++++++++++ test/webapp/jstl/foreach-begin-negative.jsp | 21 ++++++++ test/webapp/jstl/foreach-end-negative.jsp | 22 ++++++++ test/webapp/jstl/foreach-step-zero.jsp | 21 ++++++++ test/webapp/jstl/fortokens-begin-negative.jsp | 21 ++++++++ test/webapp/jstl/fortokens-step-zero.jsp | 21 ++++++++ 9 files changed, 262 insertions(+), 16 deletions(-) diff --git a/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java b/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java index 89a162e340..e69446a0f2 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java +++ b/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java @@ -54,18 +54,54 @@ public final class ForEach implements TagPlugin { return; } - // We must have a begin and end attributes + // We must have begin and end attributes. Evaluate the loop control + // attributes once and validate them, as the setters of the reference + // implementation LoopTagSupport do. index = ctxt.getTemporaryVariableName(); - ctxt.generateJavaSource("for (int " + index + " = "); - ctxt.generateAttribute("begin"); - ctxt.generateJavaSource("; " + index + " <= "); - ctxt.generateAttribute("end"); + + String beginName = ctxt.getTemporaryVariableName(); + ctxt.generateJavaSource("int " + beginName + " = "); + if (hasBegin) { + ctxt.generateAttribute("begin"); + } else { + ctxt.generateJavaSource("0"); + } + ctxt.generateJavaSource(";"); + if (hasBegin) { + ctxt.generateJavaSource("if (" + beginName + " < 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'begin' < 0\");}"); + } + + String endName = ctxt.getTemporaryVariableName(); + ctxt.generateJavaSource("int " + endName + " = "); + if (hasEnd) { + ctxt.generateAttribute("end"); + } else { + // Same default as the reference implementation, which results in + // zero iterations when end is not specified. + ctxt.generateJavaSource("-1"); + } + ctxt.generateJavaSource(";"); + if (hasEnd) { + ctxt.generateJavaSource("if (" + endName + " < 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'end' < 0\");}"); + } + + String stepName = null; if (hasStep) { - ctxt.generateJavaSource("; " + index + "+="); + stepName = ctxt.getTemporaryVariableName(); + ctxt.generateJavaSource("int " + stepName + " = "); ctxt.generateAttribute("step"); - ctxt.generateJavaSource(") {"); + ctxt.generateJavaSource(";"); + ctxt.generateJavaSource("if (" + stepName + " <= 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'step' <= 0\");}"); + } + + ctxt.generateJavaSource("for (int " + index + " = " + beginName + "; " + index + " <= " + endName + ";"); + if (hasStep) { + ctxt.generateJavaSource(" " + index + " += " + stepName + ") {"); } else { - ctxt.generateJavaSource("; " + index + "++) {"); + ctxt.generateJavaSource(" " + index + "++) {"); } // If var is specified, sync the var attribute @@ -98,6 +134,8 @@ public final class ForEach implements TagPlugin { ctxt.generateJavaSource("int " + beginV + " = "); ctxt.generateAttribute("begin"); ctxt.generateJavaSource(";"); + ctxt.generateJavaSource("if (" + beginV + " < 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'begin' < 0\");}"); } if (hasEnd) { indexV = ctxt.getTemporaryVariableName(); @@ -106,12 +144,16 @@ public final class ForEach implements TagPlugin { ctxt.generateJavaSource("int " + endV + " = "); ctxt.generateAttribute("end"); ctxt.generateJavaSource(";"); + ctxt.generateJavaSource("if (" + endV + " < 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'end' < 0\");}"); } if (hasStep) { stepV = ctxt.getTemporaryVariableName(); ctxt.generateJavaSource("int " + stepV + " = "); ctxt.generateAttribute("step"); ctxt.generateJavaSource(";"); + ctxt.generateJavaSource("if (" + stepV + " <= 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'step' <= 0\");}"); } String iterV = ctxt.getTemporaryVariableName(); diff --git a/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java b/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java index 8ae2858edb..05cd8f8156 100644 --- a/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java +++ b/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java @@ -78,6 +78,8 @@ public class ForTokens implements TagPlugin { ctxt.generateJavaSource("int " + beginName + " = "); ctxt.generateAttribute("begin"); ctxt.generateJavaSource(";"); + ctxt.generateJavaSource("if (" + beginName + " < 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'begin' < 0\");}"); ctxt.generateJavaSource("for(int " + index + " = 0; " + index + " < " + beginName + " && " + stName + ".hasMoreTokens(); " + index + "++, " + stName + ".nextToken()){}"); } else { @@ -88,15 +90,14 @@ public class ForTokens implements TagPlugin { // record the end place as the last index, otherwise, record it as "end"; // default end place is the last index if (hasEnd) { - ctxt.generateJavaSource("int " + endName + " = 0;"); - ctxt.generateJavaSource("if((" + tokensCountName + " - 1) < "); + String endSpecifiedName = ctxt.getTemporaryVariableName(); + ctxt.generateJavaSource("int " + endSpecifiedName + " = "); ctxt.generateAttribute("end"); - ctxt.generateJavaSource("){"); - ctxt.generateJavaSource(" " + endName + " = " + tokensCountName + " - 1;"); - ctxt.generateJavaSource("}else{"); - ctxt.generateJavaSource(" " + endName + " = "); - ctxt.generateAttribute("end"); - ctxt.generateJavaSource(";}"); + ctxt.generateJavaSource(";"); + ctxt.generateJavaSource("if (" + endSpecifiedName + " < 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'end' < 0\");}"); + ctxt.generateJavaSource("int " + endName + " = (" + tokensCountName + " - 1) < " + endSpecifiedName + + " ? " + tokensCountName + " - 1 : " + endSpecifiedName + ";"); } else { ctxt.generateJavaSource("int " + endName + " = " + tokensCountName + " - 1;"); } @@ -107,6 +108,8 @@ public class ForTokens implements TagPlugin { ctxt.generateJavaSource("int " + stepName + " = "); ctxt.generateAttribute("step"); ctxt.generateJavaSource(";"); + ctxt.generateJavaSource("if (" + stepName + " <= 0) {" + + "throw new jakarta.servlet.jsp.JspTagException(\"'step' <= 0\");}"); } else { ctxt.generateJavaSource("int " + stepName + " = 1;"); } diff --git a/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java b/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java index 869961163b..50905ff2cb 100644 --- a/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java +++ b/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java @@ -72,4 +72,41 @@ public class TestForEach extends AbstractTestTag { Assert.assertEquals(2, body.split("java\\.lang\\.Integer", -1).length - 1); Assert.assertFalse(body, body.contains("java.lang.String")); } + + + /* + * The reference implementation validates the begin, end and step + * attributes in their setters, throwing a JspTagException. The plugin + * must generate the equivalent checks rather than, for example, looping + * forever on step="0". The pages wrap the tag in c:catch and render the + * caught exception, pinning the reference exception type and message. + */ + @Test + public void testStepZero() throws Exception { + doTestInvalidAttribute("foreach-step-zero.jsp", "'step' <= 0"); + } + + + @Test + public void testBeginNegative() throws Exception { + doTestInvalidAttribute("foreach-begin-negative.jsp", "'begin' < 0"); + } + + + @Test + public void testEndNegative() throws Exception { + doTestInvalidAttribute("foreach-end-negative.jsp", "'end' < 0"); + } + + + private void doTestInvalidAttribute(String jsp, String message) throws Exception { + ByteChunk res = new ByteChunk(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/" + jsp, res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertTrue(res.toString(), res.toString() + .contains("jakarta.servlet.jsp.JspTagException: " + message)); + } } diff --git a/test/org/apache/jasper/tagplugins/jstl/core/TestForTokens.java b/test/org/apache/jasper/tagplugins/jstl/core/TestForTokens.java new file mode 100644 index 0000000000..7c1aa63e44 --- /dev/null +++ b/test/org/apache/jasper/tagplugins/jstl/core/TestForTokens.java @@ -0,0 +1,58 @@ +/* + * 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 TestForTokens extends AbstractTestTag { + + /* + * The reference implementation validates the begin, end and step + * attributes in their setters, throwing a JspTagException. The plugin + * must generate the equivalent checks rather than, for example, failing + * with an ArithmeticException on step="0". The pages wrap the tag in + * c:catch and render the caught exception, pinning the reference + * exception type and message. + */ + @Test + public void testStepZero() throws Exception { + doTestInvalidAttribute("fortokens-step-zero.jsp", "'step' <= 0"); + } + + + @Test + public void testBeginNegative() throws Exception { + doTestInvalidAttribute("fortokens-begin-negative.jsp", "'begin' < 0"); + } + + + private void doTestInvalidAttribute(String jsp, String message) throws Exception { + ByteChunk res = new ByteChunk(); + + int rc = getUrl("http://localhost:"; + getPort() + + "/test/jstl/" + jsp, res, null); + + Assert.assertEquals(HttpServletResponse.SC_OK, rc); + Assert.assertTrue(res.toString(), res.toString() + .contains("jakarta.servlet.jsp.JspTagException: " + message)); + } +} diff --git a/test/webapp/jstl/foreach-begin-negative.jsp b/test/webapp/jstl/foreach-begin-negative.jsp new file mode 100644 index 0000000000..88dbcdf4c9 --- /dev/null +++ b/test/webapp/jstl/foreach-begin-negative.jsp @@ -0,0 +1,21 @@ +<%-- + 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 begin="-1" end="10" var="i">Should not be reached</c:forEach></c:catch> +<p>error=${err}</p> diff --git a/test/webapp/jstl/foreach-end-negative.jsp b/test/webapp/jstl/foreach-end-negative.jsp new file mode 100644 index 0000000000..123db53581 --- /dev/null +++ b/test/webapp/jstl/foreach-end-negative.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']}" begin="0" end="-1" + var="i">Should not be reached</c:forEach></c:catch> +<p>error=${err}</p> diff --git a/test/webapp/jstl/foreach-step-zero.jsp b/test/webapp/jstl/foreach-step-zero.jsp new file mode 100644 index 0000000000..753bbead87 --- /dev/null +++ b/test/webapp/jstl/foreach-step-zero.jsp @@ -0,0 +1,21 @@ +<%-- + 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 begin="0" end="10" step="0" var="i">Should not be reached</c:forEach></c:catch> +<p>error=${err}</p> diff --git a/test/webapp/jstl/fortokens-begin-negative.jsp b/test/webapp/jstl/fortokens-begin-negative.jsp new file mode 100644 index 0000000000..837e378fd9 --- /dev/null +++ b/test/webapp/jstl/fortokens-begin-negative.jsp @@ -0,0 +1,21 @@ +<%-- + 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:forTokens items="a,b,c" delims="," begin="-1" var="t">Should not be reached</c:forTokens></c:catch> +<p>error=${err}</p> diff --git a/test/webapp/jstl/fortokens-step-zero.jsp b/test/webapp/jstl/fortokens-step-zero.jsp new file mode 100644 index 0000000000..02d6f20485 --- /dev/null +++ b/test/webapp/jstl/fortokens-step-zero.jsp @@ -0,0 +1,21 @@ +<%-- + 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:forTokens items="a,b,c" delims="," step="0" var="t">Should not be reached</c:forTokens></c:catch> +<p>error=${err}</p> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
