[🐞]: `mod_version_check_packet` breaks Velocity proxy support

Sander0542 commented 3 weeks ago

Before Continuing:

[X] Always check you are using the latest version of the mods and its dependencies
[X] Remove mod that enhances Minecraft: Optifine, Sodium, others. The issue still persists.
[X] If you are unsure which mod is the culprit. Disable all of your mods and enable them 1-2 mods each time to isolate the culprit
[X] Confirm that there is no existing issue with a similar description submitted in the list of issues.

Version

1.20.1

Loader

Forge

EveryCompat Version

everycomp-1.20-2.6.66

Moonlight Lib Version

moonlight-1.20-2.12.6-forge

Issue with mods

Velocity 3.0.0

This is not another mod, but a minecraft server proxy.

Issue Detail

When the mod_version_check_packet setting is enabled, this mod sends a packet to the server to query the correct versions. This packet contains the Integer.MIN_VALUE which causes the proxy to disconnect the client.

The following section of code causes the client to disconnect

https://github.com/MehVahdJukaar/WoodGood/blob/e71d856801c7cc003b284d35d4816d5954f11fdf/forge/src/main/java/net/mehvahdjukaar/every_compat/forge/EveryCompatForge.java#L169-L176

More information can be found in the following ticket: https://github.com/PaperMC/Velocity/issues/1370

OPTIONAL: Latest.log | Crash-report Attachment

The following log comes from the Velocity proxy, not the mod.

[20:18:40] [Netty epoll Worker #1/ERROR] [com.velocitypowered.proxy.connection.MinecraftConnection]: [server connection] Sander0542 -> valhelsia6: exception encountered in org.adde0109.ambassador.velocity.backend.ForgeLoginSessionHandler@e1cc3f2
io.netty.handler.codec.CorruptedFrameException: Error decoding class com.velocitypowered.proxy.protocol.packet.LoginPluginMessagePacket Direction CLIENTBOUND Protocol 1.20 State LOGIN ID 4
    at com.velocitypowered.proxy.protocol.netty.MinecraftDecoder.handleDecodeFailure(MinecraftDecoder.java:130) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at com.velocitypowered.proxy.protocol.netty.MinecraftDecoder.tryDecode(MinecraftDecoder.java:85) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at com.velocitypowered.proxy.protocol.netty.MinecraftDecoder.channelRead(MinecraftDecoder.java:60) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:800) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:509) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:407) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at java.base/java.lang.Thread.run(Unknown Source) [?:?]
Caused by: io.netty.handler.codec.CorruptedFrameException: Bad VarInt decoded
    at com.velocitypowered.proxy.protocol.ProtocolUtils.readVarInt(ProtocolUtils.java:132) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at com.velocitypowered.proxy.protocol.packet.LoginPluginMessagePacket.decode(LoginPluginMessagePacket.java:66) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    at com.velocitypowered.proxy.protocol.netty.MinecraftDecoder.tryDecode(MinecraftDecoder.java:83) ~[velocity-3.3.0-SNAPSHOT-400.jar:3.3.0-SNAPSHOT (git-9d25d309-b400)]
    ... 24 more

OPTIONAL: To Produce

Configure Velocity and the backend Forge server.
Connect to the proxy server, which forwards the connection to the backend Forge server.
Get disconnected because of an invalid packet.

Xelbayria commented 3 weeks ago

U need to update your EveryComp to the latest version, v2.6.72 & Moonlight Lib to v2.12.14. Try test them to see if the issue persist.

MehVahdJukaar commented 3 weeks ago

Hm what? Integer min value where? That packet works and thus must be valid for vanilla so I don't see how it could be our issue. The packet contains a map of strings and map encoding is done by vanilla code. If there's any integer is there. Rest is strings. https://github.com/MehVahdJukaar/WoodGood/blob/e71d856801c7cc003b284d35d4816d5954f11fdf/common/src/main/java/net/mehvahdjukaar/every_compat/ECNetworking.java#L45

Xelbayria commented 2 weeks ago

Any news on this issue?

456dev commented 3 days ago

the int here is message id: see https://wiki.vg/Protocol#Login_Plugin_Request this here is filled in by forge in the internals of the simple network channel as int.minvalue as a getter is not provided on creation.

while int.min value is a valid value for it, it shouldnt occur in normal gameplay (as the docs say it should be unique vs all other messages).

int.min value was previously treated as a sentinel value for parsing error in velocity, and is fixed as of build 416, released ~20 days ago

i think the correct way of doing it is with the loginIndexGetter and loginIndexSetter on SimpleChannel.MessageBuilder, but there is very little docs

MehVahdJukaar commented 3 days ago

I still dont know where this -1 would be in my code. If you mean its in the message id its not. Those message have Ids 0 and 1. An also even if they were to use a negative or -1 value, if forge allowed it would still be a velocity issue. If forge allows the packet then its not my issue

MehVahdJukaar / WoodGood