From 9ccf34c8425d94508031d607d688fbd208edc618 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Mon, 22 Jun 2026 18:59:33 +0200 Subject: [PATCH 1/5] fix: strip null/empty item stacks from parcel content (#221) Empty/air inventory slots could slip past the GUI write filters and round-trip through the persister as null elements, causing a NullPointerException when CollectionGui read itemStack.getType() on a collected parcel. Normalize the item list in the ParcelContent record's compact constructor so null and empty stacks are dropped and the exposed list is immutable. This is the single chokepoint every read and write path goes through, so it heals already-corrupted rows on read and prevents bad writes. Adds Mockito to mock ItemStack in unit tests, since paper-api ItemStacks cannot be constructed without a running server (RegistryAccess). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01R4YqhTV42FvN95HanC7XJc --- build.gradle.kts | 2 + buildSrc/src/main/kotlin/Versions.kt | 1 + .../parcellockers/content/ParcelContent.java | 10 +++++ .../content/ParcelContentTest.java | 38 +++++++++++++++++++ 4 files changed, 51 insertions(+) create mode 100644 src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java diff --git a/build.gradle.kts b/build.gradle.kts index be08d5547..5c470ab48 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -88,6 +88,8 @@ dependencies { testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${Versions.JUNIT}") testRuntimeOnly("org.junit.platform:junit-platform-launcher") + testImplementation("org.mockito:mockito-core:${Versions.MOCKITO}") + testImplementation("org.testcontainers:junit-jupiter:${Versions.TESTCONTAINERS}") testImplementation("org.testcontainers:mysql:${Versions.TESTCONTAINERS}") testImplementation("mysql:mysql-connector-java:${Versions.MYSQL_CONNECTOR}") diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt index d0dc40f10..8e318c21c 100644 --- a/buildSrc/src/main/kotlin/Versions.kt +++ b/buildSrc/src/main/kotlin/Versions.kt @@ -32,4 +32,5 @@ object Versions { const val JUNIT = "6.1.0" const val TESTCONTAINERS = "1.21.4" const val MYSQL_CONNECTOR = "8.0.33" + const val MOCKITO = "5.14.2" } diff --git a/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java b/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java index 0a10c3a7e..1df6d6880 100644 --- a/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java +++ b/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java @@ -7,4 +7,14 @@ public record ParcelContent(UUID uniqueId, List items) { + public ParcelContent { + // Guard against null/empty stacks leaking into the content (issue #221): empty/air slots + // can slip past the GUI write filters and round-trip through the persister as nulls, which + // would later NPE when the collection GUI reads itemStack.getType(). + items = items == null + ? List.of() + : items.stream() + .filter(item -> item != null && !item.isEmpty()) + .toList(); + } } diff --git a/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java b/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java new file mode 100644 index 000000000..26fe03734 --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java @@ -0,0 +1,38 @@ +package com.eternalcode.parcellockers.content; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.List; +import java.util.UUID; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.Test; + +class ParcelContentTest { + + @Test + void dropsNullItems() { + // Reproduces issue #221: a null element in the content list makes CollectionGui + // NPE on itemStack.getType(). The model must never expose null items. + ItemStack stone = mock(ItemStack.class); + + ParcelContent content = new ParcelContent(UUID.randomUUID(), Arrays.asList(stone, null)); + + assertEquals(List.of(stone), content.items()); + } + + @Test + void dropsEmptyItems() { + // Empty/air slots can slip past the GUI write filters; they must not be exposed + // as content, otherwise they round-trip through the persister as nulls. + ItemStack stone = mock(ItemStack.class); + ItemStack air = mock(ItemStack.class); + when(air.isEmpty()).thenReturn(true); + + ParcelContent content = new ParcelContent(UUID.randomUUID(), List.of(stone, air)); + + assertEquals(List.of(stone), content.items()); + } +} From 1ecdb8ebe9df51181ed0fd30fc4d8cbd8b2c640a Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:00:57 +0200 Subject: [PATCH 2/5] fix: stop using legacy ItemStack serialization for parcel content Legacy (Spigot map) ItemStack serialization drops empty/air stacks to null, the upstream cause of the issue #221 NPE. The jackson-bukkit Paper module defaults to an NBT byte-array format (ItemStack#serializeAsBytes) that round-trips empties safely, and its deserializer auto-detects the format, so previously stored legacy-format rows still read correctly and are rewritten in the new format on next save. The ParcelContent null/empty guard remains as defense in depth. Note: the serialization round-trip cannot be exercised by the existing test harness (paper-api ItemStacks require a running server, and the integration tests are Docker-gated), so this relies on the library's documented backward-compatible auto-detection. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01R4YqhTV42FvN95HanC7XJc --- .../database/persister/ItemStackPersister.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java b/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java index 63d87811c..301b12c7f 100644 --- a/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java +++ b/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java @@ -17,11 +17,12 @@ public class ItemStackPersister extends BaseDataType { private static final ItemStackPersister instance = new ItemStackPersister(); + // Paper plugins must NOT use legacy (Spigot map) ItemStack serialization: it drops empty/air + // stacks to null, which caused the NPE in issue #221. The default Paper serializer uses an NBT + // byte array (ItemStack#serializeAsBytes) that round-trips empties safely. The deserializer + // auto-detects and still reads any data previously written in the legacy format. private static final ObjectMapper JSON = JsonMapper.builder() - .addModule(JacksonPaper.builder() - .useLegacyItemStackSerialization() - .build() - ) + .addModule(JacksonPaper.builder().build()) .build(); private ItemStackPersister() { From dbfde431fb922033600be0ced582e739d264974f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20K=C4=99dziora?= <77227023+Jakubk15@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:05:43 +0200 Subject: [PATCH 3/5] Update src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../com/eternalcode/parcellockers/content/ParcelContent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java b/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java index 1df6d6880..3e04bfb0b 100644 --- a/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java +++ b/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java @@ -11,7 +11,7 @@ public record ParcelContent(UUID uniqueId, List items) { // Guard against null/empty stacks leaking into the content (issue #221): empty/air slots // can slip past the GUI write filters and round-trip through the persister as nulls, which // would later NPE when the collection GUI reads itemStack.getType(). - items = items == null + items = items == null || items.isEmpty() ? List.of() : items.stream() .filter(item -> item != null && !item.isEmpty()) From a1ed767515e0024dc97963d82213aeca34578305 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:07:37 +0200 Subject: [PATCH 4/5] test: cover null items list in ParcelContent guard Adds a case verifying the compact constructor defaults a null items list to an empty list, closing the coverage gap on the defensive guard. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01R4YqhTV42FvN95HanC7XJc --- .../parcellockers/content/ParcelContentTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java b/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java index 26fe03734..2983c662f 100644 --- a/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java +++ b/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java @@ -35,4 +35,11 @@ void dropsEmptyItems() { assertEquals(List.of(stone), content.items()); } + + @Test + void handlesNullItemsList() { + ParcelContent content = new ParcelContent(UUID.randomUUID(), null); + + assertEquals(List.of(), content.items()); + } } From c8c0a0c29851c3de7544c46d9acc9374b3fa173a Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Mon, 22 Jun 2026 21:06:32 +0200 Subject: [PATCH 5/5] fix: guard against null parcel description in AdminParcelEditGui A parcel description may legitimately be null (SendingGui stores a blank description as null). AdminParcelEditGui.button() passed the value straight to String.replace(), which throws NPE when the replacement is null, crashing the admin edit GUI on InventoryClickEvent. Coerce null placeholder values to an empty string via a testable nullToEmpty() helper and cover it with unit tests. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Egi4whB4fVKQL3XH4PpkX9 --- .../admin/AdminParcelEditGui.java | 10 +++++-- .../admin/AdminParcelEditGuiTest.java | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 src/test/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGuiTest.java diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java index bea26810a..60fa2f476 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java @@ -137,11 +137,17 @@ public void show(Player player) { private GuiItem button(ConfigItem template, String placeholder, String value, dev.triumphteam.gui.components.GuiAction action) { ConfigItem item = template.clone(); - return item.name(item.name().replace(placeholder, value)) - .lore(item.lore().stream().map(line -> line.replace(placeholder, value)).toList()) + String replacement = nullToEmpty(value); + return item.name(item.name().replace(placeholder, replacement)) + .lore(item.lore().stream().map(line -> line.replace(placeholder, replacement)).toList()) .toGuiItem(action); } + /** Coerces a nullable placeholder value to empty so {@link String#replace} never sees a null replacement. */ + static String nullToEmpty(String value) { + return value == null ? "" : value; + } + private void apply(Player player, CompletableFuture future) { future.thenAccept(result -> { this.notifyResult(player, result); diff --git a/src/test/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGuiTest.java b/src/test/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGuiTest.java new file mode 100644 index 000000000..32b466e5b --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGuiTest.java @@ -0,0 +1,30 @@ +package com.eternalcode.parcellockers.gui.implementation.admin; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +class AdminParcelEditGuiTest { + + @Test + @DisplayName("Should return empty string for a null placeholder value") + void nullToEmptyWhenValueIsNull() { + assertEquals("", AdminParcelEditGui.nullToEmpty(null)); + } + + @Test + @DisplayName("Should return the original value when not null") + void nullToEmptyWhenValueIsNotNull() { + assertEquals("desc", AdminParcelEditGui.nullToEmpty("desc")); + } + + @Test + @DisplayName("Should not throw when substituting a null parcel description into a template") + void replaceWithNullDescriptionDoesNotThrow() { + String template = "Description: {DESCRIPTION}"; + assertDoesNotThrow(() -> template.replace("{DESCRIPTION}", AdminParcelEditGui.nullToEmpty(null))); + assertEquals("Description: ", template.replace("{DESCRIPTION}", AdminParcelEditGui.nullToEmpty(null))); + } +}