Skip to content

Commit

Permalink
Improve/cleanup injector (#2992)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ingrim4 committed Jun 17, 2024
1 parent 2cad996 commit 47bcd53
Show file tree
Hide file tree
Showing 28 changed files with 526 additions and 851 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -607,8 +607,6 @@ private void processPacket(int workerID, PacketEvent packet, String methodName)

} catch (OutOfMemoryError e) {
throw e;
} catch (ThreadDeath e) {
throw e;
} catch (Throwable e) {
// Minecraft doesn't want your Exception.
filterManager.getErrorReporter().reportMinimal(listener.getPlugin(), methodName, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import org.apache.commons.lang.builder.ToStringBuilder;
import org.apache.commons.lang.builder.ToStringStyle;
import org.bukkit.Bukkit;
import org.bukkit.plugin.Plugin;

import com.comphenix.protocol.ProtocolConfig;
import com.comphenix.protocol.ProtocolLib;
import com.comphenix.protocol.ProtocolLibrary;
import com.comphenix.protocol.ProtocolLogger;
import com.comphenix.protocol.collections.ExpireHashMap;
Expand All @@ -41,11 +45,6 @@
import com.google.common.base.Preconditions;
import com.google.common.primitives.Primitives;

import org.apache.commons.lang.builder.ToStringBuilder;
import org.apache.commons.lang.builder.ToStringStyle;
import org.bukkit.Bukkit;
import org.bukkit.plugin.Plugin;

/**
* Internal class used to handle exceptions.
*
Expand Down Expand Up @@ -479,7 +478,7 @@ public static String getStringDescription(Object value) {
} catch (LinkageError ex) {
// Apache is probably missing
apacheCommonsMissing = true;
} catch (ThreadDeath | OutOfMemoryError e) {
} catch (OutOfMemoryError e) {
throw e;
} catch (Throwable ex) {
// Don't use the error logger to log errors in error logging (that could lead to infinite loops)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public enum ListenerOptions {
* Disable the automatic game phase detection that will normally force {@link GamePhase#LOGIN} when a packet ID is
* known to be transmitted during login.
*/
@Deprecated
DISABLE_GAMEPHASE_DETECTION,

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@

package com.comphenix.protocol.events;

import java.util.*;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Set;

import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.injector.GamePhase;
Expand Down Expand Up @@ -90,20 +95,6 @@ public static Builder newBuilder(ListeningWhitelist template) {
return new Builder(template);
}

/**
* Construct a copy of a given enum.
*
* @param options - the options to copy, or NULL to indicate the empty set.
* @return A copy of the enum set.
*/
private static <T extends Enum<T>> EnumSet<T> safeEnumSet(Collection<T> options, Class<T> enumClass) {
if (options != null && !options.isEmpty()) {
return EnumSet.copyOf(options);
} else {
return EnumSet.noneOf(enumClass);
}
}

/**
* Construct a copy of a given set.
*
Expand Down Expand Up @@ -311,6 +302,7 @@ public Builder types(Collection<PacketType> types) {
* @param gamePhase - the gamephase.
* @return This builder, for chaining.
*/
@Deprecated
public Builder gamePhase(GamePhase gamePhase) {
this.gamePhase = gamePhase;
return this;
Expand All @@ -321,6 +313,7 @@ public Builder gamePhase(GamePhase gamePhase) {
*
* @return This builder, for chaining.
*/
@Deprecated
public Builder gamePhaseBoth() {
return gamePhase(GamePhase.BOTH);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*
* @author Kristian
*/
@Deprecated
public enum GamePhase {
/**
* Only listen for packets sent or received before a player has logged in.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package com.comphenix.protocol.injector;

import java.util.Deque;

import com.comphenix.protocol.ProtocolLibrary;
import com.comphenix.protocol.ProtocolManager;
import com.comphenix.protocol.error.ErrorReporter;
import com.comphenix.protocol.events.NetworkMarker;
import com.comphenix.protocol.events.PacketEvent;
import com.comphenix.protocol.events.PacketPostListener;
import com.comphenix.protocol.events.ScheduledPacket;
import java.util.Deque;

/**
* Represents a processor for network markers.
Expand Down Expand Up @@ -43,7 +44,7 @@ public void invokePostEvent(PacketEvent event, NetworkMarker marker) {
for (PacketPostListener listener : marker.getPostListeners()) {
try {
listener.onPostEvent(event);
} catch (OutOfMemoryError | ThreadDeath e) {
} catch (OutOfMemoryError e) {
throw e;
} catch (Throwable e) {
this.reporter.reportMinimal(listener.getPlugin(), "SentListener.run()", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,6 @@ public PacketConstructor withPacket(PacketType type, Object[] values) {
result = unwrapper.unwrapItem(values[i]);
} catch (OutOfMemoryError e) {
throw e;
} catch (ThreadDeath e) {
throw e;
} catch (Throwable e) {
lastException = e;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,10 @@
import com.comphenix.protocol.injector.netty.WirePacket;
import com.comphenix.protocol.injector.netty.manager.NetworkManagerInjector;
import com.comphenix.protocol.injector.packet.PacketRegistry;
import com.comphenix.protocol.injector.player.PlayerInjectionHandler;
import com.comphenix.protocol.injector.player.PlayerInjectionHandler.ConflictStrategy;
import com.comphenix.protocol.utility.MinecraftReflection;
import com.comphenix.protocol.utility.MinecraftVersion;
import com.google.common.collect.ImmutableSet;

import io.netty.channel.Channel;

public class PacketFilterManager implements ListenerInvoker, InternalManager {

// plugin verifier reports
Expand All @@ -80,7 +76,6 @@ public class PacketFilterManager implements ListenerInvoker, InternalManager {
private final Set<PacketListener> registeredListeners;

// injectors
private final PlayerInjectionHandler playerInjectionHandler;
private final NetworkManagerInjector networkManagerInjector;

// status of this manager
Expand Down Expand Up @@ -111,18 +106,16 @@ public PacketFilterManager(PacketFilterBuilder builder) {
// injectors
this.networkManagerInjector = new NetworkManagerInjector(
builder.getLibrary(),
builder.getServer(),
this,
builder.getReporter());
this.playerInjectionHandler = this.networkManagerInjector.getPlayerInjectionHandler();

// ensure that all packet types are loaded and synced
PacketRegistry.getClientPacketTypes();
PacketRegistry.getServerPacketTypes();

// hook into all connected players
for (Player player : this.server.getOnlinePlayers()) {
this.playerInjectionHandler.injectPlayer(player, ConflictStrategy.BAIL_OUT);
this.networkManagerInjector.getInjector(player).inject();
}
}

Expand Down Expand Up @@ -180,7 +173,7 @@ public void sendServerPacket(Player receiver, PacketContainer packet, NetworkMar
}

// process outbound
this.playerInjectionHandler.sendServerPacket(receiver, packet, marker, filters);
this.networkManagerInjector.getInjector(receiver).sendServerPacket(packet.getHandle(), marker, filters);
}
}

Expand All @@ -192,13 +185,7 @@ public void sendWirePacket(Player receiver, int id, byte[] bytes) {
@Override
public void sendWirePacket(Player receiver, WirePacket packet) {
if (!this.closed) {
// special case - we just throw the wire packet down the pipeline without processing it
Channel outboundChannel = this.playerInjectionHandler.getChannel(receiver);
if (outboundChannel == null) {
throw new IllegalArgumentException("Unable to obtain connection of player " + receiver);
}

outboundChannel.writeAndFlush(packet);
this.networkManagerInjector.getInjector(receiver).sendWirePacket(packet);
}
}

Expand Down Expand Up @@ -240,13 +227,13 @@ public void receiveClientPacket(Player sender, PacketContainer packet, NetworkMa
}

// post to the player inject, reset our cancel state change
this.playerInjectionHandler.receiveClientPacket(sender, nmsPacket);
this.networkManagerInjector.getInjector(sender).receiveClientPacket(nmsPacket);
}
}

@Override
public int getProtocolVersion(Player player) {
return this.playerInjectionHandler.getProtocolVersion(player);
return this.networkManagerInjector.getInjector(player).getProtocolVersion();
}

@Override
Expand Down Expand Up @@ -447,12 +434,12 @@ public void registerEvents(PluginManager manager, Plugin plugin) {

@EventHandler(priority = EventPriority.LOWEST)
public void handleLogin(PlayerLoginEvent event) {
PacketFilterManager.this.playerInjectionHandler.updatePlayer(event.getPlayer());
networkManagerInjector.getInjector(event.getPlayer()).inject();
}

@EventHandler(priority = EventPriority.LOWEST)
public void handleJoin(PlayerJoinEvent event) {
PacketFilterManager.this.playerInjectionHandler.updatePlayer(event.getPlayer());
networkManagerInjector.getInjector(event.getPlayer()).inject();
}

@EventHandler(priority = EventPriority.MONITOR)
Expand Down
40 changes: 9 additions & 31 deletions src/main/java/com/comphenix/protocol/injector/netty/Injector.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package com.comphenix.protocol.injector.netty;

import java.net.SocketAddress;

import org.bukkit.entity.Player;

import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.PacketType.Protocol;
import com.comphenix.protocol.events.NetworkMarker;
import org.bukkit.entity.Player;

/**
* Represents an injected client connection.
Expand All @@ -23,12 +26,8 @@ public interface Injector {
* Inject the current channel.
* <p>
* Note that only active channels can be injected.
*
* @return TRUE if we injected the channel, false if we could not inject or it was already injected.
*/
boolean inject();

void uninject();
void inject();

/**
* Close the current injector.
Expand All @@ -46,16 +45,7 @@ public interface Injector {

void receiveClientPacket(Object packet);

/**
* Retrieve the current protocol state.
*
* @return The current protocol.
* @deprecated use {@link #getCurrentProtocol(PacketType.Sender)} instead.
*/
@Deprecated
default Protocol getCurrentProtocol() {
return this.getCurrentProtocol(PacketType.Sender.SERVER);
}
void sendWirePacket(WirePacket packet);

/**
* Retrieve the current protocol state. Note that since 1.20.2 the client and server direction can be in different
Expand All @@ -66,21 +56,7 @@ default Protocol getCurrentProtocol() {
*/
Protocol getCurrentProtocol(PacketType.Sender sender);

/**
* Retrieve the network marker associated with a given packet.
*
* @param packet - the packet.
* @return The network marker.
*/
NetworkMarker getMarker(Object packet);

/**
* Associate a given network marker with a specific packet.
*
* @param packet - the NMS packet.
* @param marker - the associated marker.
*/
void saveMarker(Object packet, NetworkMarker marker);
SocketAddress getAddress();

/**
* Retrieve the current player or temporary player associated with the injector.
Expand All @@ -98,6 +74,8 @@ default Protocol getCurrentProtocol() {

void disconnect(String message);

boolean isConnected();

/**
* Determine if the channel has already been injected.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
package com.comphenix.protocol.injector.netty.channel;

import java.net.SocketAddress;

import org.bukkit.entity.Player;

import com.comphenix.protocol.PacketType;
import com.comphenix.protocol.PacketType.Protocol;
import com.comphenix.protocol.events.NetworkMarker;
import com.comphenix.protocol.injector.netty.Injector;
import org.bukkit.entity.Player;
import com.comphenix.protocol.injector.netty.WirePacket;

final class EmptyInjector implements Injector {

Expand All @@ -17,17 +21,20 @@ public EmptyInjector(Player player) {
}

@Override
public int getProtocolVersion() {
return Integer.MIN_VALUE;
public SocketAddress getAddress() {
if (this.player != null) {
return this.player.getAddress();
}
return null;
}

@Override
public boolean inject() {
return false;
public int getProtocolVersion() {
return Integer.MIN_VALUE;
}

@Override
public void uninject() {
public void inject() {
}

@Override
Expand All @@ -43,17 +50,12 @@ public void receiveClientPacket(Object packet) {
}

@Override
public Protocol getCurrentProtocol(PacketType.Sender sender) {
return Protocol.HANDSHAKING;
}

@Override
public NetworkMarker getMarker(Object packet) {
return null;
public void sendWirePacket(WirePacket packet) {
}

@Override
public void saveMarker(Object packet, NetworkMarker marker) {
public Protocol getCurrentProtocol(PacketType.Sender sender) {
return Protocol.HANDSHAKING;
}

@Override
Expand All @@ -70,6 +72,11 @@ public void setPlayer(Player player) {
public void disconnect(String message) {
}

@Override
public boolean isConnected() {
return false;
}

@Override
public boolean isInjected() {
return false;
Expand Down
Loading

0 comments on commit 47bcd53

Please sign in to comment.