From d387b2d792c6fe556e1c1de4fa92b9922c0363e7 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Sun, 3 Mar 2013 14:49:20 +0100 Subject: [PATCH] Improve client packet interception by about 37%. --- .../injector/packet/ProxyPacketInjector.java | 58 +++++++++++++------ .../injector/packet/ReadPacketModifier.java | 43 +++++--------- 2 files changed, 55 insertions(+), 46 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ProxyPacketInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ProxyPacketInjector.java index f4224bba..391c321f 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ProxyPacketInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ProxyPacketInjector.java @@ -23,13 +23,13 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; - import org.bukkit.entity.Player; import net.sf.cglib.proxy.Callback; import net.sf.cglib.proxy.Enhancer; import net.sf.cglib.proxy.Factory; +import net.sf.cglib.proxy.CallbackFilter; +import net.sf.cglib.proxy.NoOp; import com.comphenix.protocol.Packets; import com.comphenix.protocol.error.ErrorReporter; @@ -39,6 +39,8 @@ import com.comphenix.protocol.injector.ListenerInvoker; import com.comphenix.protocol.injector.player.PlayerInjectionHandler; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; +import com.comphenix.protocol.reflect.MethodInfo; +import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract; import com.comphenix.protocol.utility.MinecraftReflection; /** @@ -47,7 +49,15 @@ import com.comphenix.protocol.utility.MinecraftReflection; * @author Kristian */ class ProxyPacketInjector implements PacketInjector { - + /** + * Matches the readPacketData(DataInputStream) method in Packet. + */ + private static FuzzyMethodContract readPacket = FuzzyMethodContract.newBuilder(). + returnTypeVoid(). + parameterExactType(DataInputStream.class). + parameterCount(1). + build(); + // The "put" method that associates a packet ID with a packet class private static Method putMethod; private static Object intHashMap; @@ -61,11 +71,11 @@ class ProxyPacketInjector implements PacketInjector { // Allows us to determine the sender private PlayerInjectionHandler playerInjection; - // Allows us to look up read packet injectors - private Map readModifier; - // Class loader private ClassLoader classLoader; + + // Share callback filter + private CallbackFilter filter; public ProxyPacketInjector(ClassLoader classLoader, ListenerInvoker manager, PlayerInjectionHandler playerInjection, ErrorReporter reporter) throws IllegalAccessException { @@ -74,7 +84,6 @@ class ProxyPacketInjector implements PacketInjector { this.manager = manager; this.playerInjection = playerInjection; this.reporter = reporter; - this.readModifier = new ConcurrentHashMap(); initialize(); } @@ -85,11 +94,9 @@ class ProxyPacketInjector implements PacketInjector { */ @Override public void undoCancel(Integer id, Object packet) { - ReadPacketModifier modifier = readModifier.get(id); - // See if this packet has been cancelled before - if (modifier != null && modifier.hasCancelled(packet)) { - modifier.removeOverride(packet); + if (ReadPacketModifier.hasCancelled(packet)) { + ReadPacketModifier.removeOverride(packet); } } @@ -137,18 +144,34 @@ class ProxyPacketInjector implements PacketInjector { throw new IllegalStateException("Packet " + packetID + " has already been injected."); } + if (filter == null) { + filter = new CallbackFilter() { + @Override + public int accept(Method method) { + // Skip methods defined in Object + if (method.getDeclaringClass().equals(Object.class)) + return 0; + else if (readPacket.isMatch(MethodInfo.fromMethod(method), null)) + return 1; + else + return 2; + } + }; + } + // Subclass the specific packet class ex.setSuperclass(old); - ex.setCallbackType(ReadPacketModifier.class); + ex.setCallbackFilter(filter); + ex.setCallbackTypes(new Class[] { NoOp.class, ReadPacketModifier.class, ReadPacketModifier.class }); ex.setClassLoader(classLoader); Class proxy = ex.createClass(); - // Create the proxy handler - ReadPacketModifier modifier = new ReadPacketModifier(packetID, this, reporter); - readModifier.put(packetID, modifier); - + // Create the proxy handlers + ReadPacketModifier modifierReadPacket = new ReadPacketModifier(packetID, this, reporter, true); + ReadPacketModifier modifierRest = new ReadPacketModifier(packetID, this, reporter, false); + // Add a static reference - Enhancer.registerStaticCallbacks(proxy, new Callback[] { modifier }); + Enhancer.registerStaticCallbacks(proxy, new Callback[] { NoOp.INSTANCE, modifierReadPacket, modifierRest }); try { // Override values @@ -184,7 +207,6 @@ class ProxyPacketInjector implements PacketInjector { putMethod.invoke(intHashMap, packetID, old); previous.remove(packetID); - readModifier.remove(packetID); registry.remove(proxy); overwritten.remove(packetID); return true; diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ReadPacketModifier.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ReadPacketModifier.java index 1d1e6494..67ff3da2 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ReadPacketModifier.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/ReadPacketModifier.java @@ -19,23 +19,17 @@ package com.comphenix.protocol.injector.packet; import java.io.DataInputStream; import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.Collections; import java.util.Map; -import java.util.WeakHashMap; import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; +import com.google.common.collect.MapMaker; import net.sf.cglib.proxy.MethodInterceptor; import net.sf.cglib.proxy.MethodProxy; class ReadPacketModifier implements MethodInterceptor { - - @SuppressWarnings("rawtypes") - private static Class[] parameters = { DataInputStream.class }; - // A cancel marker private static final Object CANCEL_MARKER = new Object(); @@ -46,20 +40,24 @@ class ReadPacketModifier implements MethodInterceptor { // Report errors private ErrorReporter reporter; - // Whether or not a packet has been cancelled - private static Map override = Collections.synchronizedMap(new WeakHashMap()); + // If this is a read packet data method + private boolean isReadPacketDataMethod; - public ReadPacketModifier(int packetID, ProxyPacketInjector packetInjector, ErrorReporter reporter) { + // Whether or not a packet has been cancelled + private static Map override = new MapMaker().weakKeys().makeMap(); + + public ReadPacketModifier(int packetID, ProxyPacketInjector packetInjector, ErrorReporter reporter, boolean isReadPacketDataMethod) { this.packetID = packetID; this.packetInjector = packetInjector; this.reporter = reporter; + this.isReadPacketDataMethod = isReadPacketDataMethod; } /** * Remove any packet overrides. * @param packet - the packet to rever */ - public void removeOverride(Object packet) { + public static void removeOverride(Object packet) { override.remove(packet); } @@ -68,7 +66,7 @@ class ReadPacketModifier implements MethodInterceptor { * @param packet - the given packet. * @return Overriden object. */ - public Object getOverride(Object packet) { + public static Object getOverride(Object packet) { return override.get(packet); } @@ -77,23 +75,15 @@ class ReadPacketModifier implements MethodInterceptor { * @param packet - the packet to check. * @return TRUE if it has been cancelled, FALSE otherwise. */ - public boolean hasCancelled(Object packet) { + public static boolean hasCancelled(Object packet) { return getOverride(packet) == CANCEL_MARKER; } - + @Override public Object intercept(Object thisObj, Method method, Object[] args, MethodProxy proxy) throws Throwable { - - Object returnValue = null; - String methodName = method.getName(); - - // We always pass these down (otherwise, we'll end up with a infinite loop) - if (methodName.equals("hashCode") || methodName.equals("equals") || methodName.equals("toString")) { - return proxy.invokeSuper(thisObj, args); - } - // Atomic retrieval Object overridenObject = override.get(thisObj); + Object returnValue = null; if (overridenObject != null) { // This packet has been cancelled @@ -111,9 +101,7 @@ class ReadPacketModifier implements MethodInterceptor { } // Is this a readPacketData method? - if (returnValue == null && - Arrays.equals(method.getParameterTypes(), parameters)) { - + if (isReadPacketDataMethod) { try { // We need this in order to get the correct player DataInputStream input = (DataInputStream) args[0]; @@ -134,10 +122,9 @@ class ReadPacketModifier implements MethodInterceptor { } } catch (Throwable e) { // Minecraft cannot handle this error - reporter.reportDetailed(this, "Cannot handle clienet packet.", e, args[0]); + reporter.reportDetailed(this, "Cannot handle client packet.", e, args[0]); } } - return returnValue; }