Fix several crash/robustness issues in packet handling, line wrapping, and fluid GUI

KevinWoodWL · KevinWoodWL · commit 0a119a2f847e · 2026-06-08T16:53:11.000+08:00
PlayerPacketHandler: translate copies of item stacks and rebuild outgoing
packets instead of mutating shared NMS ItemStacks in place. The handler runs
on Netty's I/O thread while the same stacks may be touched by the main thread;
mutating their component maps races and can corrupt packets (observed as
ArrayIndexOutOfBoundsException / "Can't find id for 'null'" on 1.21.6+). Also
wrap packet rewriting in try/catch so a failure falls back to sending the
original packet rather than dropping the connection.

LineWrapping: guard against an infinite loop / StringIndexOutOfBounds when
wrapping text that has no spaces (e.g. CJK languages) or when a single token is
longer than the wrap limit. Handles empty content, non-positive remaining
width, and clamps the cut index to the content length.

FluidButton: make currentFluid nullable and resolve it via getOrNull so an
empty/0-size fluids list can no longer throw IndexOutOfBounds; fall back to a
shared error item and bail out of click handling when no fluid is available.

FluidIntersectionMarker: resolve the connected pipe defensively (pipeOrNull).
When the held intersection display or its pipe display entity has been lost,
getWaila now hides the WAILA and getPickItem returns null instead of NPE'ing
the WAILA update tick.
diff --git a/nms/src/main/kotlin/io/github/pylonmc/rebar/nms/packet/PlayerPacketHandler.kt b/nms/src/main/kotlin/io/github/pylonmc/rebar/nms/packet/PlayerPacketHandler.kt
@@ -7,9 +7,11 @@ import io.github.pylonmc.rebar.culling.BlockCullingEngine
 import io.github.pylonmc.rebar.i18n.PlayerTranslationHandler
 import io.github.pylonmc.rebar.nms.entity.BlockTextureEntityImpl
 import io.github.pylonmc.rebar.util.position.BlockPosition
+import com.mojang.datafixers.util.Pair
 import io.netty.channel.ChannelDuplexHandler
 import io.netty.channel.ChannelHandlerContext
 import io.netty.channel.ChannelPromise
+import net.minecraft.core.NonNullList
 import net.minecraft.network.HashedPatchMap
 import net.minecraft.network.HashedStack
 import net.minecraft.network.protocol.Packet
@@ -57,28 +59,58 @@ class PlayerPacketHandler(private val player: ServerPlayer, val handler: PlayerT
     private inner class PacketHandler : ChannelDuplexHandler() {
         override fun write(ctx: ChannelHandlerContext, packet: Any, promise: ChannelPromise) {
             @Suppress("UNCHECKED_CAST")
-            val packet = packet as? Packet<in ClientGamePacketListener> ?: return super.write(ctx, packet, promise)
-            super.write(ctx, handleOutgoingPacket(packet), promise)
+            val pkt = packet as? Packet<in ClientGamePacketListener>
+            if (pkt == null) {
+                super.write(ctx, packet, promise)
+                return
+            }
+
+            try {
+                super.write(ctx, handleOutgoingPacket(pkt), promise)
+            } catch (e: Exception) {
+                Rebar.logger.log(Level.WARNING, "Failed to rewrite outgoing packet, sending original", e)
+                super.write(ctx, pkt, promise)
+            }
         }
 
         override fun channelRead(ctx: ChannelHandlerContext, packet: Any) {
             @Suppress("UNCHECKED_CAST")
-            val packet = packet as? Packet<in ServerGamePacketListener> ?: return super.channelRead(ctx, packet)
-            super.channelRead(ctx, handleIncomingPacket(packet))
+            val pkt = packet as? Packet<in ServerGamePacketListener>
+            if (pkt == null) {
+                super.channelRead(ctx, packet)
+                return
+            }
+
+            try {
+                super.channelRead(ctx, handleIncomingPacket(pkt))
+            } catch (e: Exception) {
+                Rebar.logger.log(Level.WARNING, "Failed to rewrite incoming packet, forwarding original", e)
+                super.channelRead(ctx, pkt)
+            }
         }
     }
 
     private fun handleOutgoingPacket(packet: Packet<in ClientGamePacketListener>): Packet<in ClientGamePacketListener> =
         when (packet) {
             is ClientboundBundlePacket -> ClientboundBundlePacket(packet.subPackets().map(::handleOutgoingPacket))
 
-            is ClientboundContainerSetContentPacket -> packet.apply {
-                items.forEach(::translate)
-                translate(packet.carriedItem)
-            }
+            // Translate copies and rebuild the packet, never modify inventory/equipment stacks in place.
+            // These stacks may be shared with server-side state while this handler runs on Netty's I/O
+            // thread, and mutating their component maps can corrupt packets on 1.21.6+ servers.
+            is ClientboundContainerSetContentPacket -> ClientboundContainerSetContentPacket(
+                packet.containerId,
+                packet.stateId,
+                NonNullList.of(ItemStack.EMPTY, *packet.items.map { translate(it.copy()) }.toTypedArray()),
+                translate(packet.carriedItem.copy())
+            )
 
-            is ClientboundContainerSetSlotPacket -> packet.apply { translate(item) }
-            is ClientboundSetCursorItemPacket -> packet.apply { translate(contents) }
+            is ClientboundContainerSetSlotPacket -> ClientboundContainerSetSlotPacket(
+                packet.containerId,
+                packet.stateId,
+                packet.slot,
+                translate(packet.item.copy())
+            )
+            is ClientboundSetCursorItemPacket -> ClientboundSetCursorItemPacket(translate(packet.contents.copy()))
             is ClientboundRecipeBookAddPacket -> ClientboundRecipeBookAddPacket(
                 packet.entries.map {
                     ClientboundRecipeBookAddPacket.Entry(
@@ -136,11 +168,10 @@ class PlayerPacketHandler(private val player: ServerPlayer, val handler: PlayerT
                 )
             }
 
-            is ClientboundSetEquipmentPacket -> packet.apply {
-                slots.forEach { slot ->
-                    translate(slot.second)
-                }
-            }
+            is ClientboundSetEquipmentPacket -> ClientboundSetEquipmentPacket(
+                packet.entity,
+                packet.slots.map { Pair.of(it.first, translate(it.second.copy())) }
+            )
 
             is ClientboundBlockUpdatePacket -> packet.let {
                 val cullingJob = BlockCullingEngine.getCullingJob(player.uuid) ?: return@let it
diff --git a/rebar/src/main/kotlin/io/github/pylonmc/rebar/content/fluid/FluidIntersectionMarker.kt b/rebar/src/main/kotlin/io/github/pylonmc/rebar/content/fluid/FluidIntersectionMarker.kt
@@ -49,19 +49,27 @@ class FluidIntersectionMarker : RebarBlock, EntityHolderRebarBlock, BlockBreakRe
         }
     }
 
-    override fun getWaila(player: Player): WailaDisplay?
-        = WailaDisplay(defaultWailaTranslationKey.arguments(RebarArgument.of("pipe", this.pipe.stack.effectiveName())))
+    override fun getWaila(player: Player): WailaDisplay? {
+        val pipeItem = pipeOrNull() ?: return null
+        return WailaDisplay(defaultWailaTranslationKey.arguments(RebarArgument.of("pipe", pipeItem.stack.effectiveName())))
+    }
 
     val pipe: RebarItem
-        get() {
-            check(fluidIntersectionDisplay.connectedPipeDisplays.isNotEmpty())
-            val uuid = fluidIntersectionDisplay.connectedPipeDisplays.iterator().next()
-            return EntityStorage.getAs<FluidPipeDisplay?>(uuid)!!.pipe
+        get() = pipeOrNull() ?: error("FluidIntersectionMarker has no resolvable pipe display")
+
+    private fun pipeOrNull(): RebarItem? {
+        val display = try {
+            fluidIntersectionDisplay
+        } catch (_: Throwable) {
+            return null
         }
+        val uuid = display.connectedPipeDisplays.firstOrNull() ?: return null
+        return EntityStorage.getAs<FluidPipeDisplay?>(uuid)?.pipe
+    }
 
     override fun getDropItem(context: BlockBreakContext) = null
 
-    override fun getPickItem(player: Player) = pipe.stack
+    override fun getPickItem(player: Player) = pipeOrNull()?.stack
 
     companion object {
         @JvmField
diff --git a/rebar/src/main/kotlin/io/github/pylonmc/rebar/guide/button/FluidButton.kt b/rebar/src/main/kotlin/io/github/pylonmc/rebar/guide/button/FluidButton.kt
@@ -15,6 +15,7 @@ import org.bukkit.entity.Player
 import org.bukkit.event.inventory.ClickType
 import xyz.xenondevs.invui.Click
 import xyz.xenondevs.invui.item.AbstractItem
+import xyz.xenondevs.invui.item.ItemProvider
 
 /**
  * Represents a fluid in the guide.
@@ -52,44 +53,49 @@ open class FluidButton(
     constructor(input: RecipeInput.Fluid) : this(input.amountMillibuckets, *input.fluids.toTypedArray())
 
     val fluids = fluids.shuffled()
-    val currentFluid: RebarFluid
-        get() = this.fluids[(Bukkit.getCurrentTick() / 20) % this.fluids.size]
+    val currentFluid: RebarFluid?
+        get() = this.fluids.getOrNull((Bukkit.getCurrentTick() / 20) % this.fluids.size)
 
     init {
         require(fluids.isNotEmpty()) { "Fluids list cannot be empty" }
     }
 
     override fun getUpdatePeriod(what: Int): Int = if (fluids.size > 1) 20 else -1
 
-    override fun getItemProvider(player: Player) = try {
-        val stack = if (amount == null) {
-            preDisplayDecorator.invoke(ItemStackBuilder.of(currentFluid.item))
+    override fun getItemProvider(player: Player): ItemProvider = try {
+        val fluid = currentFluid ?: return buildErrorItem()
+        val itemStack = fluid.item
+
+        if (amount == null) {
+            preDisplayDecorator.invoke(ItemStackBuilder.of(itemStack))
         } else {
-            preDisplayDecorator.invoke(ItemStackBuilder.of(currentFluid.item))
+            preDisplayDecorator.invoke(ItemStackBuilder.of(itemStack))
                 .name(
                     Component.translatable(
                         "rebar.guide.button.fluid.name",
-                        RebarArgument.of("fluid", currentFluid.item.getData(DataComponentTypes.ITEM_NAME)!!),
+                        RebarArgument.of("fluid", itemStack.getData(DataComponentTypes.ITEM_NAME)!!),
                         RebarArgument.of("amount", UnitFormat.MILLIBUCKETS.format(amount).decimalPlaces(2))
                     )
                 )
         }
-        stack
     } catch (e: Exception) {
         e.printStackTrace()
-        ItemStackBuilder.of(Material.BARRIER)
-            .name(Component.translatable("rebar.guide.button.fluid.error"))
+        buildErrorItem()
     }
 
+    private fun buildErrorItem() = ItemStackBuilder.of(Material.BARRIER)
+        .name(Component.translatable("rebar.guide.button.fluid.error"))
+
     override fun handleClick(clickType: ClickType, player: Player, click: Click) {
         try {
+            val fluid = currentFluid ?: return
             if (clickType.isLeftClick) {
-                val page = FluidRecipesPage(currentFluid.key)
+                val page = FluidRecipesPage(fluid.key)
                 if (page.pages.isNotEmpty()) {
                     page.open(player)
                 }
             } else {
-                val page = FluidUsagesPage(currentFluid)
+                val page = FluidUsagesPage(fluid)
                 if (page.pages.isNotEmpty()) {
                     page.open(player)
                 }
diff --git a/rebar/src/main/kotlin/io/github/pylonmc/rebar/i18n/LineWrapping.kt b/rebar/src/main/kotlin/io/github/pylonmc/rebar/i18n/LineWrapping.kt
@@ -90,18 +90,33 @@ private fun wrapLine(
         // which are handled clientside) but this is fine, it can't be perfect.
         var content = component.content()
         while (currentLineLength + content.length > RebarConfig.TRANSLATION_WRAP_LIMIT) {
+            if (content.isEmpty()) {
+                break
+            }
 
             // Make sure we snap to the end of a word (i.e. don't cut words in half)
-            var endIndex = RebarConfig.TRANSLATION_WRAP_LIMIT - currentLineLength
-            while (endIndex != 0 && content[endIndex] != ' ') {
+            val maxCharsOnLine = RebarConfig.TRANSLATION_WRAP_LIMIT - currentLineLength
+            if (maxCharsOnLine <= 0) {
+                lines.add(currentLine)
+                currentLine = DEFAULT_COMPONENT
+                currentLineLength = 0
+                continue
+            }
+
+            var endIndex = maxCharsOnLine.coerceAtMost(content.length)
+            while (endIndex > 0 && endIndex < content.length && content[endIndex] != ' ') {
                 endIndex -= 1
             }
 
+            if (endIndex <= 0) {
+                endIndex = maxCharsOnLine.coerceAtMost(content.length)
+            }
+
             currentLine = currentLine.append(Component.text(content.substring(0, endIndex)).style(style))
             lines.add(currentLine)
             currentLine = DEFAULT_COMPONENT
             currentLineLength = 0
-            content = if (endIndex+1 == content.length) "" else content.substring(endIndex+1)
+            content = if (endIndex >= content.length) "" else content.substring(endIndex + 1)
         }
         currentLine = currentLine.append(Component.text(content).style(style))
         currentLineLength += content.length
