From be9bbc924e4508371f91927bab805bda29f978ed Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 27 Aug 2013 22:09:06 +0200 Subject: [PATCH] Experimental fix for issue #7 on Github. Attempt to detect the correct "get" method in a IntHashMap. --- .../protocol/injector/EntityUtilities.java | 38 +--- .../injector/packet/ProxyPacketInjector.java | 25 +-- .../reflect/fuzzy/AbstractFuzzyMember.java | 9 + .../reflect/fuzzy/FuzzyFieldContract.java | 6 + .../protocol/reflect/fuzzy/FuzzyMatchers.java | 55 +++++- .../reflect/fuzzy/FuzzyMethodContract.java | 6 + .../protocol/utility/MinecraftReflection.java | 50 ++++- .../protocol/wrappers/WrappedIntHashMap.java | 176 ++++++++++++++++++ .../utility/StreamSerializerTest.java | 7 + 9 files changed, 312 insertions(+), 60 deletions(-) create mode 100644 ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedIntHashMap.java diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java index f20a2c4c..fbf9eac1 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/EntityUtilities.java @@ -21,7 +21,6 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -35,8 +34,8 @@ import org.bukkit.entity.Player; import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; -import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract; import com.comphenix.protocol.utility.MinecraftReflection; +import com.comphenix.protocol.wrappers.WrappedIntHashMap; import com.google.common.collect.Lists; /** @@ -51,7 +50,6 @@ class EntityUtilities { private static Field trackedPlayersField; private static Field trackerField; - private static Method hashGetMethod; private static Method scanPlayersMethod; /* @@ -180,7 +178,7 @@ class EntityUtilities { if (trackedEntitiesField == null) { @SuppressWarnings("rawtypes") - Set ignoredTypes = new HashSet(); + Set ignoredTypes = new HashSet(); // Well, this is more difficult. But we're looking for a Minecraft object that is not // created by the constructor(s). @@ -197,42 +195,14 @@ class EntityUtilities { // Read the entity hashmap Object trackedEntities = null; - + try { trackedEntities = FieldUtils.readField(trackedEntitiesField, tracker, true); } catch (IllegalAccessException e) { throw new FieldAccessException("Cannot access 'trackedEntities' field due to security limitations.", e); } - // Getting the "get" method is pretty hard, but first - try to just get it by name - if (hashGetMethod == null) { - - Class type = trackedEntities.getClass(); - - try { - hashGetMethod = type.getMethod("get", int.class); - } catch (NoSuchMethodException e) { - // Then it's probably the lowest named method that takes an int-parameter and returns a object - hashGetMethod = FuzzyReflection.fromClass(type).getMethod( - FuzzyMethodContract.newBuilder().banModifier(Modifier.STATIC). - parameterCount(1). - parameterExactType(int.class). - returnTypeExact(Object.class). - build() - ); - } - } - - // Wrap exceptions - try { - return hashGetMethod.invoke(trackedEntities, entityID); - } catch (IllegalArgumentException e) { - throw e; - } catch (IllegalAccessException e) { - throw new FieldAccessException("Security limitation prevents access to 'get' method in IntHashMap", e); - } catch (InvocationTargetException e) { - throw new RuntimeException("Exception occurred in Minecraft.", e); - } + return WrappedIntHashMap.fromHandle(trackedEntities).get(entityID); } /** 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 48364818..0d2785ba 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 @@ -20,7 +20,6 @@ package com.comphenix.protocol.injector.packet; import java.io.DataInput; import java.io.DataInputStream; import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Map; import java.util.Set; @@ -48,6 +47,7 @@ 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; +import com.comphenix.protocol.wrappers.WrappedIntHashMap; /** * This class is responsible for adding or removing proxy objects that intercepts recieved packets. @@ -67,9 +67,7 @@ class ProxyPacketInjector implements PacketInjector { } private static class IntHashMapLookup implements PacketClassLookup { - // The "put" method that associates a packet ID with a packet class - private Method putMethod; - private Object intHashMap; + private WrappedIntHashMap intHashMap; public IntHashMapLookup() throws IllegalAccessException { initialize(); @@ -77,32 +75,21 @@ class ProxyPacketInjector implements PacketInjector { @Override public void setLookup(int packetID, Class clazz) { - try { - putMethod.invoke(intHashMap, packetID, clazz); - } catch (IllegalArgumentException e) { - throw new RuntimeException("Illegal argument.", e); - } catch (IllegalAccessException e) { - throw new RuntimeException("Cannot access method.", e); - } catch (InvocationTargetException e) { - throw new RuntimeException("Exception occured in IntHashMap.put.", e); - } + intHashMap.put(packetID, clazz); } private void initialize() throws IllegalAccessException { if (intHashMap == null) { // We're looking for the first static field with a Minecraft-object. This should be a IntHashMap. Field intHashMapField = FuzzyReflection.fromClass(MinecraftReflection.getPacketClass(), true). - getFieldByType(MinecraftReflection.getMinecraftObjectRegex()); + getFieldByType("packetIdMap", MinecraftReflection.getIntHashMapClass()); try { - intHashMap = FieldUtils.readField(intHashMapField, (Object) null, true); + intHashMap = WrappedIntHashMap.fromHandle( + FieldUtils.readField(intHashMapField, (Object) null, true)); } catch (IllegalArgumentException e) { throw new RuntimeException("Minecraft is incompatible.", e); } - - // Now, get the "put" method. - putMethod = FuzzyReflection.fromObject(intHashMap). - getMethodByParameters("put", int.class, Object.class); } } } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/AbstractFuzzyMember.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/AbstractFuzzyMember.java index 40147caf..a50dac2c 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/AbstractFuzzyMember.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/AbstractFuzzyMember.java @@ -1,6 +1,7 @@ package com.comphenix.protocol.reflect.fuzzy; import java.lang.reflect.Member; +import java.lang.reflect.Modifier; import java.util.Map; import java.util.regex.Pattern; @@ -46,6 +47,14 @@ public abstract class AbstractFuzzyMember extends AbstractFuzz return this; } + /** + * Require that every matching member is public. + * @return This builder, for chaining. + */ + public Builder requirePublic() { + return requireModifier(Modifier.PUBLIC); + } + /** * Add a given bit-field of modifers that will skip or ignore members. * @param modifier - bit-field of modifiers to skip or ignore. diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyFieldContract.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyFieldContract.java index 4f4929d2..1b0c7b5a 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyFieldContract.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyFieldContract.java @@ -34,6 +34,12 @@ public class FuzzyFieldContract extends AbstractFuzzyMember { return this; } + @Override + public Builder requirePublic() { + super.requirePublic(); + return this; + } + @Override public Builder nameRegex(String regex) { super.nameRegex(regex); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMatchers.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMatchers.java index 8e0f5db6..b863d949 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMatchers.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMatchers.java @@ -4,6 +4,9 @@ import java.lang.reflect.Member; import java.util.Set; import java.util.regex.Pattern; +import javax.annotation.Nonnull; + +import com.google.common.base.Preconditions; import com.google.common.collect.Sets; /** @@ -12,14 +15,56 @@ import com.google.common.collect.Sets; * @author Kristian */ public class FuzzyMatchers { + // Constant matchers + private static AbstractFuzzyMatcher> MATCH_ALL = new AbstractFuzzyMatcher>() { + @Override + public boolean isMatch(Class value, Object parent) { + return true; + } + + @Override + protected int calculateRoundNumber() { + return 0; + } + }; + private FuzzyMatchers() { // Don't make this constructable } + /** + * Construct a class matcher that matches an array with a given component matcher. + * @param componentMatcher - the component matcher. + * @return A new array matcher. + */ + public static AbstractFuzzyMatcher> matchArray(@Nonnull final AbstractFuzzyMatcher> componentMatcher) { + Preconditions.checkNotNull(componentMatcher, "componentMatcher cannot be NULL."); + return new AbstractFuzzyMatcher>() { + @Override + public boolean isMatch(Class value, Object parent) { + return value.isArray() && componentMatcher.isMatch(value.getComponentType(), parent); + } + + @Override + protected int calculateRoundNumber() { + // We're just above object + return -1; + } + }; + } + + /** + * Retrieve a fuzzy matcher that will match any class. + * @return A class matcher. + */ + public static AbstractFuzzyMatcher> matchAll() { + return MATCH_ALL; + } + /** * Construct a class matcher that matches types exactly. * @param matcher - the matching class. - * @return A new class mathcher. + * @return A new class matcher. */ public static AbstractFuzzyMatcher> matchExact(Class matcher) { return new ClassExactMatcher(matcher, ClassExactMatcher.Options.MATCH_EXACT); @@ -28,7 +73,7 @@ public class FuzzyMatchers { /** * Construct a class matcher that matches any of the given classes exactly. * @param classes - list of classes to match. - * @return A new class mathcher. + * @return A new class matcher. */ public static AbstractFuzzyMatcher> matchAnyOf(Class... classes) { return matchAnyOf(Sets.newHashSet(classes)); @@ -37,7 +82,7 @@ public class FuzzyMatchers { /** * Construct a class matcher that matches any of the given classes exactly. * @param classes - set of classes to match. - * @return A new class mathcher. + * @return A new class matcher. */ public static AbstractFuzzyMatcher> matchAnyOf(Set> classes) { return new ClassSetMatcher(classes); @@ -46,7 +91,7 @@ public class FuzzyMatchers { /** * Construct a class matcher that matches super types of the given class. * @param matcher - the matching type must be a super class of this type. - * @return A new class mathcher. + * @return A new class matcher. */ public static AbstractFuzzyMatcher> matchSuper(Class matcher) { return new ClassExactMatcher(matcher, ClassExactMatcher.Options.MATCH_SUPER); @@ -55,7 +100,7 @@ public class FuzzyMatchers { /** * Construct a class matcher that matches derived types of the given class. * @param matcher - the matching type must be a derived class of this type. - * @return A new class mathcher. + * @return A new class matcher. */ public static AbstractFuzzyMatcher> matchDerived(Class matcher) { return new ClassExactMatcher(matcher, ClassExactMatcher.Options.MATCH_DERIVED); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMethodContract.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMethodContract.java index c761554b..54b50f7a 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMethodContract.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/fuzzy/FuzzyMethodContract.java @@ -98,6 +98,12 @@ public class FuzzyMethodContract extends AbstractFuzzyMember { return this; } + @Override + public Builder requirePublic() { + super.requirePublic(); + return this; + } + @Override public Builder banModifier(int modifier) { super.banModifier(modifier); diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java index f0a1eae5..71f80a1a 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java @@ -419,6 +419,15 @@ public class MinecraftReflection { return getDataWatcherClass().isAssignableFrom(obj.getClass()); } + /** + * Determine if the given object is an IntHashMap object. + * @param obj - the given object. + * @return TRUE if it is, FALSE otherwise. + */ + public static boolean isIntHashMap(Object obj) { + return getIntHashMapClass().isAssignableFrom(obj.getClass()); + } + /** * Determine if the given object is a CraftItemStack instancey. * @param obj - the given object. @@ -1010,6 +1019,45 @@ public class MinecraftReflection { } } + /** + * Retrieve the IntHashMap class. + * @return IntHashMap class. + */ + public static Class getIntHashMapClass() { + try { + return getMinecraftClass("IntHashMap"); + } catch (RuntimeException e) { + final Class parent = getEntityTrackerClass(); + + // Expected structure of a IntHashMap + final FuzzyClassContract intHashContract = FuzzyClassContract.newBuilder(). + // add(int key, Object value) + method(FuzzyMethodContract.newBuilder(). + parameterCount(2). + parameterExactType(int.class, 0). + parameterExactType(Object.class, 1).requirePublic() + ). + // Object get(int key) + method(FuzzyMethodContract.newBuilder(). + parameterCount(1). + parameterExactType(int.class). + returnTypeExact(Object.class).requirePublic() + ). + // Finally, there should be an array of some kind + field(FuzzyFieldContract.newBuilder(). + typeMatches(FuzzyMatchers.matchArray(FuzzyMatchers.matchAll())) + ). + build(); + + final AbstractFuzzyMatcher intHashField = FuzzyFieldContract.newBuilder(). + typeMatches(getMinecraftObjectMatcher().and(intHashContract)). + build(); + + // Use the type of the first field that matches + return setMinecraftClass("IntHashMap", FuzzyReflection.fromClass(parent).getField(intHashField).getType()); + } + } + /** * Retrieve the attribute modifier class. * @return Attribute modifier class. @@ -1295,6 +1343,4 @@ public class MinecraftReflection { public static String getNetLoginHandlerName() { return getNetLoginHandlerClass().getSimpleName(); } - - } diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedIntHashMap.java b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedIntHashMap.java new file mode 100644 index 00000000..5a89a683 --- /dev/null +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedIntHashMap.java @@ -0,0 +1,176 @@ +package com.comphenix.protocol.wrappers; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; + +import javax.annotation.Nonnull; + +import com.comphenix.protocol.reflect.FuzzyReflection; +import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract; +import com.comphenix.protocol.utility.MinecraftReflection; +import com.google.common.base.Preconditions; + +/** + * Represents a wrapper for the internal IntHashMap in Minecraft. + * @author Kristian + */ +public class WrappedIntHashMap { + private static final Class INT_HASH_MAP = MinecraftReflection.getIntHashMapClass(); + + private static Method PUT_METHOD; + private static Method GET_METHOD; + + private Object handle; + + /** + * Construct an IntHashMap wrapper around an instance. + * @param handle - the NMS instance. + */ + private WrappedIntHashMap(Object handle) { + this.handle = handle; + } + + /** + * Construct a new IntHashMap. + * @return A new IntHashMap. + */ + public static WrappedIntHashMap newMap() { + try { + return new WrappedIntHashMap(INT_HASH_MAP.newInstance()); + } catch (Exception e) { + throw new RuntimeException("Unable to construct IntHashMap.", e); + } + } + + /** + * Construct a wrapper around a given NMS IntHashMap. + * @param handle - the NMS IntHashMap. + * @return The created wrapped. + * @throws IllegalArgumentException If the handle is not an IntHasMap. + */ + public static WrappedIntHashMap fromHandle(@Nonnull Object handle) { + Preconditions.checkNotNull(handle, "handle cannot be NULL"); + Preconditions.checkState(MinecraftReflection.isIntHashMap(handle), + "handle is a " + handle.getClass() + ", not an IntHashMap."); + + return new WrappedIntHashMap(handle); + } + + /** + * Associates a specified key with the given value in the integer map. + *

+ * If the key has already been associated with a value, then it will be replaced by the new value. + * @param key - the key to insert. + * @param value - the value to insert. Cannot be NULL. + * @throws RuntimeException If the reflection machinery failed. + */ + public void put(int key, Object value) { + Preconditions.checkNotNull(value, "value cannot be NULL."); + + initializePutMethod(); + putInternal(key, value); + } + + /** + * Invoked when a value must be inserted into the underlying map, regardless of preconditions. + * @param key - the key. + * @param value - the value to insert. + */ + private void putInternal(int key, Object value) { + try { + PUT_METHOD.invoke(handle, key, value); + } catch (IllegalArgumentException e) { + throw new RuntimeException("Illegal argument.", e); + } catch (IllegalAccessException e) { + throw new RuntimeException("Cannot access method.", e); + } catch (InvocationTargetException e) { + throw new RuntimeException("Exception occured in " + PUT_METHOD + " for" + handle, e); + } + } + + /** + * Retrieve the value associated with a specific key, or NULL if not found. + * @param key - the integer key. + * @return The associated value, or NULL. + */ + public Object get(int key) { + initializeGetMethod(); + + try { + return GET_METHOD.invoke(handle, key); + } catch (IllegalArgumentException e) { + throw new RuntimeException("Illegal argument.", e); + } catch (IllegalAccessException e) { + throw new RuntimeException("Cannot access method.", e); + } catch (InvocationTargetException e) { + throw new RuntimeException("Unable to invoke " + GET_METHOD + " on " + handle, e); + } + } + + /** + * Remove a mapping of a key to a value if it is present. + * @param key - the key of the mapping to remove. + * @return TRUE if a key-value pair was removed, FALSE otherwise. + */ + public boolean remove(int key) { + Object prev = get(key); + + if (prev != null) { + putInternal(key, null); + return true; + } + return false; + } + + private void initializePutMethod() { + if (PUT_METHOD == null) { + // Fairly straight forward + PUT_METHOD = FuzzyReflection.fromClass(INT_HASH_MAP).getMethod( + FuzzyMethodContract.newBuilder(). + banModifier(Modifier.STATIC). + parameterCount(2). + parameterExactType(int.class). + parameterExactType(Object.class). + build()); + } + } + + private void initializeGetMethod() { + if (GET_METHOD == null) { + WrappedIntHashMap temp = WrappedIntHashMap.newMap(); + String expected = "hello"; + + // Initialize a value + temp.put(1, expected); + + // Determine which method to trust + for (Method method : FuzzyReflection.fromClass(INT_HASH_MAP). + getMethodListByParameters(Object.class, new Class[] { int.class })) { + + // Skip static methods + if (Modifier.isStatic(method.getModifiers())) + continue; + + try { + // See if we found the method we are looking for + if (expected.equals(method.invoke(temp.getHandle(), 1))) { + GET_METHOD = method; + return; + } + } catch (Exception e) { + // Suppress + } + } + throw new IllegalStateException("Unable to find appropriate GET_METHOD for IntHashMap."); + } + } + + /** + * Retrieve the underlying IntHashMap object. + * @return The underlying object. + */ + public Object getHandle() { + return handle; + } +} diff --git a/ProtocolLib/src/test/java/com/comphenix/protocol/utility/StreamSerializerTest.java b/ProtocolLib/src/test/java/com/comphenix/protocol/utility/StreamSerializerTest.java index f67225e1..557334cf 100644 --- a/ProtocolLib/src/test/java/com/comphenix/protocol/utility/StreamSerializerTest.java +++ b/ProtocolLib/src/test/java/com/comphenix/protocol/utility/StreamSerializerTest.java @@ -8,6 +8,8 @@ import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; +import net.minecraft.server.v1_6_R2.IntHashMap; + import org.bukkit.Material; import org.bukkit.craftbukkit.v1_6_R2.inventory.CraftItemFactory; import org.bukkit.inventory.ItemStack; @@ -28,6 +30,11 @@ public class StreamSerializerTest { BukkitInitialization.initializeItemMeta(); } + @Test + public void testMinecraftReflection() { + assertEquals(IntHashMap.class, MinecraftReflection.getIntHashMapClass()); + } + @Test public void testSerializer() throws IOException { ItemStack before = new ItemStack(Material.GOLD_AXE);