network: correct exception for packet not implemented

src/main/java/de/bixilon/minosoft/data/registries/versions/Versions.kt

@@ -21,7 +21,6 @@ import de.bixilon.kutil.primitive.IntUtil.toInt
 import de.bixilon.minosoft.Minosoft
 import de.bixilon.minosoft.assets.util.FileUtil.readJson
 import de.bixilon.minosoft.protocol.packets.factory.C2SPacketType
-import de.bixilon.minosoft.protocol.packets.factory.PacketDirection
 import de.bixilon.minosoft.protocol.packets.factory.PacketTypeRegistry
 import de.bixilon.minosoft.protocol.packets.factory.S2CPacketType
 import de.bixilon.minosoft.protocol.protocol.ProtocolStates
@@ -64,14 +63,14 @@ object Versions : Iterable<Version> {
                     when (val s2c = mapping["s2c"]) {
                         is List<*> -> {
                             // just play
-                            s2cPackets = mapOf(ProtocolStates.PLAY to readPacketMapping(versionId, PacketDirection.SERVER_TO_CLIENT, s2c.unsafeCast()) { PacketTypeRegistry.getS2C(ProtocolStates.PLAY, it) })
+                            s2cPackets = mapOf(ProtocolStates.PLAY to readS2PPacketMapping(versionId, ProtocolStates.PLAY, s2c.unsafeCast()))
                         }
                         is Map<*, *> -> {
                             // map other states
                             val packets: MutableMap<ProtocolStates, AbstractBiMap<S2CPacketType, Int>> = mutableMapOf()
                             for ((stateName, packetMapping) in s2c) {
                                 val state = ProtocolStates[stateName.toString()]
-                                packets[state] = readPacketMapping(versionId, PacketDirection.SERVER_TO_CLIENT, packetMapping.unsafeCast()) { PacketTypeRegistry.getS2C(state, it) }
+                                packets[state] = readS2PPacketMapping(versionId, state, packetMapping.unsafeCast())
                             }
                             s2cPackets = packets
                         }
@@ -79,14 +78,14 @@ object Versions : Iterable<Version> {
                     }
                     when (val c2s = mapping["c2s"]) {
                         is List<*> -> {
-                            c2sPackets = mapOf(ProtocolStates.PLAY to readPacketMapping(versionId, PacketDirection.CLIENT_TO_SERVER, c2s.unsafeCast()) { PacketTypeRegistry.getC2S(ProtocolStates.PLAY, it) })
+                            c2sPackets = mapOf(ProtocolStates.PLAY to readC2SPacketMapping(versionId, ProtocolStates.PLAY, c2s.unsafeCast()))
                         }
                         is Map<*, *> -> {
                             // map other states
                             val packets: MutableMap<ProtocolStates, AbstractBiMap<C2SPacketType, Int>> = mutableMapOf()
                             for ((stateName, packetMapping) in c2s) {
                                 val state = ProtocolStates[stateName.toString()]
-                                packets[state] = readPacketMapping(versionId, PacketDirection.CLIENT_TO_SERVER, packetMapping.unsafeCast()) { PacketTypeRegistry.getC2S(state, it) }
+                                packets[state] = readC2SPacketMapping(versionId, state, packetMapping.unsafeCast())
                             }
                             c2sPackets = packets
                         }
@@ -115,21 +114,31 @@ object Versions : Iterable<Version> {
         }
     }
 
-    private fun <T> readPacketMapping(versionId: Int, direction: PacketDirection, list: List<String>, typeGetter: (name: String) -> T?): MutableBiMap<T, Int> {
+    private fun <T> readPacketMapping(versionId: Int, list: List<String>, typeGetter: (name: String) -> T): MutableBiMap<T, Int> {
         val map: MutableBiMap<T, Int> = mutableBiMapOf()
-        var packetId = 0 // To not mess up ids when packet is not registered
         for (name in list) {
             val packetType = typeGetter(name)
-            if (packetType == null) {
+            map.put(packetType, map.size)?.let { Log.log(LogMessageType.VERSION_LOADING, LogLevels.WARN) { "Packet $name registered twice (version=$versionId)" } }
-                Log.log(LogMessageType.VERSION_LOADING, LogLevels.WARN) { "Packet $name is not registered (versionId=$versionId, direction=$direction)!" }
-                packetId++
-                continue
-            }
-            map.put(packetType, packetId++)?.let { Log.log(LogMessageType.VERSION_LOADING, LogLevels.WARN) { "Packet $name registered twice (version=$versionId)" } }
         }
         return map
     }
 
+    private fun readS2PPacketMapping(versionId: Int, state: ProtocolStates, list: List<String>): AbstractBiMap<S2CPacketType, Int> {
+        return readPacketMapping(versionId, list) {
+            PacketTypeRegistry.getS2C(state, it)?.let { type -> return@readPacketMapping type }
+            Log.log(LogMessageType.VERSION_LOADING, LogLevels.WARN) { "Packet $it is not registered (versionId=$versionId, state=$state, direction=SERVER_TO_CLIENT)!" }
+            return@readPacketMapping S2CPacketType.EMPTY()
+        }
+    }
+
+    private fun readC2SPacketMapping(versionId: Int, state: ProtocolStates, list: List<String>): AbstractBiMap<C2SPacketType, Int> {
+        return readPacketMapping(versionId, list) {
+            PacketTypeRegistry.getC2S(state, it)?.let { type -> return@readPacketMapping type }
+            Log.log(LogMessageType.VERSION_LOADING, LogLevels.WARN) { "Packet $it is not registered (versionId=$versionId, state=$state, direction=CLIENT_TO_SERVER)!" }
+            return@readPacketMapping C2SPacketType.EMPTY()
+        }
+    }
+
     operator fun get(name: String?): Version? {
         if (name == "automatic") {
             return AUTOMATIC

src/main/java/de/bixilon/minosoft/protocol/network/network/client/exceptions/implementation/S2CPacketNotImplementedException.kt

@@ -13,11 +13,15 @@
 
 package de.bixilon.minosoft.protocol.network.network.client.exceptions.implementation
 
+import de.bixilon.minosoft.data.registries.versions.Version
 import de.bixilon.minosoft.protocol.network.network.client.exceptions.NetworkException
-import de.bixilon.minosoft.protocol.packets.factory.S2CPacketType
+import de.bixilon.minosoft.protocol.protocol.ProtocolStates
+import de.bixilon.minosoft.util.KUtil.toHex
 
 class S2CPacketNotImplementedException(
-    val packetType: S2CPacketType,
+    val packetId: Int,
+    val state: ProtocolStates,
+    val version: Version?,
 ) : NetworkException() {
-    override val message: String = "$packetType"
+    override val message: String = "packetId=0x${packetId.toHex()}, state=$state, version=$version"
 }

src/main/java/de/bixilon/minosoft/protocol/network/network/client/pipeline/ClientPacketHandler.kt

@@ -28,7 +28,6 @@ import de.bixilon.minosoft.protocol.network.network.client.NettyClient
 import de.bixilon.minosoft.protocol.network.network.client.exceptions.NetworkException
 import de.bixilon.minosoft.protocol.network.network.client.exceptions.PacketHandleException
 import de.bixilon.minosoft.protocol.network.network.client.exceptions.WrongConnectionException
-import de.bixilon.minosoft.protocol.packets.factory.PacketTypeRegistry
 import de.bixilon.minosoft.protocol.packets.factory.S2CPacketType
 import de.bixilon.minosoft.protocol.packets.s2c.PlayS2CPacket
 import de.bixilon.minosoft.protocol.packets.s2c.S2CPacket
@@ -38,7 +37,7 @@ import io.netty.channel.SimpleChannelInboundHandler
 
 class ClientPacketHandler(
     private val client: NettyClient,
-) : SimpleChannelInboundHandler<S2CPacket>() {
+) : SimpleChannelInboundHandler<QueuedS2CP<*>>() {
     private val connection: Connection = client.connection
     private val handlers: MutableSet<ThreadPoolRunnable> = synchronizedSetOf()
 
@@ -52,15 +51,14 @@ class ClientPacketHandler(
         }
     }
 
-    override fun channelRead0(context: ChannelHandlerContext, packet: S2CPacket) {
+    override fun channelRead0(context: ChannelHandlerContext, queued: QueuedS2CP<*>) {
-        val type = PacketTypeRegistry.getS2C(packet::class.java) ?: throw IllegalStateException("Packet type is null?")
+        if (queued.type.threadSafe) {
-        if (type.threadSafe) {
             val runnable = ThreadPoolRunnable()
-            runnable.runnable = Runnable { tryHandle(type, packet);handlers -= runnable }
+            runnable.runnable = Runnable { tryHandle(queued.type, queued.packet);handlers -= runnable }
             handlers += runnable
             DefaultThreadPool += runnable
         } else {
-            tryHandle(type, packet)
+            tryHandle(queued.type, queued.packet)
         }
     }
 

src/main/java/de/bixilon/minosoft/protocol/network/network/client/pipeline/ExceptionHandler.kt

@@ -21,23 +21,28 @@ import de.bixilon.minosoft.util.logging.LogLevels
 import de.bixilon.minosoft.util.logging.LogMessageType
 import io.netty.channel.ChannelDuplexHandler
 import io.netty.channel.ChannelHandlerContext
+import io.netty.handler.codec.DecoderException
+import io.netty.handler.codec.EncoderException
 
 class ExceptionHandler(
     private val client: NettyClient,
 ) : ChannelDuplexHandler() {
 
     override fun exceptionCaught(context: ChannelHandlerContext, cause: Throwable) {
-        Log.log(LogMessageType.NETWORK_PACKETS_IN, LogLevels.WARN) { cause }
+        var realCause = cause
-        if (cause !is NetworkException) {
+        if (cause is DecoderException) {
-            client.disconnect()
+            realCause = cause.cause ?: cause
-            return
+        } else if (cause is EncoderException) {
+            realCause = cause.cause ?: cause
         }
-        if (cause is CriticalNetworkException) {
+        Log.log(LogMessageType.NETWORK_PACKETS_IN, LogLevels.WARN) { realCause }
+        if (realCause !is NetworkException || realCause is CriticalNetworkException) {
             client.disconnect()
             return
         }
     }
 
+
     companion object {
         const val NAME = "exception_handler"
     }

src/main/java/de/bixilon/minosoft/protocol/network/network/client/pipeline/QueuedS2CP.kt

@@ -0,0 +1,22 @@
+/*
+ * Minosoft
+ * Copyright (C) 2020-2022 Moritz Zwerger
+ *
+ * This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with this program. If not, see <https://www.gnu.org/licenses/>.
+ *
+ * This software is not affiliated with Mojang AB, the original developer of Minecraft.
+ */
+
+package de.bixilon.minosoft.protocol.network.network.client.pipeline
+
+import de.bixilon.minosoft.protocol.packets.factory.S2CPacketType
+import de.bixilon.minosoft.protocol.packets.s2c.S2CPacket
+
+data class QueuedS2CP<T : S2CPacket>(
+    val type: S2CPacketType,
+    val packet: T,
+)

src/main/java/de/bixilon/minosoft/protocol/network/network/client/pipeline/encoding/PacketDecoder.kt

@@ -22,6 +22,7 @@ import de.bixilon.minosoft.protocol.network.network.client.exceptions.PacketBuff
 import de.bixilon.minosoft.protocol.network.network.client.exceptions.PacketReadException
 import de.bixilon.minosoft.protocol.network.network.client.exceptions.ciritical.UnknownPacketIdException
 import de.bixilon.minosoft.protocol.network.network.client.exceptions.implementation.S2CPacketNotImplementedException
+import de.bixilon.minosoft.protocol.network.network.client.pipeline.QueuedS2CP
 import de.bixilon.minosoft.protocol.packets.factory.S2CPacketType
 import de.bixilon.minosoft.protocol.packets.s2c.S2CPacket
 import de.bixilon.minosoft.protocol.protocol.InByteBuffer
@@ -44,6 +45,10 @@ class PacketDecoder(
 
         val packetType = version?.s2cPackets?.get(state)?.getKey(packetId) ?: Protocol.S2C_PACKET_MAPPING[state]?.getKey(packetId) ?: throw UnknownPacketIdException(packetId, state, version)
 
+        if (packetType.clazz == S2CPacket::class.java) {
+            throw S2CPacketNotImplementedException(packetId, state, version)
+        }
+
         val packet = try {
             readPacket(packetType, data)
         } catch (exception: NetworkException) {
@@ -54,7 +59,7 @@ class PacketDecoder(
             throw PacketReadException(error)
         }
 
-        out += packet
+        out += QueuedS2CP(packetType, packet)
     }
 
     private fun readPacket(type: S2CPacketType, data: ByteArray): S2CPacket {
@@ -63,7 +68,7 @@ class PacketDecoder(
         } else {
             InByteBuffer(data)
         }
-        val packet = type.factory?.createPacket(buffer) ?: throw S2CPacketNotImplementedException(type)
+        val packet = type.factory?.createPacket(buffer) ?: throw IllegalStateException("Packet factory is null?")
         if (buffer.pointer < buffer.size) {
             throw PacketBufferUnderflowException(type, buffer.size, buffer.pointer)
         }

src/main/java/de/bixilon/minosoft/protocol/packets/factory/C2SPacketType.kt

@@ -19,12 +19,16 @@ import de.bixilon.minosoft.protocol.protocol.ProtocolStates
 class C2SPacketType(
     val state: ProtocolStates,
     val clazz: Class<out C2SPacket>,
-    val annotation: LoadPacket = clazz.getAnnotation(LoadPacket::class.java),
+    val annotation: LoadPacket?,
+    override val threadSafe: Boolean = annotation!!.threadSafe,
 ) : AbstractPacketType {
     override val direction = PacketDirection.CLIENT_TO_SERVER
-    override val threadSafe: Boolean get() = annotation.threadSafe
 
     override fun toString(): String {
         return clazz.toString()
     }
+
+    companion object {
+        val EMPTY = { C2SPacketType(ProtocolStates.HANDSHAKING, C2SPacket::class.java, null, false) }
+    }
 }

src/main/java/de/bixilon/minosoft/protocol/packets/factory/S2CPacketType.kt

@@ -23,11 +23,11 @@ class S2CPacketType(
     val state: ProtocolStates,
     val clazz: Class<out S2CPacket>,
     private val packetErrorHandler: PacketErrorHandler?,
-    val annotation: LoadPacket = clazz.getAnnotation(LoadPacket::class.java),
+    val annotation: LoadPacket?,
     val factory: PacketFactory? = null,
+    override val threadSafe: Boolean = annotation!!.threadSafe,
 ) : AbstractPacketType, PacketErrorHandler {
     override val direction = PacketDirection.SERVER_TO_CLIENT
-    override val threadSafe: Boolean get() = annotation.threadSafe
 
 
     override fun onError(error: Throwable, connection: Connection) {
@@ -37,4 +37,9 @@ class S2CPacketType(
     override fun toString(): String {
         return clazz.toString()
     }
+
+
+    companion object {
+        val EMPTY = { S2CPacketType(ProtocolStates.HANDSHAKING, S2CPacket::class.java, null, null, null, false) }
+    }
 }