From da694ca5ed8aabd252b9e6214a0e56a81a7215ca Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 13 Sep 2012 11:01:08 +0200 Subject: [PATCH] Added support for default values in packets. --- .../comphenix/protocol/ProtocolLibrary.java | 3 +- .../comphenix/protocol/ProtocolManager.java | 14 ++ .../injector/PacketFilterManager.java | 20 +- .../protocol/injector/PlayerInjector.java | 176 +++++++++++++----- .../protocol/injector/StructureCache.java | 2 +- .../protocol/reflect/StructureModifier.java | 130 +++++++++++-- .../reflect/StructureModifierTest.java | 3 +- 7 files changed, 283 insertions(+), 65 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/ProtocolLibrary.java b/ProtocolLib/src/com/comphenix/protocol/ProtocolLibrary.java index 3b2711c9..68060fbb 100644 --- a/ProtocolLib/src/com/comphenix/protocol/ProtocolLibrary.java +++ b/ProtocolLib/src/com/comphenix/protocol/ProtocolLibrary.java @@ -35,11 +35,12 @@ public class ProtocolLibrary extends JavaPlugin { @Override public void onDisable() { protocolManager.close(); + protocolManager = null; } /** * Retrieves the packet protocol manager. - * @return Packet protocol manager. + * @return Packet protocol manager, or NULL if it has been disabled. */ public static ProtocolManager getProtocolManager() { return protocolManager; diff --git a/ProtocolLib/src/com/comphenix/protocol/ProtocolManager.java b/ProtocolLib/src/com/comphenix/protocol/ProtocolManager.java index 3e37fc2d..c109d32d 100644 --- a/ProtocolLib/src/com/comphenix/protocol/ProtocolManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/ProtocolManager.java @@ -75,6 +75,20 @@ public interface ProtocolManager { * @return New encapsulated Minecraft packet. */ public PacketContainer createPacket(int id); + + /** + * Constructs a new encapsulated Minecraft packet with the given ID. + *

+ * If set to true, the skip default option will prevent the system from assigning + * non-primitive fields in the packet to a new default instance. For instance, certain + * packets - like Packet60Explosion - require a List or Set to be non-null. If the + * skipDefaults option is false, the List or Set will be automatically created. + * + * @param id - packet ID. + * @param skipDefaults - TRUE to skip setting default values, FALSE otherwise. + * @return New encapsulated Minecraft packet. + */ + public PacketContainer createPacket(int id, boolean skipDefaults); /** * Retieves a set of every enabled packet. diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index c96e5cea..804d0391 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -217,7 +217,23 @@ public final class PacketFilterManager implements ProtocolManager { @Override public PacketContainer createPacket(int id) { - return new PacketContainer(id); + return createPacket(id, false); + } + + @Override + public PacketContainer createPacket(int id, boolean skipDefaults) { + PacketContainer packet = new PacketContainer(id); + + // Use any default values if possible + if (!skipDefaults) { + try { + packet.getModifier().writeDefaults(); + } catch (IllegalAccessException e) { + throw new RuntimeException("Security exception.", e); + } + } + + return packet; } @Override @@ -324,6 +340,8 @@ public final class PacketFilterManager implements ProtocolManager { if (packetInjector != null) packetInjector.cleanupAll(); + // Remove listeners + packetListeners.clear(); playerInjection.clear(); connectionLookup.clear(); hasClosed = true; diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java index dcbefb72..49def95c 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java @@ -2,14 +2,19 @@ package com.comphenix.protocol.injector; import java.io.DataInputStream; import java.lang.reflect.Field; -import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Proxy; +import java.util.Collections; +import java.util.List; +import java.util.ArrayList; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import net.minecraft.server.EntityPlayer; import net.minecraft.server.Packet; +import net.sf.cglib.proxy.Enhancer; +import net.sf.cglib.proxy.MethodInterceptor; +import net.sf.cglib.proxy.MethodProxy; import org.bukkit.craftbukkit.entity.CraftPlayer; import org.bukkit.entity.Player; @@ -18,9 +23,19 @@ import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; +import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.reflect.VolatileField; +import com.google.common.collect.Sets; class PlayerInjector { + + /** + * Marker interface that indicates a packet is fake and should not be processed. + * @author Kristian + */ + public interface FakePacket { + // Nothing + } // Cache previously retrieved fields private static Field serverHandlerField; @@ -28,15 +43,27 @@ class PlayerInjector { private static Field inputField; private static Field netHandlerField; + // To add our injected array lists + private static StructureModifier networkModifier; + // And methods private static Method queueMethod; private static Method processMethod; private Player player; private boolean hasInitialized; - + // Reference to the player's network manager - private VolatileField networkManager; + private Object networkManager; + + // Current net handler + private Object netHandler; + + // Overridden fields + private List overridenLists = new ArrayList(); + + // Packets to ignore + private Set ignoredPackets = Sets.newSetFromMap(new ConcurrentHashMap()); // The packet manager and filters private PacketFilterManager manager; @@ -44,9 +71,6 @@ class PlayerInjector { // Previous data input private DataInputStream cachedInput; - - // Current net handler - private Object netHandler; public PlayerInjector(Player player, PacketFilterManager manager, Set packetFilters) throws IllegalAccessException { this.player = player; @@ -70,9 +94,11 @@ class PlayerInjector { Object serverHandler = FieldUtils.readField(serverHandlerField, notchEntity); // Next, get the network manager - if (networkManagerField == null) + if (networkManagerField == null) { networkManagerField = FuzzyReflection.fromObject(serverHandler).getFieldByType(".*NetworkManager"); - networkManager = new VolatileField(networkManagerField, serverHandler); + networkModifier = new StructureModifier(networkManagerField.getType(), null); + } + networkManager = FieldUtils.readField(networkManagerField, serverHandler); // And the queue method if (queueMethod == null) @@ -81,7 +107,7 @@ class PlayerInjector { // And the data input stream that we'll use to identify a player if (inputField == null) - inputField = FuzzyReflection.fromObject(networkManager.getOldValue(), true). + inputField = FuzzyReflection.fromObject(networkManager, true). getFieldByType("java\\.io\\.DataInputStream"); } } @@ -111,7 +137,7 @@ class PlayerInjector { // Get the handler if (netHandler != null) - netHandler = FieldUtils.readField(netHandlerField, networkManager.getOldValue(), true); + netHandler = FieldUtils.readField(netHandlerField, networkManager, true); return netHandler; } @@ -152,12 +178,15 @@ class PlayerInjector { * @param InvocationTargetException If an error occured when sending the packet. */ public void sendServerPacket(Packet packet, boolean filtered) throws InvocationTargetException { - Object networkDelegate = filtered ? networkManager.getValue() : networkManager.getOldValue(); - if (networkDelegate != null) { + if (networkManager != null) { try { + if (!filtered) { + ignoredPackets.add(packet); + } + // Note that invocation target exception is a wrapper for a checked exception - queueMethod.invoke(networkDelegate, packet); + queueMethod.invoke(networkManager, packet); } catch (IllegalArgumentException e) { throw e; @@ -171,47 +200,101 @@ class PlayerInjector { } } + @SuppressWarnings("serial") public void injectManager() { if (networkManager != null) { - final Class networkInterface = networkManagerField.getType(); - final Object networkDelegate = networkManager.getOldValue(); + + @SuppressWarnings("rawtypes") + StructureModifier list = networkModifier.withType(List.class); - // Create our proxy object - Object networkProxy = Proxy.newProxyInstance(networkInterface.getClassLoader(), - new Class[] { networkInterface }, new InvocationHandler() { + // Subclass both send queues + for (Field field : list.getFields()) { + VolatileField overwriter = new VolatileField(field, networkManager, true); - @Override - public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { - // OH OH! The queue method! - if (method.equals(queueMethod)) { - Packet packet = (Packet) args[0]; - - if (packet != null) { + overwriter.setValue(Collections.synchronizedList(new ArrayList() { + @Override + public boolean add(Packet packet) { + + // Check for fake packets and ignored packets + if (packet instanceof FakePacket) { + return true; + } else if (ignoredPackets.contains(packet)) { + ignoredPackets.remove(packet); + } else { packet = handlePacketRecieved(packet); + } + + // A NULL packet indicate cancelling + try { + if (packet != null) { + super.add(packet); + } else { + // We'll use the FakePacket marker instead of preventing the filters + sendServerPacket(createNegativePacket(packet), true); + } - // A NULL packet indicate cancelling - if (packet != null) - args[0] = packet; - else - return null; + // Collection.add contract + return true; + + } catch (InvocationTargetException e) { + throw new RuntimeException("Reverting cancelled packet failed.", e.getTargetException()); } } - - // Delegate to our underlying class - try { - return method.invoke(networkDelegate, args); - } catch (InvocationTargetException e) { - throw e.getCause(); - } - } - }); - - // Inject it, if we can. - networkManager.setValue(networkProxy); + })); + overridenLists.add(overwriter); + } } } + /** + * Used by a hack that reverses the effect of a cancelled packet. Returns a packet + * whereby every int method's return value is inverted (a => -a). + * + * @param source - packet to invert. + * @return The inverted packet. + */ + private Packet createNegativePacket(Packet source) { + Enhancer ex = new Enhancer(); + Class type = source.getClass(); + + // We want to subtract the byte amount that were added to the running + // total of outstanding packets. Otherwise, cancelling too many packets + // might cause a "disconnect.overflow" error. + // + // We do that by constructing a special packet of the same type that returns + // a negative integer for all zero-parameter integer methods. This includes the + // size() method, which is used by the queue method to count the number of + // bytes to add. + // + // Essentially, we have: + // + // public class NegativePacket extends [a packet] { + // @Override + // public int size() { + // return -super.size(); + // } + // ect. + // } + ex.setInterfaces(new Class[] { FakePacket.class } ); + ex.setUseCache(true); + ex.setClassLoader(type.getClassLoader()); + ex.setSuperclass(type); + ex.setCallback(new MethodInterceptor() { + @Override + public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { + if (method.getReturnType().equals(int.class) && args.length == 0) { + Integer result = (Integer) proxy.invokeSuper(obj, args); + return -result; + } else { + return proxy.invokeSuper(obj, args); + } + } + }); + + return (Packet) ex.create(); + } + /** * Allows a packet to be recieved by the listeners. * @param packet - packet to recieve. @@ -246,7 +329,7 @@ class PlayerInjector { return cachedInput; // Save to cache - cachedInput = (DataInputStream) FieldUtils.readField(inputField, networkManager.getOldValue(), true); + cachedInput = (DataInputStream) FieldUtils.readField(inputField, networkManager, true); return cachedInput; } catch (IllegalAccessException e) { @@ -256,6 +339,9 @@ class PlayerInjector { public void cleanupAll() { // Clean up - networkManager.revertValue(); + for (VolatileField overriden : overridenLists) { + overriden.revertValue(); + } + overridenLists.clear(); } } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java b/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java index 9aab6ab2..e19e2555 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/StructureCache.java @@ -37,7 +37,7 @@ public class StructureCache { // Use the vanilla class definition if (result == null) { - result = new StructureModifier(MinecraftRegistry.getPacketClassFromID(id, true)); + result = new StructureModifier(MinecraftRegistry.getPacketClassFromID(id, true), Packet.class); structureModifiers.put(id, result); } diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/StructureModifier.java b/ProtocolLib/src/com/comphenix/protocol/reflect/StructureModifier.java index 62faff1a..6dc22ec8 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/StructureModifier.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/StructureModifier.java @@ -4,10 +4,14 @@ import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; +import com.google.gson.internal.Primitives; import net.minecraft.server.Packet; @@ -25,27 +29,40 @@ public class StructureModifier { private Class fieldType; private List data = new ArrayList(); + // Improved default values + private Set defaultFields; + // Cache of previous types private Map subtypeCache; - public StructureModifier(Class targetType) { - this(targetType, Object.class, getFields(targetType), null, new HashMap()); - } - - private StructureModifier(Class targetType, Class fieldType, List data, - EquivalentConverter converter, Map subTypeCache) { - this.targetType = targetType; - this.fieldType = fieldType; - this.data = data; - this.converter = converter; - this.subtypeCache = subTypeCache; + public StructureModifier(Class targetType, Class superclassExclude) { + List fields = getFields(targetType, superclassExclude); + + initialize(targetType, Object.class, + fields, generateDefaultFields(fields), null, + new HashMap()); } private StructureModifier(StructureModifier other, Object target) { - this(other.targetType, other.fieldType, other.data, other.converter, other.subtypeCache); + initialize(other.targetType, other.fieldType, other.data, other.defaultFields, other.converter, other.subtypeCache); this.target = target; } + private StructureModifier() { + // Consumers of this method should call "initialize" + } + + private void initialize(Class targetType, Class fieldType, + List data, Set defaultFields, + EquivalentConverter converter, Map subTypeCache) { + this.targetType = targetType; + this.fieldType = fieldType; + this.data = data; + this.defaultFields = defaultFields; + this.converter = converter; + this.subtypeCache = subTypeCache; + } + /** * Reads the value of a field given its index. * @param fieldIndex - index of the field. @@ -67,6 +84,20 @@ public class StructureModifier { else return (TField) result; } + + /** + * Reads the value of a field if and ONLY IF it exists. + * @param fieldIndex - index of the field. + * @return Value of the field, or NULL if it doesn't exist. + * @throws IllegalAccessException If we're unable to read the field due to a security limitation. + */ + public TField readSafely(int fieldIndex) throws IllegalAccessException { + if (fieldIndex >= 0 && fieldIndex < data.size()) { + return read(fieldIndex); + } else { + return null; + } + } /** * Writes the value of a field given its index. @@ -89,6 +120,20 @@ public class StructureModifier { return this; } + /** + * Writes the value of a given field IF and ONLY if it exists. + * @param fieldIndex - index of the potential field. + * @param value - new value of the field. + * @return This structure modifer - for chaining. + * @throws IllegalAccessException If we're unable to write to the field due to a security limitation. + */ + public StructureModifier writeSafely(int fieldIndex, TField value) throws IllegalAccessException { + if (fieldIndex >= 0 && fieldIndex < data.size()) { + write(fieldIndex, value); + } + return this; + } + /** * Correctly modifies the value of a field. * @param fieldIndex - index of the field to modify. @@ -110,6 +155,22 @@ public class StructureModifier { return withType(fieldType, null); } + /** + * Sets all non-primitive fields to a more fitting default value. See {@link DefaultInstance}. + * @return The current structure modifier - for chaining. + * @throws IllegalAccessException If we're unable to write to the fields due to a security limitation. + */ + public StructureModifier writeDefaults() throws IllegalAccessException { + + // Write a default instance to every field + for (Field field : defaultFields) { + FieldUtils.writeField(field, target, + DefaultInstances.getDefault(field.getType()), true); + } + + return this; + } + /** * Retrieves a structure modifier that only reads and writes fields of a given type. * @param fieldType - the type, or supertype, of every field to modify. @@ -128,16 +189,21 @@ public class StructureModifier { } else if (result == null) { List filtered = new ArrayList(); + Set defaults = new HashSet(); for (Field field : data) { if (fieldType.isAssignableFrom(field.getType())) { filtered.add(field); + + if (defaultFields.contains(field)) + defaults.add(field); } } // Cache structure modifiers - result = new StructureModifier(targetType, fieldType, filtered, - converter, new HashMap()); + result = new StructureModifier(); + result.initialize(targetType, fieldType, filtered, defaults, + converter, new HashMap()); subtypeCache.put(fieldType, result); } @@ -199,8 +265,37 @@ public class StructureModifier { return copy; } + /** + * Retrieves a list of the fields matching the constraints of this structure modifier. + * @return List of fields. + */ + public List getFields() { + return ImmutableList.copyOf(data); + } + + // Used to generate plausible default values + private static Set generateDefaultFields(List fields) { + + Set requireDefaults = new HashSet(); + + for (Field field : fields) { + Class type = field.getType(); + + // First, ignore primitive fields + if (!Primitives.isPrimitive(type)) { + // Next, see if we actually can generate a default value + if (DefaultInstances.getDefault(type) != null) { + // If so, require it + requireDefaults.add(field); + } + } + } + + return requireDefaults; + } + // Used to filter out irrelevant fields - private static List getFields(Class type) { + private static List getFields(Class type, Class superclassExclude) { List result = new ArrayList(); // Retrieve every private and public field @@ -208,7 +303,10 @@ public class StructureModifier { int mod = field.getModifiers(); // Ignore static, final and "abstract packet" fields - if (!Modifier.isFinal(mod) && !Modifier.isStatic(mod) && !field.getDeclaringClass().equals(Packet.class)) { + if (!Modifier.isFinal(mod) && !Modifier.isStatic(mod) && ( + superclassExclude == null || !field.getDeclaringClass().equals(Packet.class) + )) { + result.add(field); } } diff --git a/ProtocolLib/test/com/comphenix/protocol/reflect/StructureModifierTest.java b/ProtocolLib/test/com/comphenix/protocol/reflect/StructureModifierTest.java index d3168d87..3d1c8de0 100644 --- a/ProtocolLib/test/com/comphenix/protocol/reflect/StructureModifierTest.java +++ b/ProtocolLib/test/com/comphenix/protocol/reflect/StructureModifierTest.java @@ -6,6 +6,7 @@ import net.minecraft.server.Packet103SetSlot; import org.junit.Test; +import com.avaje.ebeaninternal.server.cluster.Packet; import com.comphenix.protocol.reflect.StructureModifier; public class StructureModifierTest { @@ -14,7 +15,7 @@ public class StructureModifierTest { public void test() throws IllegalAccessException { Packet103SetSlot move = new Packet103SetSlot(); - StructureModifier modifier = new StructureModifier(Packet103SetSlot.class); + StructureModifier modifier = new StructureModifier(Packet103SetSlot.class, Packet.class); move.a = 1; int value = (Integer) modifier.withTarget(move).withType(int.class).read(0);