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

fix invalid packet types due to state mismatch when calling packet events #2568

Merged
merged 1 commit into from
Oct 25, 2023
Merged

fix invalid packet types due to state mismatch when calling packet events #2568

merged 1 commit into from
Oct 25, 2023

Conversation

derklaro
Copy link
Contributor

In 1.20.2 Mojang decided to share some packet classes between the play and configuration state. This leads to an issue that resolving the packet type by class is no longer possible, as it either returns only the play or the configuration state packet type. Therefore we need to register packets based on 2 keys: protocol and class.

I also rewrote how we retreive the protocol state, as in 1.20.2 the client and server direction can be in different protocol states. The resolve process works by using the protocol specific AttributeKey(s):

  • in 1.8 - 1.20.1 there is only a single key named "protocol" that directly holds the protocol state
  • in 1.20.2 there are no two keys (for client and server) that hold a CodecData object that contains the protocol state

When sending or receiving a packet the protocol state is resolved from the wrapped channel and then the correct packet type is selected when calling the packet event. This fix was confirmed to work by the reporter in #2552 (comment) .

For the future Mojang might decide to not even make two classes for the same packet anymore (for example KeepAlive could be shared between states and directions). In that case we need to extend the packet registry to also handle that, but for now this fix does the job good enough imo.

Closes #2561, Closes #2552

@Potothingi
Copy link

When will this be merged? Should I directly build and use this PR?

@SrBedrock
Copy link

SrBedrock commented Oct 17, 2023

I'm getting this error when using the version with this PR, is it InteractiveChat's fault?

[18:43:28 ERROR]: [InteractiveChat] Unhandled exception occurred in onPacketReceiving(PacketEvent) for InteractiveChat
com.comphenix.protocol.reflect.FieldAccessException: Field index 0 is out of bounds for length 0
        at com.comphenix.protocol.reflect.FieldAccessException.fromFormat(FieldAccessException.java:49) ~[ProtocolLib.jar:?]
        at com.comphenix.protocol.reflect.StructureModifier.read(StructureModifier.java:245) ~[ProtocolLib.jar:?]
        at com.loohp.interactivechat.listeners.ClientSettingPacket.handlePacketReceiving(ClientSettingPacket.java:88) ~[InteractiveChat-4.2.8.0.jar:?]
        at com.loohp.interactivechat.listeners.ClientSettingPacket$1.onPacketReceiving(ClientSettingPacket.java:64) ~[InteractiveChat-4.2.8.0.jar:?]
        at com.comphenix.protocol.injector.SortedPacketListenerList.invokeReceivingListener(SortedPacketListenerList.java:122) ~[ProtocolLib.jar:?]
        at com.comphenix.protocol.injector.SortedPacketListenerList.invokePacketRecieving(SortedPacketListenerList.java:75) ~[ProtocolLib.jar:?]
        at com.comphenix.protocol.injector.PacketFilterManager.postPacketToListeners(PacketFilterManager.java:555) ~[ProtocolLib.jar:?]
        at com.comphenix.protocol.injector.PacketFilterManager.invokePacketReceiving(PacketFilterManager.java:519) ~[ProtocolLib.jar:?]
        at com.comphenix.protocol.injector.netty.manager.NetworkManagerInjector.onPacketReceiving(NetworkManagerInjector.java:119) ~[ProtocolLib.jar:?]
        at com.comphenix.protocol.injector.netty.channel.NettyChannelInjector.processInboundPacket(NettyChannelInjector.java:488) ~[ProtocolLib.jar:?]
        at com.comphenix.protocol.injector.netty.channel.InboundPacketInterceptor.channelRead(InboundPacketInterceptor.java:33) ~[ProtocolLib.jar:?]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346) ~[netty-codec-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318) ~[netty-codec-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103) ~[netty-codec-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346) ~[netty-codec-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:333) ~[netty-codec-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:454) ~[netty-codec-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290) ~[netty-codec-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286) ~[netty-handler-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.handler.flush.FlushConsolidationHandler.channelRead(FlushConsolidationHandler.java:152) ~[netty-handler-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919) ~[netty-transport-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:800) ~[netty-transport-classes-epoll-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.epoll.EpollEventLoop.processReady(EpollEventLoop.java:509) ~[netty-transport-classes-epoll-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:407) ~[netty-transport-classes-epoll-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997) ~[netty-common-4.1.97.Final.jar:4.1.97.Final]
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) ~[netty-common-4.1.97.Final.jar:4.1.97.Final]
        at java.lang.Thread.run(Thread.java:833) ~[?:?]

@derklaro
Copy link
Contributor Author

@SrBedrock yes, it's not related with this PR (maybe indirectly, but still not the PRs fault)

@SrBedrock
Copy link

When using the master version the error does not occur, I will wait for the PR to be accepted to report it to the plugin dev.

@derklaro
Copy link
Contributor Author

@LOOHP you mind taking a look?

@LOOHP
Copy link
Contributor

LOOHP commented Oct 17, 2023

@SrBedrock Mind if I know which version of Minecraft you produced that error?

@SrBedrock
Copy link

@LOOHP

[19:29:08 INFO]: Checking version, please wait...
[19:29:08 INFO]: Current: git-Purpur-2082 (MC: 1.20.2)*
Previous: git-Purpur-2081 (MC: 1.20.2)
* You are running the latest version

@LOOHP
Copy link
Contributor

LOOHP commented Oct 17, 2023

@derklaro Here's the code that registered the packet listener and here's is the line of code that threw the error.

As far as I'm aware, PacketType.Play.Client.SETTINGS should have been replaced by PacketType.Configuration.Client.CLIENT_INFORMATION on 1.20.2 and therefore it's listeners should never have been invoked, but in this case it still is. 🤔
Or did I just overlook it? That PacketType.Play.Client.SETTINGS actually still exists and just had its class name changed?

@dmulloy2 dmulloy2 merged commit af33a2a into dmulloy2:master Oct 25, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants