Skip to content

Commit

Permalink
#3519: Queue configuration phase packets from API methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@md-5
md-5 committed Sep 23, 2023
1 parent f486a25 commit 0509303
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 9 deletions.
17 changes: 14 additions & 3 deletions protocol/src/main/java/net/md_5/bungee/protocol/Protocol.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,8 @@ public enum Protocol
/*========================================================================*/
public static final int MAX_PACKET_ID = 0xFF;
/*========================================================================*/
final DirectionData TO_SERVER = new DirectionData( this, ProtocolConstants.Direction.TO_SERVER );
final DirectionData TO_CLIENT = new DirectionData( this, ProtocolConstants.Direction.TO_CLIENT );
public final DirectionData TO_SERVER = new DirectionData( this, ProtocolConstants.Direction.TO_SERVER );
public final DirectionData TO_CLIENT = new DirectionData( this, ProtocolConstants.Direction.TO_CLIENT );

public static void main(String[] args)
{
Expand Down Expand Up @@ -719,7 +719,7 @@ private static ProtocolMapping map(int protocol, int id)
return new ProtocolMapping( protocol, id );
}

static final class DirectionData
public static final class DirectionData
{

private final TIntObjectMap<ProtocolData> protocols = new TIntObjectHashMap<>();
Expand Down Expand Up @@ -802,6 +802,17 @@ private void registerPacket(Class<? extends DefinedPacket> packetClass, Supplier
}
}

public boolean hasPacket(Class<? extends DefinedPacket> packet, int version)
{
ProtocolData protocolData = getProtocolData( version );
if ( protocolData == null )
{
throw new BadPacketException( "Unsupported protocol version" );
}

return protocolData.packetMap.containsKey( packet );
}

final int getId(Class<? extends DefinedPacket> packet, int version)
{

Expand Down
35 changes: 29 additions & 6 deletions proxy/src/main/java/net/md_5/bungee/UserConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Objects;
import java.util.Queue;
import java.util.UUID;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.logging.Level;
import lombok.Getter;
import lombok.NonNull;
Expand Down Expand Up @@ -142,6 +143,7 @@ public final class UserConnection implements ProxiedPlayer
@Setter
private ForgeServerHandler forgeServerHandler;
/*========================================================================*/
private final Queue<DefinedPacket> packetQueue = new ConcurrentLinkedQueue<>();
private final Unsafe unsafe = new Unsafe()
{
@Override
Expand Down Expand Up @@ -177,6 +179,27 @@ public void sendPacket(PacketWrapper packet)
ch.write( packet );
}

public void sendPacketQueued(DefinedPacket packet)
{
Protocol encodeProtocol = ch.getEncodeProtocol();
if ( encodeProtocol != Protocol.GAME && !encodeProtocol.TO_CLIENT.hasPacket( packet.getClass(), getPendingConnection().getVersion() ) )
{
packetQueue.add( packet );
} else
{
unsafe().sendPacket( packet );
}
}

public void sendQueuedPackets()
{
DefinedPacket packet;
while ( ( packet = packetQueue.poll() ) != null )
{
unsafe().sendPacket( packet );
}
}

@Deprecated
public boolean isActive()
{
Expand Down Expand Up @@ -489,10 +512,10 @@ private void sendMessage(ChatMessageType position, UUID sender, String message)
position = ChatMessageType.SYSTEM;
}

unsafe().sendPacket( new SystemChat( message, position.ordinal() ) );
sendPacketQueued( new SystemChat( message, position.ordinal() ) );
} else
{
unsafe().sendPacket( new Chat( message, (byte) position.ordinal(), sender ) );
sendPacketQueued( new Chat( message, (byte) position.ordinal(), sender ) );
}
}

Expand All @@ -513,7 +536,7 @@ private void sendMessage(ChatMessageType position, UUID sender, BaseComponent...
net.md_5.bungee.protocol.packet.Title title = new net.md_5.bungee.protocol.packet.Title();
title.setAction( net.md_5.bungee.protocol.packet.Title.Action.ACTIONBAR );
title.setText( ComponentSerializer.toString( message ) );
unsafe.sendPacket( title );
sendPacketQueued( title );
}
} else
{
Expand All @@ -524,7 +547,7 @@ private void sendMessage(ChatMessageType position, UUID sender, BaseComponent...
@Override
public void sendData(String channel, byte[] data)
{
unsafe().sendPacket( new PluginMessage( channel, data, forgeClientHandler.isForgeUser() ) );
sendPacketQueued( new PluginMessage( channel, data, forgeClientHandler.isForgeUser() ) );
}

@Override
Expand Down Expand Up @@ -700,7 +723,7 @@ public void setTabHeader(BaseComponent header, BaseComponent footer)
header = ChatComponentTransformer.getInstance().transform( this, true, header )[0];
footer = ChatComponentTransformer.getInstance().transform( this, true, footer )[0];

unsafe().sendPacket( new PlayerListHeaderFooter(
sendPacketQueued( new PlayerListHeaderFooter(
ComponentSerializer.toString( header ),
ComponentSerializer.toString( footer )
) );
Expand All @@ -712,7 +735,7 @@ public void setTabHeader(BaseComponent[] header, BaseComponent[] footer)
header = ChatComponentTransformer.getInstance().transform( this, true, header );
footer = ChatComponentTransformer.getInstance().transform( this, true, footer );

unsafe().sendPacket( new PlayerListHeaderFooter(
sendPacketQueued( new PlayerListHeaderFooter(
ComponentSerializer.toString( header ),
ComponentSerializer.toString( footer )
) );
Expand Down
9 changes: 9 additions & 0 deletions proxy/src/main/java/net/md_5/bungee/connection/UpstreamBridge.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import net.md_5.bungee.protocol.packet.ClientChat;
import net.md_5.bungee.protocol.packet.ClientCommand;
import net.md_5.bungee.protocol.packet.ClientSettings;
import net.md_5.bungee.protocol.packet.FinishConfiguration;
import net.md_5.bungee.protocol.packet.KeepAlive;
import net.md_5.bungee.protocol.packet.LoginAcknowledged;
import net.md_5.bungee.protocol.packet.PlayerListItem;
Expand Down Expand Up @@ -334,6 +335,14 @@ public void handle(StartConfiguration startConfiguration) throws Exception
}
}

@Override
public void handle(FinishConfiguration finishConfiguration) throws Exception
{
con.sendQueuedPackets();

super.handle( finishConfiguration );
}

@Override
public String toString()
{
Expand Down

8 comments on commit 0509303

@bob7l
Copy link
Contributor

@bob7l bob7l commented on 0509303 Sep 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There will need to be a queue for downstream packets too, like Server#sendData (PluginMessage), since plugins expect the user to be in GAME state during the ServerSwitchEvent for example. BungeeTitle will also need to be using the new queue function as it's calling Unsafe#sendPacket still

Then the to-server queue can be flushed on DownstreamBridge#handle(Login) to somewhat retain the plugin developers intentions.

@md-5
Copy link
Member Author

@md-5 md-5 commented on 0509303 Sep 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PluginMessage is part of the configuration protocol so that shouldn't be an issue

@xism4
Copy link
Contributor

@xism4 xism4 commented on 0509303 Sep 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably some plugins use internals

@NEZNAMY
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I request all occurences of player.unsafe().sendPacket in class https://github.com/SpigotMC/BungeeCord/blob/master/proxy/src/main/java/net/md_5/bungee/tab/ServerUnique.java to be replaced with player.sendPacketQueued as well? I'm using the update methods to rely on BungeeCord's tablist entry tracker to remove them all on server switch. This change cannot be done from plugin side.

@md-5
Copy link
Member Author

@md-5 md-5 commented on 0509303 Sep 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear what you're asking / doing given that isn't API

@bob7l
Copy link
Contributor

@bob7l bob7l commented on 0509303 Sep 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PluginMessage is part of the configuration protocol so that shouldn't be an issue

True if the player is joining for the first time, but if they join, then do /server other, it'll throw an exception if a plugin is sending a PluginMessage downstream: https://pastebin.com/cR4EFQgk

Must be a bug then. This only happens if a plugin is sending a payload on the ServerSwitchEvent to the downstream server, just to reiterate

Example code:

   @EventHandler
    public void onServerSwitch(ServerSwitchEvent e) {
        e.getPlayer().getServer().sendData("fish",new byte[] {1, 2, 3, 4, 5, 6, 7, 8, 9});
    }

@NEZNAMY
Copy link

@NEZNAMY NEZNAMY commented on 0509303 Sep 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the API gives me this (also thrown when using sendPacketQueued directly for other packets):

java.lang.NullPointerException: Cannot invoke "net.md_5.bungee.protocol.MinecraftEncoder.getProtocol()" because the return value of "io.netty.channel.ChannelPipeline.get(java.lang.Class)" is null
at net.md_5.bungee.netty.ChannelWrapper.getEncodeProtocol(ChannelWrapper.java:51)
at net.md_5.bungee.UserConnection.sendPacketQueued(UserConnection.java:184)
at net.md_5.bungee.UserConnection.setTabHeader(UserConnection.java:726)

And this:

io.netty.handler.codec.EncoderException: java.lang.IllegalArgumentException: Cannot get ID for packet class net.md_5.bungee.protocol.packet.PluginMessage in phase LOGIN with direction TO_SERVER
	at io.netty.handler.codec.MessageToByteEncoder.write(MessageToByteEncoder.java:125)
	at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:881)
	at io.netty.channel.AbstractChannelHandlerContext.invokeWriteAndFlush(AbstractChannelHandlerContext.java:940)
	at io.netty.channel.AbstractChannelHandlerContext$WriteTask.run(AbstractChannelHandlerContext.java:1247)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:470)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:566)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalArgumentException: Cannot get ID for packet class net.md_5.bungee.protocol.packet.PluginMessage in phase LOGIN with direction TO_SERVER
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:463)
	at net.md_5.bungee.protocol.Protocol$DirectionData.getId(Protocol.java:824)
	at net.md_5.bungee.protocol.MinecraftEncoder.encode(MinecraftEncoder.java:25)
	at net.md_5.bungee.protocol.MinecraftEncoder.encode(MinecraftEncoder.java:10)
	at io.netty.handler.codec.MessageToByteEncoder.write(MessageToByteEncoder.java:107)
	... 10 more

When sendPacketQueued does not throw, the packet may get lost. Then, on server switch, I get disconnected with:

io.netty.handler.codec.EncoderException: java.lang.IllegalArgumentException: Cannot get ID for packet class net.md_5.bungee.protocol.packet.StartConfiguration in phase CONFIGURATION with direction TO_CLIENT
	at io.netty.handler.codec.MessageToByteEncoder.write(MessageToByteEncoder.java:125)
	at io.netty.channel.AbstractChannelHandlerContext.invokeWrite0(AbstractChannelHandlerContext.java:881)
	at io.netty.channel.AbstractChannelHandlerContext.invokeWriteAndFlush(AbstractChannelHandlerContext.java:940)
	at io.netty.channel.AbstractChannelHandlerContext.write(AbstractChannelHandlerContext.java:966)
	at io.netty.channel.AbstractChannelHandlerContext.writeAndFlush(AbstractChannelHandlerContext.java:934)
	at io.netty.channel.DefaultChannelPipeline.writeAndFlush(DefaultChannelPipeline.java:1020)
	at io.netty.channel.AbstractChannel.writeAndFlush(AbstractChannel.java:311)
	at net.md_5.bungee.netty.ChannelWrapper.write(ChannelWrapper.java:85)
	at net.md_5.bungee.UserConnection$1.sendPacket(UserConnection.java:152)
	at net.md_5.bungee.ServerConnector.cutThrough(ServerConnector.java:334)
	at net.md_5.bungee.ServerConnector.handle(ServerConnector.java:153)
	at net.md_5.bungee.protocol.packet.LoginSuccess.handle(LoginSuccess.java:62)
	at net.md_5.bungee.netty.HandlerBoss.channelRead(HandlerBoss.java:124)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.timeout.IdleStateHandler.channelRead(IdleStateHandler.java:286)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalArgumentException: Cannot get ID for packet class net.md_5.bungee.protocol.packet.StartConfiguration in phase CONFIGURATION with direction TO_CLIENT
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:463)
	at net.md_5.bungee.protocol.Protocol$DirectionData.getId(Protocol.java:824)
	at net.md_5.bungee.protocol.MinecraftEncoder.encode(MinecraftEncoder.java:25)
	at net.md_5.bungee.protocol.MinecraftEncoder.encode(MinecraftEncoder.java:10)
	at io.netty.handler.codec.MessageToByteEncoder.write(MessageToByteEncoder.java:107)
	... 44 more
14:21:21 [INFO] [_NEZNAMY_] disconnected with: §fEncoderException : java.lang.IllegalArgumentException: Cannot get ID for packet class net.md_5.bungee.protocol.packet.StartConfiguration in phase CONFIGURATION with direction TO_CLIENT @ io.netty.handler.codec.MessageToByteEncoder:125

Not clear what you're asking / doing given that isn't API

There isn't any TabList API, so I'm sending packets. Instead of using sendPacket directly, I am calling that method to make the packet go though entry tracker that automatically clears all entries on server switch, so I don't need to do that anymore. I'll just force my way into the uuid collection using reflection.

@md-5
Copy link
Member Author

@md-5 md-5 commented on 0509303 Sep 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please open a separate ticket for any bugs or API requests.

True if the player is joining for the first time, but if they join, then do /server other, it'll throw an exception if a plugin is sending a PluginMessage downstream: https://pastebin.com/cR4EFQgk

I'm looking into this one, but I think it is difficult to fix given the way bungee is architected. A queue would be a temporary fix, but not correct

Please sign in to comment.