From e25ab31316e69d234988bfc77c9db40cfee8d2e1 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Wed, 17 Jul 2013 05:48:34 +0200 Subject: [PATCH] Generate a new class for each packet type, as getID is final. We can't override the getID method in Packet, as it is marked as final. Instead, we'll just generate a seperate class for each packet ID that needs to be intercepted. We can't inherit from the packet ID of any particular class, as it may get recognized and modified by the sendPacket() method. This could be a problem if this recogition is necessary, but we'll come back to this later. --- .../protocol/injector/ListenerInvoker.java | 7 + .../injector/PacketFilterManager.java | 12 ++ .../injector/packet/InterceptWritePacket.java | 100 +++++++++--- .../injector/packet/WritePacketModifier.java | 143 ++++++++++-------- .../injector/player/PlayerInjector.java | 8 +- 5 files changed, 179 insertions(+), 91 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/ListenerInvoker.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/ListenerInvoker.java index e0da7b83..1fd888d5 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/ListenerInvoker.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/ListenerInvoker.java @@ -18,6 +18,7 @@ package com.comphenix.protocol.injector; import com.comphenix.protocol.events.PacketEvent; +import com.comphenix.protocol.injector.packet.InterceptWritePacket; /** * Represents an object that initiate the packet listeners. @@ -45,6 +46,12 @@ public interface ListenerInvoker { */ public abstract int getPacketID(Object packet); + /** + * Retrieve the object responsible for intercepting write packets. + * @return Object that intercepts write packets. + */ + public InterceptWritePacket getInterceptWritePacket(); + /** * Associate a given class with the given packet ID. Internal method. * @param clazz - class to associate. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java index 0d9b1a1e..aca665f0 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java @@ -53,6 +53,7 @@ import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.error.Report; import com.comphenix.protocol.error.ReportType; import com.comphenix.protocol.events.*; +import com.comphenix.protocol.injector.packet.InterceptWritePacket; import com.comphenix.protocol.injector.packet.PacketInjector; import com.comphenix.protocol.injector.packet.PacketInjectorBuilder; import com.comphenix.protocol.injector.packet.PacketRegistry; @@ -134,6 +135,9 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok // Different injection types per game phase private PlayerInjectionHandler playerInjection; + // Intercepting write packet methods + private InterceptWritePacket interceptWritePacket; + // The two listener containers private SortedPacketListenerList recievedListeners; private SortedPacketListenerList sendingListeners; @@ -202,6 +206,9 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok // The plugin verifier this.pluginVerifier = new PluginVerifier(builder.getLibrary()); + // The write packet interceptor + interceptWritePacket = new InterceptWritePacket(classLoader, reporter); + // Use the correct injection type if (builder.isNettyEnabled()) { spigotInjector = new SpigotPacketInjector(classLoader, reporter, this, server); @@ -274,6 +281,11 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok return ImmutableSet.copyOf(packetListeners); } + @Override + public InterceptWritePacket getInterceptWritePacket() { + return interceptWritePacket; + } + /** * Warn of common programming mistakes. * @param plugin - plugin to check. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/InterceptWritePacket.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/InterceptWritePacket.java index 4231b56d..55448597 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/InterceptWritePacket.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/InterceptWritePacket.java @@ -2,11 +2,11 @@ package com.comphenix.protocol.injector.packet; import java.io.DataOutput; import java.lang.reflect.Method; +import java.util.concurrent.ConcurrentMap; import net.sf.cglib.proxy.Callback; import net.sf.cglib.proxy.CallbackFilter; import net.sf.cglib.proxy.Enhancer; -import net.sf.cglib.proxy.NoOp; import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.error.Report; @@ -16,6 +16,7 @@ import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.reflect.MethodInfo; import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract; import com.comphenix.protocol.utility.MinecraftReflection; +import com.google.common.collect.Maps; /** * Retrieve a packet instance that has its write method intercepted. @@ -34,35 +35,41 @@ public class InterceptWritePacket { parameterCount(1). build(); - private ClassLoader classLoader; - private ErrorReporter reporter; - private CallbackFilter filter; private boolean writePacketIntercepted; + private ConcurrentMap> proxyClasses = Maps.newConcurrentMap(); + + private ClassLoader classLoader; + private ErrorReporter reporter; + + private WritePacketModifier modifierWrite; + private WritePacketModifier modifierRest; + public InterceptWritePacket(ClassLoader classLoader, ErrorReporter reporter) { this.classLoader = classLoader; - this.reporter = reporter; + this.reporter = reporter; + + // Initialize modifiers + this.modifierWrite = new WritePacketModifier(reporter, true); + this.modifierRest = new WritePacketModifier(reporter, false); } - /** - * Construct a new instance of the proxy object. - * @return New instance of proxy. - */ - public Object constructProxy(Object proxyObject, PacketEvent event, NetworkMarker marker) { + private Class createProxyClass(int packetId) { // Construct the proxy object Enhancer ex = new Enhancer(); - // Initialize the shared filter + // Attempt to share callback filter if (filter == null) { filter = new CallbackFilter() { @Override public int accept(Method method) { // Skip methods defined in Object if (WRITE_PACKET.isMatch(MethodInfo.fromMethod(method), null)) { - return 1; - } else { + writePacketIntercepted = true; return 0; + } else { + return 1; } } }; @@ -71,15 +78,16 @@ public class InterceptWritePacket { // Subclass the generic packet class ex.setSuperclass(MinecraftReflection.getPacketClass()); ex.setCallbackFilter(filter); + ex.setUseCache(false); + ex.setClassLoader(classLoader); - ex.setCallbacks(new Callback[] { - NoOp.INSTANCE, - new WritePacketModifier(reporter, proxyObject, event, marker) - }); + ex.setCallbackTypes( new Class[] { WritePacketModifier.class, WritePacketModifier.class }); + Class proxyClass = ex.createClass(); - Object proxy = ex.create(); - - if (proxy != null) { + // Register write modifiers too + Enhancer.registerStaticCallbacks(proxyClass, new Callback[] { modifierWrite, modifierRest }); + + if (proxyClass != null) { // Check that we found the read method if (!writePacketIntercepted) { reporter.reportWarning(this, @@ -87,6 +95,56 @@ public class InterceptWritePacket { messageParam(MinecraftReflection.getPacketClass())); } } - return proxy; + return proxyClass; + } + + private Class getProxyClass(int packetId) { + Class stored = proxyClasses.get(packetId); + + // Concurrent pattern + if (stored == null) { + final Class created = createProxyClass(packetId); + stored = proxyClasses.putIfAbsent(packetId, created); + + // We won! + if (stored == null) { + stored = created; + PacketRegistry.getPacketToID().put(stored, packetId); + } + } + return stored; + } + + /** + * Construct a new instance of the proxy object. + * @return New instance of proxy, or NULL if we failed. + */ + public Object constructProxy(Object proxyObject, PacketEvent event, NetworkMarker marker) { + Class proxyClass = null; + + try { + proxyClass = getProxyClass(event.getPacketID()); + Object generated = proxyClass.newInstance(); + + modifierWrite.register(generated, proxyObject, event, marker); + modifierRest.register(generated, proxyObject, event, marker); + return generated; + + } catch (Exception e) { + reporter.reportWarning(this, + Report.newBuilder(REPORT_CANNOT_CONSTRUCT_WRITE_PROXY). + messageParam(proxyClass)); + return null; + } + } + + /** + * Invoked when the write packet proxy class should be removed. + */ + public void cleanup() { + // Remove all proxy classes from the registry + for (Class stored : proxyClasses.values()) { + PacketRegistry.getPacketToID().remove(stored); + } } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/WritePacketModifier.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/WritePacketModifier.java index a4be84d0..9712ef52 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/WritePacketModifier.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/packet/WritePacketModifier.java @@ -21,6 +21,7 @@ import java.io.ByteArrayOutputStream; import java.io.DataOutput; import java.io.DataOutputStream; import java.lang.reflect.Method; +import java.util.Map; import java.util.PriorityQueue; import com.comphenix.protocol.error.ErrorReporter; @@ -29,6 +30,7 @@ import com.comphenix.protocol.error.ReportType; import com.comphenix.protocol.events.NetworkMarker; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.events.PacketOutputHandler; +import com.google.common.collect.MapMaker; import net.sf.cglib.proxy.MethodInterceptor; import net.sf.cglib.proxy.MethodProxy; @@ -36,89 +38,98 @@ import net.sf.cglib.proxy.MethodProxy; public class WritePacketModifier implements MethodInterceptor { public static final ReportType REPORT_CANNOT_WRITE_SERVER_PACKET = new ReportType("Cannot write server packet."); + private static class ProxyInformation { + // Marker that contains custom writers + public final Object proxyObject; + public final PacketEvent event; + public final NetworkMarker marker; + + public ProxyInformation(Object proxyObject, PacketEvent event, NetworkMarker marker) { + this.proxyObject = proxyObject; + this.event = event; + this.marker = marker; + } + } + + private Map proxyLookup = new MapMaker().weakKeys().makeMap(); + // Report errors private final ErrorReporter reporter; - // Marker that contains custom writers - private final Object proxyObject; - private final PacketEvent event; - private final NetworkMarker marker; + // Whether or not this represents the write method + private boolean isWriteMethod; - public WritePacketModifier(ErrorReporter reporter, Object proxyObject, PacketEvent event, NetworkMarker marker) { - this.proxyObject = proxyObject; - this.event = event; - this.marker = marker; + public WritePacketModifier(ErrorReporter reporter, boolean isWriteMethod) { this.reporter = reporter; + this.isWriteMethod = isWriteMethod; } + /** + * Associate the given generated instance of a class and the given parameteters. + * @param generatedClass - the generated class. + * @param proxyObject - the object to call from the generated class. + * @param event - the packet event. + * @param marker - the network marker. + */ + public void register(Object generatedClass, Object proxyObject, PacketEvent event, NetworkMarker marker) { + proxyLookup.put(generatedClass, new ProxyInformation(proxyObject, event, marker)); + } + @Override public Object intercept(Object thisObj, Method method, Object[] args, MethodProxy proxy) throws Throwable { - PriorityQueue handlers = (PriorityQueue) marker.getOutputHandlers(); + ProxyInformation information = proxyLookup.get(thisObj); + + if (information == null) { + // This is really bad - someone forgot to register the proxy + throw new RuntimeException("Cannot find proxy information for " + thisObj); + } + + if (isWriteMethod) { + PriorityQueue handlers = (PriorityQueue) + information.marker.getOutputHandlers(); + + // If every output handler has been removed - ignore everything + if (!handlers.isEmpty()) { + try { + DataOutput output = (DataOutput) args[0]; - // If every output handler has been removed - ignore everything - if (!handlers.isEmpty()) { - try { - DataOutput output = (DataOutput) args[0]; - - // First - we need the initial buffer - ByteArrayOutputStream outputBufferStream = new ByteArrayOutputStream(); - proxy.invokeSuper(proxyObject, new Object[] { new DataOutputStream(outputBufferStream) }); - byte[] outputBuffer = outputBufferStream.toByteArray(); - - // Let each handler prepare the actual output - while (!handlers.isEmpty()) { - PacketOutputHandler handler = handlers.poll(); + // First - we need the initial buffer + ByteArrayOutputStream outputBufferStream = new ByteArrayOutputStream(); + proxy.invoke(information.proxyObject, new Object[] { new DataOutputStream(outputBufferStream) }); + byte[] outputBuffer = outputBufferStream.toByteArray(); - try { - byte[] changed = handler.handle(event, outputBuffer); + // Let each handler prepare the actual output + while (!handlers.isEmpty()) { + PacketOutputHandler handler = handlers.poll(); - // Don't break just because a plugin returned NULL - if (changed != null) { - outputBuffer = changed; - } else { - throw new IllegalStateException("Handler cannot return a NULL array."); + try { + byte[] changed = handler.handle(information.event, outputBuffer); + + // Don't break just because a plugin returned NULL + if (changed != null) { + outputBuffer = changed; + } else { + throw new IllegalStateException("Handler cannot return a NULL array."); + } + } catch (Exception e) { + reporter.reportMinimal(handler.getPlugin(), "PacketOutputHandler.handle()", e); } - } catch (Exception e) { - reporter.reportMinimal(handler.getPlugin(), "PacketOutputHandler.handle()", e); } - } - // Write that output to the network stream - output.write(outputBuffer); - - } catch (Throwable e) { - // Minecraft cannot handle this error - reporter.reportDetailed(this, - Report.newBuilder(REPORT_CANNOT_WRITE_SERVER_PACKET).callerParam(args[0]).error(e) - ); + // Write that output to the network stream + output.write(outputBuffer); + return null; + + } catch (Throwable e) { + // Minecraft cannot handle this error + reporter.reportDetailed(this, + Report.newBuilder(REPORT_CANNOT_WRITE_SERVER_PACKET).callerParam(args[0]).error(e) + ); + } } } - + // Default to the super method - return proxy.invokeSuper(proxyObject, args); - } - - /** - * Retrieve the proxied Minecraft object. - * @return The proxied object. - */ - public Object getProxyObject() { - return proxyObject; - } - - /** - * Retrieve the associated packet event. - * @return The packet event. - */ - public PacketEvent getEvent() { - return event; - } - - /** - * Retrieve the network marker that is in use. - * @return The network marker. - */ - public NetworkMarker getMarker() { - return marker; + return proxy.invoke(information.proxyObject, args); } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java index 72e5b2e1..25df28f4 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java @@ -143,9 +143,7 @@ public abstract class PlayerInjector implements SocketInjector { this.reporter = reporter; this.player = player; this.invoker = invoker; - - // Intercept the write method - writePacketInterceptor = new InterceptWritePacket(classLoader, reporter); + this.writePacketInterceptor = invoker.getInterceptWritePacket(); } /** @@ -523,8 +521,10 @@ public abstract class PlayerInjector implements SocketInjector { * Remove all hooks and modifications. */ public final void cleanupAll() { - if (!clean) + if (!clean) { cleanHook(); + writePacketInterceptor.cleanup(); + } clean = true; }