From 5c18a6a2f1c452eae36b4f272f62fdcfc030ca5e Mon Sep 17 00:00:00 2001 From: renechoi Date: Sun, 2 Aug 2026 11:32:17 +0900 Subject: [PATCH] Fix SubList.get() accepting an index equal to its own size The bounds check used `index > size()`, so `get(size())` fell through to `AbstractImmutableList.this.get(fromIndex + index)`. When the sub list stops before the end of the backing list that resolves to a real element outside the sub list and is returned without any error: `copy(List.of("a","b","c")).subList(0, 1).get(1)` answers "b". Only a sub list reaching the end of its parent fails, and then with an ArrayIndexOutOfBoundsException from the backing array rather than the IndexOutOfBoundsException the class builds for this case. Applied to the mdo template and to both generated copies, otherwise the next regeneration reintroduces it. listIterator(int) keeps `index > size()` a few lines above. That bound is correct there, since List.listIterator(int) accepts size() as a cursor position, and a test pins it so the two are not aligned by mistake later. Fixes #12595 --- .../maven/api/xml/ImmutableCollections.java | 2 +- .../internal/xml/ImmutableCollections.java | 2 +- .../xml/ImmutableCollectionsTest.java | 74 +++++++++++++++++++ src/mdo/java/ImmutableCollections.java | 2 +- 4 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 impl/maven-xml/src/test/java/org/apache/maven/internal/xml/ImmutableCollectionsTest.java diff --git a/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/ImmutableCollections.java b/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/ImmutableCollections.java index ca1c4707e890..243f888bc1e2 100644 --- a/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/ImmutableCollections.java +++ b/api/maven-api-xml/src/main/java/org/apache/maven/api/xml/ImmutableCollections.java @@ -297,7 +297,7 @@ private SubList(int fromIndex, int toIndex) { @Override public E get(int index) { - if (index < 0 || index > size()) { + if (index < 0 || index >= size()) { throw outOfBounds(index); } return AbstractImmutableList.this.get(fromIndex + index); diff --git a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/ImmutableCollections.java b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/ImmutableCollections.java index dc6651da94d9..c16eb54637f5 100644 --- a/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/ImmutableCollections.java +++ b/impl/maven-xml/src/main/java/org/apache/maven/internal/xml/ImmutableCollections.java @@ -297,7 +297,7 @@ private SubList(int fromIndex, int toIndex) { @Override public E get(int index) { - if (index < 0 || index > size()) { + if (index < 0 || index >= size()) { throw outOfBounds(index); } return AbstractImmutableList.this.get(fromIndex + index); diff --git a/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/ImmutableCollectionsTest.java b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/ImmutableCollectionsTest.java new file mode 100644 index 000000000000..651714ba9ff2 --- /dev/null +++ b/impl/maven-xml/src/test/java/org/apache/maven/internal/xml/ImmutableCollectionsTest.java @@ -0,0 +1,74 @@ +/* + * 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.maven.internal.xml; + +import java.util.Arrays; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class ImmutableCollectionsTest { + + private static List abc() { + return ImmutableCollections.copy(Arrays.asList("a", "b", "c")); + } + + @Test + void subListRejectsIndexEqualToItsSize() { + List sub = abc().subList(0, 1); + + assertEquals(1, sub.size()); + assertEquals("a", sub.get(0)); + assertThrows(IndexOutOfBoundsException.class, () -> sub.get(1)); + } + + @Test + void subListDoesNotReachPastItsEnd() { + // A sub list that stops before the end of the backing list is the case that used to + // read past its own bounds instead of failing: the parent list resolves the index + // and hands back an element the sub list does not contain. + List sub = abc().subList(1, 2); + + assertEquals(List.of("b"), sub); + assertThrows(IndexOutOfBoundsException.class, () -> sub.get(1)); + assertThrows(IndexOutOfBoundsException.class, () -> sub.get(-1)); + } + + @Test + void subListIterationStaysWithinBounds() { + List sub = abc().subList(0, 2); + + assertEquals(List.of("a", "b"), sub); + assertEquals(List.of("a", "b"), List.copyOf(sub)); + } + + @Test + void listIteratorStillAcceptsIndexEqualToSize() { + // List.listIterator(int) is specified to accept size() as a valid cursor position, + // so that bound is deliberately not the same as the one for get(int). + List list = abc(); + + assertEquals(3, list.size()); + assertEquals(false, list.listIterator(3).hasNext()); + assertThrows(IndexOutOfBoundsException.class, () -> list.listIterator(4)); + } +} diff --git a/src/mdo/java/ImmutableCollections.java b/src/mdo/java/ImmutableCollections.java index 4b69b4d3f554..0c0b32ca80be 100644 --- a/src/mdo/java/ImmutableCollections.java +++ b/src/mdo/java/ImmutableCollections.java @@ -300,7 +300,7 @@ private SubList(int fromIndex, int toIndex) { @Override public E get(int index) { - if (index < 0 || index > size()) { + if (index < 0 || index >= size()) { throw outOfBounds(index); } return AbstractImmutableList.this.get(fromIndex + index);