[Class] PacketProtocol - Easy packet interception [NMS]

This resource is no longer availabe.

 

Nice, but there are a few concurrency issues you should be aware of:

1. Your loop through packetListeners will be executed in the Netty worker threads, and it it thus not thread-safe to add (or remove - if you provide that option) listeners in the main thread after PacketProtocol has been constructed. Admittedly, this is not very likely to trigger in your example, as you only add a listener once, during construction. But it can happen, especially during plugin reloading when there are many players already online. And when it does, it crashes the caller with a ConcurrentModificationException.
   
   The fix is simple - just replace the ArrayList in packetListeners with a concurrent data structure, such as CopyOnWriteArrayList, or use locking.
   
2. Iterating over List<NetworkManager> in line 118 isn't thread safe either. You have to synchronize on the list before any for-loop, as per the official documentation (also see the Minecraft source code).
   
3. But these two can "only" crash the plugin. The real problem is that you're removing a channel handler in the main thread, which I know from experience can cause a deadlock and permanently freeze the server. More details here.
   
   To be fair, this does require intimate knowledge of the implementation of Netty in Minecraft, and I was unaware of it myself until it started causing problems in ProtocolLib. It's somewhat difficult to trigger, so I never got any complaints directed at TinyProtocol, even thought it is equally vulnerable.

Aside from that, this does have the advantage of being able to intercepting ping packets. But it would be nice if you could also intercept all the other packets in the LOGIN, STATUS and HANDSHAKE protocols.

And if we're going to compare features with TinyProtocol, I should mention that it is version independent without affecting performance. That way, you're not forced to recompile your plugin when CraftBukkit decides to increment the package version from v1_7_R2 to v1_7_R3 or v1_8_R0. Oh, and you should catch any exceptions in the packet listener - you probably don't want clients to disconnect just because a plugin malfunctioned.

Finally - you should consider returning a packet instance instead of a boolean in your PacketListener. This is useful if you, say, want the content of a packet to depend on the observer, but the packet instance is shared when broadcasted to a group of players. The only solution then is to clone the packet, before modifying it.

 

Comphenix How would one go about intercepting the LOGIN, STATUS and HANDSHAKE packets?

 

You basically have to insert your channel handler as early as possible. In ProtocolLib, I do that by inserting my own channel inbound handler in the pipeline that normally prepares a channel for a new connection.

In fact, there are many stages to this process:

1. Netty accepts a new connection from a client, and constructs a new channel wrapper around this connection.
2. ProtocolLib's own custom channel inbound handler inserts a channel initializer (beginInitProtocol) at the beginning of the channel's pipeline. I use a proxied list here, just in case.
3. ServerBootstrap now takes over (which is constructed here), and inserts Minecraft's custom channel initializer. into the channel's pipeline. This class is responsible for setting up the pipeline with packet encoders, decoders, splitters and prependers, along with hooking the NetworkManager up as the final reciever.
4. The channel is registered on a specific thread (EventLoop), which fires beginInitProtocol from step 2. All it does, is insert endInitProtocol at the end of the pipeline.
5. Next, Minecraft's own channel initializer is executed, and the entire vanilla pipeline is constructed. Note that all channel initializers are removed automatically when they're done preparing the pipeline.
6. Finally, endInitProtocol is executed, and ProtocolLib's own channel handlers can be inserted at the right locations in the pipeline.

The only problem here is that it's a bit long and complicated for something called TinyProtocol ...

 

Comphenix Ah I see. Thank you for the clear explenation

 

The class has been updated to V1.1, thanks to Comphenix

I updated the class and I think I fixed all problems you found (correct me if I'm wrong).

Btw, thanks for the great advises you always have for everyone, I really appriciate you put time in other people,

-BigTeddy98, Sander.

 

No problem.

Ah - you need to synchronize on the list, not PacketProtocol (which is what the synchronized method keyworddoes). So, in line 129, you need to do something the this:

Code:java

private synchronized void injectConnections() throws IllegalArgumentException, IllegalAccessException {
    // inject them all, to intercept our packets
    synchronized (list) {
        for (NetworkManager manager : list) {
            // inject connection
            injectConnection(manager);
        }
    }
}

You have to do the same for the for-loops accessing packetListeners (line 174 and 195). But, if you use CopyOnWriteArrayList, you don't have to synchronize at all, improving performance.

Another thing - shouldn't the PacketListener get a reference to the player that is receiving/sending the packet? At least include a SocketAddress.

Oh, and by the way - I added support for intercepting status and login packets in TinyProtocol as well. I'm intercepting packets all the way from the beginning, if you should have the need to handle PacketHandshakingInSetProtocol or PacketLoginInStart.

 

Aah, funny fact that now you also support Ping packets in TinyProtocol, I more like using TinyProtocol then my own class. But I will update it for other people haha.

- BigTeddy98, Sander.

 

What is the point of this when you return a NMS packet?

 

Does this also catch incoming Packets like 'PacketPlayInUseEntity' and get the player?

If so, could you update this to 1.7.R3, since I get a ClassCastException.
"CommandDispatcher cannot be cast to Serverconnection @ PacketProtocol.initialize (line-107)"
ServerConnection connection = (ServerConnection) PacketProticol.this.getServerConnection.get(server);


Thanks in advance!

 

If you still need to update, you just have to do two things:

• Update all NMS imports to net.minecraft.server.v1_7_R4
• Under the PacketProtocol constructor, this.getServerConnection change "o" to "p"

Like this. We can figure this out because of this.

 

sgtcaze iZanax
Better; remove all reflection except for privates.