Merge pull request #122 from SLNE-Development/copilot/sub-pr-113

Add ArrayDeque rationale and nesting depth limit to deep copy

Author: twisti-dev

surf-cloud-api/surf-cloud-api-common/src/main/kotlin/dev/slne/surf/cloud/api/common/player/ppdc/PersistentPlayerDataContainerView.kt

@@ -87,4 +87,23 @@ interface PersistentPlayerDataContainerView {
     fun writeToBuf(buf: SurfByteBuf)
 
     fun snapshot(): PersistentPlayerDataContainerView
+
+    companion object {
+        /**
+         * Maximum nesting depth for compound tags.
+         * This limit prevents memory exhaustion from extremely large nested structures.
+         * Set to a reasonable limit that should handle most legitimate use cases while
+         * protecting against pathological inputs.
+         */
+        const val MAX_NESTING_DEPTH = 512
+
+        inline fun ensureValidNestingDepth(
+            depth: Int,
+            exceptionFactory: (message: String) -> Throwable = ::IllegalStateException
+        ) {
+            if (depth > MAX_NESTING_DEPTH) {
+                throw exceptionFactory("Exceeded maximum allowed nesting depth of $MAX_NESTING_DEPTH. This likely indicates a corrupted or maliciously crafted data structure.")
+            }
+        }
+    }
 }

surf-cloud-core/surf-cloud-core-common/src/main/kotlin/dev/slne/surf/cloud/core/common/player/ppdc/PersistentPlayerDataContainerViewImpl.kt

@@ -143,8 +143,14 @@ abstract class PersistentPlayerDataContainerViewImpl : PersistentPlayerDataConta
      * to build a complete copy. It avoids stack overflow issues that can occur with deeply nested structures
      * when using a recursive approach.
      *
+     * Uses `ArrayDeque` instead of `Stack` for better performance characteristics:
+     * - `ArrayDeque` is not synchronized, making it faster for single-threaded use
+     * - `Stack` extends `Vector`, which has legacy synchronization overhead
+     * - `ArrayDeque` is the recommended implementation for stack operations in modern Java/Kotlin
+     *
      * @param root The root `CompoundBinaryTag` to be deep copied.
      * @return A deep copy of the specified `CompoundBinaryTag`.
+     * @throws IllegalStateException if the structure is too deeply nested (exceeds [MAX_NESTING_DEPTH])
      */
     private fun deepCopy(root: CompoundBinaryTag): CompoundBinaryTag {
         data class Frame(
@@ -181,6 +187,8 @@ abstract class PersistentPlayerDataContainerViewImpl : PersistentPlayerDataConta
                 val (key, value) = top.entries[top.idx++]
 
                 if (value is CompoundBinaryTag) {
+                    PersistentPlayerDataContainerView.ensureValidNestingDepth(stack.size)
+
                     stack.addLast(top)
                     stack.addLast(
                         Frame(

surf-cloud-core/surf-cloud-core-common/src/main/kotlin/dev/slne/surf/cloud/core/common/player/ppdc/network/PdcOp.kt

@@ -6,6 +6,7 @@ import dev.slne.surf.cloud.api.common.netty.protocol.buffer.readList
 import dev.slne.surf.cloud.api.common.netty.protocol.buffer.readUtf
 import dev.slne.surf.cloud.api.common.netty.protocol.buffer.writeCollection
 import dev.slne.surf.cloud.api.common.netty.protocol.buffer.writeUtf
+import dev.slne.surf.cloud.api.common.player.ppdc.PersistentPlayerDataContainerView
 import dev.slne.surf.cloud.api.common.util.ByIdMap
 import io.netty.buffer.ByteBuf
 import net.kyori.adventure.nbt.BinaryTag
@@ -19,12 +20,20 @@ sealed interface PdcOp {
         val value: BinaryTag
     ) : PdcOp {
         override val type = Type.PUT
+
+        init {
+            PersistentPlayerDataContainerView.ensureValidNestingDepth(path.size)
+        }
     }
 
     data class Remove(
         override val path: List<String>
     ) : PdcOp {
         override val type = Type.REMOVE
+
+        init {
+            PersistentPlayerDataContainerView.ensureValidNestingDepth(path.size)
+        }
     }
 
     companion object {