This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/11.0.x by this push:
new 62dcff2b0b Review "Validate begin, end and step in the ForEach and
ForTokens..."
62dcff2b0b is described below
commit 62dcff2b0b084c9543a6ff212017b49b6afdc124
Author: Mark Thomas <[email protected]>
AuthorDate: Fri Nov 6 10:53:22 2026 +0000
Review "Validate begin, end and step in the ForEach and ForTokens..."
Add handling for end > begin
Reduce code duplication
Co-authored by: Claude / Sonnet 5.5
Co-authored by: Codex / GPT-6.1-Sol
---
.../jasper/tagplugins/jstl/core/ForEach.java | 26 +++++----
.../jasper/tagplugins/jstl/core/ForTokens.java | 9 +--
.../tagplugins/jstl/core/LoopAttributeChecks.java | 68 ++++++++++++++++++++++
.../jasper/tagplugins/jstl/core/TestForEach.java | 16 +++++
test/webapp/jstl/foreach-begin-after-end.jsp | 40 +++++++++++++
5 files changed, 141 insertions(+), 18 deletions(-)
diff --git a/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java
b/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java
index 57353edcde..e041ad6cf9 100644
--- a/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java
+++ b/java/org/apache/jasper/tagplugins/jstl/core/ForEach.java
@@ -69,8 +69,7 @@ public final class ForEach implements TagPlugin {
}
ctxt.generateJavaSource(";");
if (hasBegin) {
- ctxt.generateJavaSource("if (" + beginName + " < 0) {" +
- "throw new jakarta.servlet.jsp.JspTagException(\"'begin' <
0\");}");
+ LoopAttributeChecks.generateBeginCheck(ctxt, beginName);
}
String endName = ctxt.getTemporaryVariableName();
@@ -84,8 +83,7 @@ public final class ForEach implements TagPlugin {
}
ctxt.generateJavaSource(";");
if (hasEnd) {
- ctxt.generateJavaSource("if (" + endName + " < 0) {" +
- "throw new jakarta.servlet.jsp.JspTagException(\"'end' <
0\");}");
+ LoopAttributeChecks.generateEndCheck(ctxt, endName);
}
String stepName = null;
@@ -94,8 +92,7 @@ public final class ForEach implements TagPlugin {
ctxt.generateJavaSource("int " + stepName + " = ");
ctxt.generateAttribute("step");
ctxt.generateJavaSource(";");
- ctxt.generateJavaSource("if (" + stepName + " <= 0) {" +
- "throw new jakarta.servlet.jsp.JspTagException(\"'step' <=
0\");}");
+ LoopAttributeChecks.generateStepCheck(ctxt, stepName);
}
ctxt.generateJavaSource("for (int " + index + " = " + beginName + "; "
+ index + " <= " + endName + ";");
@@ -136,8 +133,7 @@ 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\");}");
+ LoopAttributeChecks.generateBeginCheck(ctxt, beginV);
}
if (hasEnd) {
indexV = ctxt.getTemporaryVariableName();
@@ -146,16 +142,19 @@ 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\");}");
+ LoopAttributeChecks.generateEndCheck(ctxt, endV);
}
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\");}");
+ LoopAttributeChecks.generateStepCheck(ctxt, stepV);
+ }
+
+ // An empty range must not execute the body or consume any items.
+ if (hasBegin && hasEnd) {
+ ctxt.generateJavaSource("if (" + beginV + " <= " + endV + ") {");
}
String iterV = ctxt.getTemporaryVariableName();
@@ -248,6 +247,9 @@ public final class ForEach implements TagPlugin {
}
ctxt.generateJavaSource("}"); // while
ctxt.generateJavaSource("}"); // Not Null
+ if (hasBegin && hasEnd) {
+ ctxt.generateJavaSource("}"); // Non-empty range
+ }
}
/**
diff --git a/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java
b/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java
index 05cd8f8156..8e70b5dbfa 100644
--- a/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java
+++ b/java/org/apache/jasper/tagplugins/jstl/core/ForTokens.java
@@ -78,8 +78,7 @@ 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\");}");
+ LoopAttributeChecks.generateBeginCheck(ctxt, beginName);
ctxt.generateJavaSource("for(int " + index + " = 0; " + index + "
< " + beginName + " && " + stName +
".hasMoreTokens(); " + index + "++, " + stName +
".nextToken()){}");
} else {
@@ -94,8 +93,7 @@ public class ForTokens implements TagPlugin {
ctxt.generateJavaSource("int " + endSpecifiedName + " = ");
ctxt.generateAttribute("end");
ctxt.generateJavaSource(";");
- ctxt.generateJavaSource("if (" + endSpecifiedName + " < 0) {" +
- "throw new jakarta.servlet.jsp.JspTagException(\"'end' <
0\");}");
+ LoopAttributeChecks.generateEndCheck(ctxt, endSpecifiedName);
ctxt.generateJavaSource("int " + endName + " = (" +
tokensCountName + " - 1) < " + endSpecifiedName +
" ? " + tokensCountName + " - 1 : " + endSpecifiedName +
";");
} else {
@@ -108,8 +106,7 @@ 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\");}");
+ LoopAttributeChecks.generateStepCheck(ctxt, stepName);
} else {
ctxt.generateJavaSource("int " + stepName + " = 1;");
}
diff --git
a/java/org/apache/jasper/tagplugins/jstl/core/LoopAttributeChecks.java
b/java/org/apache/jasper/tagplugins/jstl/core/LoopAttributeChecks.java
new file mode 100644
index 0000000000..c358152007
--- /dev/null
+++ b/java/org/apache/jasper/tagplugins/jstl/core/LoopAttributeChecks.java
@@ -0,0 +1,68 @@
+/*
+ * 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 org.apache.jasper.compiler.tagplugin.TagPluginContext;
+
+/**
+ * Generates the validation of the <code>begin</code>, <code>end</code> and
<code>step</code> loop control attributes
+ * shared by the <c:forEach> and <c:forTokens> tag plugins. The
checks and messages match the setters of the
+ * reference implementation, <code>LoopTagSupport</code>.
+ */
+final class LoopAttributeChecks {
+
+ private LoopAttributeChecks() {
+ // Utility class. Hide default constructor
+ }
+
+ /**
+ * Generates a check that throws a <code>JspTagException</code> if the
named <code>begin</code> variable is
+ * negative.
+ *
+ * @param ctxt The tag plugin context
+ * @param variable The name of the generated variable holding the value of
the attribute
+ */
+ static void generateBeginCheck(TagPluginContext ctxt, String variable) {
+ generateCheck(ctxt, variable, "<", "'begin' < 0");
+ }
+
+ /**
+ * Generates a check that throws a <code>JspTagException</code> if the
named <code>end</code> variable is negative.
+ *
+ * @param ctxt The tag plugin context
+ * @param variable The name of the generated variable holding the value of
the attribute
+ */
+ static void generateEndCheck(TagPluginContext ctxt, String variable) {
+ generateCheck(ctxt, variable, "<", "'end' < 0");
+ }
+
+ /**
+ * Generates a check that throws a <code>JspTagException</code> if the
named <code>step</code> variable is not
+ * positive.
+ *
+ * @param ctxt The tag plugin context
+ * @param variable The name of the generated variable holding the value of
the attribute
+ */
+ static void generateStepCheck(TagPluginContext ctxt, String variable) {
+ generateCheck(ctxt, variable, "<=", "'step' <= 0");
+ }
+
+ private static void generateCheck(TagPluginContext ctxt, String variable,
String operator, String message) {
+ ctxt.generateJavaSource("if (" + variable + " " + operator + " 0) {" +
+ "throw new jakarta.servlet.jsp.JspTagException(\"" + message +
"\");}");
+ }
+}
diff --git a/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java
b/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java
index 50905ff2cb..3c9405b87e 100644
--- a/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java
+++ b/test/org/apache/jasper/tagplugins/jstl/core/TestForEach.java
@@ -74,6 +74,22 @@ public class TestForEach extends AbstractTestTag {
}
+ @Test
+ public void testCollectionBeginAfterEnd() throws Exception {
+ ByteChunk res = new ByteChunk();
+
+ int rc = getUrl("http://localhost:"; + getPort() +
+ "/test/jstl/foreach-begin-after-end.jsp", res, null);
+
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ String body = res.toString();
+ Assert.assertFalse(body, body.contains("FAIL"));
+ Assert.assertTrue(body, body.contains("iteratorNext=a"));
+ Assert.assertTrue(body, body.contains("equal=b"));
+ Assert.assertTrue(body, body.contains("zero=a"));
+ }
+
+
/*
* The reference implementation validates the begin, end and step
* attributes in their setters, throwing a JspTagException. The plugin
diff --git a/test/webapp/jstl/foreach-begin-after-end.jsp
b/test/webapp/jstl/foreach-begin-after-end.jsp
new file mode 100644
index 0000000000..bd3f7c29bb
--- /dev/null
+++ b/test/webapp/jstl/foreach-begin-after-end.jsp
@@ -0,0 +1,40 @@
+<%--
+ 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" import="java.util.Iterator,java.util.List" %>
+<%@ taglib uri="http://java.sun.com/jsp/jstl/core"; prefix="c" %>
+<%
+ Iterator<String> iterator = List.of("a", "b", "c", "d").iterator();
+ pageContext.setAttribute("values", iterator);
+ pageContext.setAttribute("array", new String[] { "a", "b", "c", "d" });
+%>
+<c:forEach items="${['a','b','c','d']}" begin="2" end="1" var="i">
+ <p>FAIL - list body executed</p>
+</c:forEach>
+<c:forEach items="${array}" begin="2" end="1" var="i">
+ <p>FAIL - array body executed</p>
+</c:forEach>
+<c:forEach items="${values}" begin="2" end="1" var="i">
+ <p>FAIL - iterator body executed</p>
+</c:forEach>
+<p>iteratorNext=<%= iterator.next() %></p>
+<c:forEach items="${['a','b','c']}" begin="1" end="1" var="i">
+ <p>equal=${i}</p>
+</c:forEach>
+<c:forEach items="${['a','b','c']}" end="0" var="i">
+ <p>zero=${i}</p>
+</c:forEach>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
