Skip to content

Fix #3 ArrayIndexOutOfBoundsException #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Taskeren wants to merge 1 commit into
base: main
Choose a base branch
from
+49 −2
Open

Fix #3 ArrayIndexOutOfBoundsException #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/main/java/org/glavo/nbt/tag/ParentTag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ final void ensureTagsCapacityForAdd() {

int arrayEnd = Math.min(size, tags.length);
if (index < arrayEnd - 1) {
System.arraycopy(tags, index + 1, tags, index, size - index);
System.arraycopy(tags, index + 1, tags, index, size - index - 1);

@Glavo Glavo Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本审查建议由 GPT-5 生成

[P1] removeTagFromArray 仍按 size 计算 System.arraycopy 长度，但 ParentTag 注释明确允许 ArrayTag 的 tags.length 小于 size。例如 new IntArrayTag(new int[20]).getTag(1); removeTagAt(1); 时，size == 20、tags.length == 12，这里的复制长度为 18，仍会读到 tags 数组边界之外并抛出 ArrayIndexOutOfBoundsException。这里应按 arrayEnd - index - 1 复制，而不是 size - index - 1。否则这个修复只覆盖了 ListTag/CompoundTag 容量等于 size 的场景，ArrayTag.removeAt/removeTagAt/addTag 中移动已缓存子 tag 的路径仍会崩溃。

测试缺口：新增测试只覆盖 ListTag，没有覆盖 ArrayTag 的 lazy cached tags 且 size > tags.length 的删除场景。

tags[arrayEnd - 1] = null;
} else if (oldTag != null) {
tags[index] = null;
}
Expand Down Expand Up @@ -179,7 +180,7 @@ final void moveTagToLast(T tag) {
/// If the `tag` is already a child of another tag, removes it from old parent and adds it to this tag.
@Contract(value = "_ -> this", mutates = "this,param1")
public abstract ParentTag<T> addTag(@Flow(targetIsContainer = true)
T tag) throws IllegalArgumentException;
T tag) throws IllegalArgumentException;

/// Adds all `tags` to this tag.
///
Expand Down
46 changes: 46 additions & 0 deletions src/test/java/org/glavo/nbt/tag/ListTagTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.glavo.nbt.tag;

import org.glavo.nbt.internal.ArrayAccessor;
import org.junit.jupiter.api.Test;

import java.util.function.Supplier;
Expand Down Expand Up @@ -238,4 +239,49 @@ void testCloneEqualsAndIndependentChildren() {
assertEquals("changed", cloneFirst.get());
assertContentNotEquals(tag, clone);
}

@Test
void testRemoveAt() {
ListTag<IntTag> list = new ListTag<>();
for (int i = 0; i < 9; i++) {
list.addTag(new IntTag(i));
}
assertEquals(new IntTag(6), list.removeTagAt(6));
assertEquals(8, list.size());
assertArrayEquals(new IntTag[]{
new IntTag(0), new IntTag(1), new IntTag(2),
new IntTag(3), new IntTag(4), new IntTag(5),
new IntTag(7), new IntTag(8), null,
null, null, null}, list.tags);
}

@Test
void testRemoveAtCapacity() {
ListTag<IntTag> list = new ListTag<>();
for (int i = 0; i < ArrayAccessor.DEFAULT_CAPACITY; i++) {
list.addTag(new IntTag(i));
}
assertEquals(new IntTag(6), list.removeTagAt(6));
assertEquals(ArrayAccessor.DEFAULT_CAPACITY - 1, list.size());
assertArrayEquals(new IntTag[]{
new IntTag(0), new IntTag(1), new IntTag(2),
new IntTag(3), new IntTag(4), new IntTag(5),
new IntTag(7), new IntTag(8), new IntTag(9),
new IntTag(10), new IntTag(11), null}, list.tags);
}

@Test
void testRemoveAtEnd() {
ListTag<IntTag> list = new ListTag<>();
for (int i = 0; i < ArrayAccessor.DEFAULT_CAPACITY; i++) {
list.addTag(new IntTag(i));
}
assertEquals(new IntTag(11), list.removeTagAt(11));
assertEquals(ArrayAccessor.DEFAULT_CAPACITY - 1, list.size());
assertArrayEquals(new IntTag[]{
new IntTag(0), new IntTag(1), new IntTag(2),
new IntTag(3), new IntTag(4), new IntTag(5),
new IntTag(6), new IntTag(7), new IntTag(8),
new IntTag(9), new IntTag(10), null}, list.tags);
}
}
Loading