This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 933385d7ffb4ba57501ac4a5770a5ced8b0c5574 Author: opencode <[email protected]> AuthorDate: Wed Oct 14 16:58:30 2026 +0200 Signal EmptyStackException for a negative ArrayStack peek index ArrayStack.peek(int) computed [size() - n - 1] and threw EmptyStackException only when that index fell below zero, which covers indexes beyond the bottom of the stack. A negative index moved the computed position beyond the end of the backing list instead, so get() threw an IndexOutOfBoundsException. That exception escaped Digester.peek(int), which only catches EmptyStackException, even though the method is documented to return null for any out of range index. One path where a negative index can be passed in is CallMethodRule with a negative targetOffset deeper than the current stack depth, where digester.peek(getCount() + targetOffset) then reported a wrapped IndexOutOfBoundsException rather than the intended 'Call target is null' parse error that names the offset and the stack depth. Guard both ends of the range instead. For non-negative indexes the condition is equivalent to the previous one, so no legitimate use changes, and the outcome for extreme values no longer depends on integer wrap-around. Adds TestArrayStack covering the top, the bottom, the empty stack and both out of range directions including Integer.MIN_VALUE and Integer.MAX_VALUE, plus a TestDigester test for the peek(int) null contract for negative indexes. --- .../apache/tomcat/util/digester/ArrayStack.java | 15 ++-- .../tomcat/util/digester/TestArrayStack.java | 88 ++++++++++++++++++++++ .../apache/tomcat/util/digester/TestDigester.java | 16 ++++ 3 files changed, 114 insertions(+), 5 deletions(-) diff --git a/java/org/apache/tomcat/util/digester/ArrayStack.java b/java/org/apache/tomcat/util/digester/ArrayStack.java index 22fa3e1ac4..3fbabee12e 100644 --- a/java/org/apache/tomcat/util/digester/ArrayStack.java +++ b/java/org/apache/tomcat/util/digester/ArrayStack.java @@ -98,15 +98,20 @@ public class ArrayStack<E> extends ArrayList<E> { * * @return the n'th item on the stack, zero relative * - * @throws EmptyStackException if there are not enough items on the stack to satisfy this request + * @throws EmptyStackException if there are not enough items on the stack to satisfy this request, including when + * {@code n} is negative. Note that unlike the original Commons Collections code, a + * negative index signals EmptyStackException rather than surfacing as an + * IndexOutOfBoundsException. */ public E peek(int n) throws EmptyStackException { - int m = (size() - n) - 1; - if (m < 0) { + // A negative index is out of range just like an index beyond the bottom + // of the stack. Guarding only the upper bound would let get() throw an + // IndexOutOfBoundsException, which callers such as Digester.peek(int) + // do not expect. + if (n < 0 || n >= size()) { throw new EmptyStackException(); - } else { - return get(m); } + return get(size() - 1 - n); } /** diff --git a/test/org/apache/tomcat/util/digester/TestArrayStack.java b/test/org/apache/tomcat/util/digester/TestArrayStack.java new file mode 100644 index 0000000000..9146b40588 --- /dev/null +++ b/test/org/apache/tomcat/util/digester/TestArrayStack.java @@ -0,0 +1,88 @@ +/* + * 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.tomcat.util.digester; + +import java.util.EmptyStackException; + +import org.junit.Assert; +import org.junit.Test; + +public class TestArrayStack { + + @Test + public void testPeekTopAndBottom() { + ArrayStack<String> stack = new ArrayStack<>(); + stack.push("bottom"); + stack.push("top"); + + Assert.assertEquals("top", stack.peek(0)); + Assert.assertEquals("bottom", stack.peek(1)); + } + + + @Test(expected = EmptyStackException.class) + public void testPeekEmptyStack() { + new ArrayStack<String>().peek(0); + } + + + @Test(expected = EmptyStackException.class) + public void testPeekBeyondBottom() { + ArrayStack<String> stack = new ArrayStack<>(); + stack.push("only"); + + stack.peek(1); + } + + + /* + * A negative index is out of range rather than beyond the end of the + * backing list, so it must signal EmptyStackException like the other + * out of range indexes. + */ + @Test(expected = EmptyStackException.class) + public void testPeekNegativeIndex() { + ArrayStack<String> stack = new ArrayStack<>(); + stack.push("only"); + + stack.peek(-1); + } + + + @Test(expected = EmptyStackException.class) + public void testPeekNegativeIndexEmptyStack() { + new ArrayStack<String>().peek(-1); + } + + + @Test(expected = EmptyStackException.class) + public void testPeekIntegerMinValue() { + ArrayStack<String> stack = new ArrayStack<>(); + stack.push("only"); + + stack.peek(Integer.MIN_VALUE); + } + + + @Test(expected = EmptyStackException.class) + public void testPeekIntegerMaxValue() { + ArrayStack<String> stack = new ArrayStack<>(); + stack.push("only"); + + stack.peek(Integer.MAX_VALUE); + } +} diff --git a/test/org/apache/tomcat/util/digester/TestDigester.java b/test/org/apache/tomcat/util/digester/TestDigester.java index 25eaf04f66..3f98771dd5 100644 --- a/test/org/apache/tomcat/util/digester/TestDigester.java +++ b/test/org/apache/tomcat/util/digester/TestDigester.java @@ -185,4 +185,20 @@ public class TestDigester { Assert.assertNotNull(digester.createSAXException("message", new RuntimeException("cause"))); } + + /* + * peek(int) is documented to return null for any out of range index, + * including negative ones such as the values a negative CallMethodRule + * target offset can produce. + */ + @Test + public void testPeekOutOfRangeIndexReturnsNull() { + Digester digester = new Digester(); + digester.push(new Object()); + + Assert.assertNotNull(digester.peek(0)); + Assert.assertNull(digester.peek(1)); + Assert.assertNull(digester.peek(-1)); + } + } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
