Skip to content
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.

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

Disconnected while attempting to connect to a server due to failed packet parsing #1272

Open
Fabelz opened this issue Mar 18, 2024 · 4 comments

Comments

@Fabelz
Copy link

Fabelz commented Mar 18, 2024

While tracking down another bug I'd caused in a plugin, I'd stumbled upon an issue regarding the way that velocity handles it's server connections for clients on 1.20.2 and above due to the configuration state.

Players will be disconnected with the following message if you attempt to send them to a server while a connection to another server is already in progress. This did not happen to clients prior to 1.20.2's release due to the configuration state not existing.

image

This is due to the player's state being switched to CONFIGURATION pre-server send, I believe, which doesn't allow the client to parse any play-related packets, resulting in the client kicking itself because it thinks that it has found an incorrect packet ID.

To replicate, it is relatively simple:

  1. Make yourself a /send command that does not have a cooldown
  2. Rapidly send yourself to three servers (not in the same instant, a few 100ms delays should do you good!)
  3. Disconnection should occur if the servers and client support the configuration state and if the player fails to connect to one of the servers due to a connection being "in progress".

This does not happen 100% of the time, as it is wholly dependent on the stage that the client is at when it is parsing packets.

@UserNugget
Copy link
Contributor

UserNugget commented Mar 20, 2024

Thats a client-side race condition as the client send ChatSession PLAY-state packet asynchronously.
Client can easly try to send it at the CONFIG state, and so disconnecting itself due to a protocol error.

You can disable chat signing as a workaround (like changing uuid via GameProfileRequestEvent, that will prevent the client from enabling chat signing, but will require uuid rewriting to work properly)

@Fabelz
Copy link
Author

Fabelz commented Mar 20, 2024

Thats a client-side race condition as the client send ChatSession PLAY-state packet asynchronously. Client can easly try to send it at the CONFIG state, and so disconnecting itself due to a protocol error.

You can disable chat signing as a workaround (like changing uuid via GameProfileRequestEvent, that will prevent the client from enabling chat signing)

As stated here, it is a workaround but it is unfortunately not a fix. I don't experience this problem anymore as I've avoided the initial issue that caused the players to be sent to the servers while there is a connection already in progress, but it doesn't help our commands to forcibly send players (or mass send all players, which would be a headache) to servers.

This should really be patched on velocity's end, as the server itself wouldn't be sending the join packet to request the chat session unless itself was fully out of the configuration state, which it is. I'm suspecting velocity somehow temporarily throws the client in to the configuration state while it's attempting to figure out whether it should send them to another server or not, which it shouldn't do (although admittedly I don't touch velocity's backend enough to know anything about the way it operates, I'm just making assumptions based off of the packet logging I've done).

@R00tB33rMan
Copy link
Contributor

R00tB33rMan commented May 14, 2024

[10:27:22] [Render thread/INFO]: Connecting to play.freshsmp.fun, 25565
[10:27:22] [Netty Epoll Server Login IO #0/INFO]: [STDERR]: java.freshsmp.fun./23.163.152.9:25565 LOGIN io.netty.handler.codec.EncoderException: java.io.IOException: Can't serialize unregistered packet
[10:27:22] [Netty Epoll Server Login IO #0/ERROR]: Packet error
io.netty.handler.codec.EncoderException: java.io.IOException: Can't serialize unregistered packet
at io.netty.handler.codec.MessageToByteEncoder.write(MessageToByteEncoder.java:125) ~[netty-codec-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:881) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.invokeWrite(AbstractChannelHandlerContext.java:863) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:968) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:856) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.handler.codec.MessageToMessageEncoder.write(MessageToMessageEncoder.java:113) ~[netty-codec-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:881) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.invokeWriteAndFlush(AbstractChannelHandlerContext.java:940) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:966) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.writeAndFlush(AbstractChannelHandlerContext.java:934) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannelHandlerContext.writeAndFlush(AbstractChannelHandlerContext.java:984) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.DefaultChannelPipeline.writeAndFlush(DefaultChannelPipeline.java:1025) ~[netty-transport-4.1.97.Final.jar:?]
at io.netty.channel.AbstractChannel.writeAndFlush(AbstractChannel.java:306) ~[netty-transport-4.1.97.Final.jar:?]
at net.minecraft.class_2535.method_36942(class_2535.java:321) ~[client-intermediary.jar:?]
at net.minecraft.class_2535.method_52917(class_2535.java:316) ~[client-intermediary.jar:?]
at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174) ~[netty-common-4.1.97.Final.jar:?]
at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:167) ~[netty-common-4.1.97.Final.jar:?]
at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470) ~[netty-common-4.1.97.Final.jar:?]
at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:416) ~[netty-transport-classes-epoll-4.1.97.Final.jar:?]
at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997) ~[netty-common-4.1.97.Final.jar:?]
at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) ~[netty-common-4.1.97.Final.jar:?]
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) ~[netty-common-4.1.97.Final.jar:?]
at java.lang.Thread.run(Thread.java:1570) ~[?:?]
Caused by: java.io.IOException: Can't serialize unregistered packet
at net.minecraft.class_2545.method_10838(class_2545.java:39) ~[client-intermediary.jar:?]
at net.minecraft.class_2545.encode(class_2545.java:15) ~[client-intermediary.jar:?]
at io.netty.handler.codec.MessageToByteEncoder.write(MessageToByteEncoder.java:107) ~[netty-codec-4.1.97.Final.jar:?]
... 22 more

Here's an example of this occurring to me with potentially relevant and useful logs for fixing this issue.

@UserNugget
Copy link
Contributor

Ideally it should be fixed client-side as fixing it properly at server-side will be "very hard" due to client sometimes break or delay chat signing initialization by itself. It can be partially fixed by waiting for ChatSession packet before trying to switch to the configuration state, but it will not fix this race condition completly, like if the client is overloaded with receiving skins, http requests is blocked client-side, background thread pool is overloaded or client uuid is changed by the server, client will delay or even will never send ChatSession packet.

Also, looks like Mojang fixed some client-side configuration state race conditions in 1.20.5, but this one is left untouched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants