Fix a serious bug that caused Blocks to inadvertanly be created.

Our custom IOC system (DefaultInstances) would call new Block(0) on a
MCPC based CraftBukkit system, causing air blocks to be treated as 
solid blocks. We prevented this by explicitly forbidding it from 
creating instances of ItemStack or Block in our structure modifier's
default field generator.

A second bug was caused by mismatched methods in WrappedDataWatcher,
which we solved by simply trying to use them, and then swapping them 
if we encounter any errors or weird state.

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/FuzzyMatchers.java

@@ -1,8 +1,11 @@
 package com.comphenix.protocol.reflect;
 
 import java.lang.reflect.Member;
+import java.util.Set;
 import java.util.regex.Pattern;
 
+import com.google.common.collect.Sets;
+
 /**
  * Contains factory methods for matching classes.
  * 
@@ -16,11 +19,46 @@ public class FuzzyMatchers {
 	/**
 	 * Construct a class matcher that matches types exactly.
 	 * @param matcher - the matching class.
+	 * @return A new class mathcher.
 	 */
 	public static AbstractFuzzyMatcher<Class<?>> matchExact(Class<?> matcher) {
 		return new ExactClassMatcher(matcher, ExactClassMatcher.Options.MATCH_EXACT);
 	}
 	
+	/**
+	 * Construct a class matcher that matches any of the given classes exactly.
+	 * @param classes - list of classes to match.
+	 * @return A new class mathcher.
+	 */
+	public static AbstractFuzzyMatcher<Class<?>> matchAnyOf(Class<?>... classes) {
+		return matchAnyOf(Sets.newHashSet(classes));
+	}
+	
+	/**
+	 * Construct a class matcher that matches any of the given classes exactly.
+	 * @param classes - set of classes to match.
+	 * @return A new class mathcher.
+	 */
+	public static AbstractFuzzyMatcher<Class<?>> matchAnyOf(final Set<Class<?>> classes) {
+		return new AbstractFuzzyMatcher<Class<?>>() {
+			@Override
+			public boolean isMatch(Class<?> value, Object parent) {
+				return classes.contains(value);
+			}
+			
+			@Override
+			protected int calculateRoundNumber() {
+				int roundNumber = 0;
+				
+				// The highest round number (except zero).
+				for (Class<?> clazz : classes) {
+					roundNumber = combineRounds(roundNumber, -ExactClassMatcher.getClassNumber(clazz));
+				}
+				return roundNumber;
+			}
+		};
+	}
+
 	/**
 	 * 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.

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/StructureModifier.java

@@ -26,10 +26,14 @@ import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 import com.comphenix.protocol.reflect.compiler.BackgroundCompiler;
+import com.comphenix.protocol.reflect.instances.BannedGenerator;
 import com.comphenix.protocol.reflect.instances.DefaultInstances;
+import com.comphenix.protocol.reflect.instances.InstanceProvider;
+import com.comphenix.protocol.utility.MinecraftReflection;
 import com.google.common.base.Function;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 
 /**
  * Provides list-oriented access to the fields of a Minecraft packet.
@@ -65,6 +69,18 @@ public class StructureModifier<TField> {
 	// Whether or not to automatically compile the structure modifier
 	protected boolean useStructureCompiler;
 	
+	// Instance generator we wil use
+	private static DefaultInstances DEFAULT_GENERATOR = getDefaultGenerator();
+
+	private static DefaultInstances getDefaultGenerator() {
+		List<InstanceProvider> providers = Lists.newArrayList();
+		
+		// Prevent certain classes from being generated
+		providers.add(new BannedGenerator(MinecraftReflection.getItemStackClass(), MinecraftReflection.getBlockClass()));
+		providers.addAll(DefaultInstances.DEFAULT.getRegistered());
+		return DefaultInstances.fromCollection(providers);
+	}
+			
 	/**
 	 * Creates a structure modifier.
 	 * @param targetType - the structure to modify.
@@ -528,7 +544,7 @@ public class StructureModifier<TField> {
 	private static Map<Field, Integer> generateDefaultFields(List<Field> fields) {
 		
 		Map<Field, Integer> requireDefaults = new HashMap<Field, Integer>();
-		DefaultInstances generator = DefaultInstances.DEFAULT;
+		DefaultInstances generator = DEFAULT_GENERATOR;
 		int index = 0;
 		
 		for (Field field : fields) {

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/cloning/FieldCloner.java

@@ -20,6 +20,7 @@ package com.comphenix.protocol.reflect.cloning;
 import com.comphenix.protocol.reflect.ObjectWriter;
 import com.comphenix.protocol.reflect.StructureModifier;
 import com.comphenix.protocol.reflect.instances.InstanceProvider;
+import com.comphenix.protocol.reflect.instances.NotConstructableException;
 
 /**
  * Represents a class capable of cloning objects by deeply copying its fields.
@@ -72,7 +73,11 @@ public class FieldCloner implements Cloner {
 			return false;
 		
 		// Attempt to create the type
+		try {
 			return instanceProvider.create(source.getClass()) != null;
+		} catch (NotConstructableException e) {
+			return false;
+		}
 	}
 
 	@Override

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/BannedGenerator.java

@@ -0,0 +1,36 @@
+package com.comphenix.protocol.reflect.instances;
+
+import javax.annotation.Nullable;
+
+import com.comphenix.protocol.reflect.AbstractFuzzyMatcher;
+import com.comphenix.protocol.reflect.FuzzyMatchers;
+
+/**
+ * Generator that ensures certain types will never be created.
+ * 
+ * @author Kristian
+ */
+public class BannedGenerator implements InstanceProvider {
+	private AbstractFuzzyMatcher<Class<?>> classMatcher;
+	
+	/**
+	 * Construct a generator that ensures any class that matches the given matcher is never constructed.
+	 * @param classMatcher - a class matcher.
+	 */
+	public BannedGenerator(AbstractFuzzyMatcher<Class<?>> classMatcher) {
+		this.classMatcher = classMatcher;
+	}
+	
+	public BannedGenerator(Class<?>... classes) {
+		this.classMatcher = FuzzyMatchers.matchAnyOf(classes);
+	}
+
+	@Override
+	public Object create(@Nullable Class<?> type) {
+		// Prevent these types from being constructed
+		if (classMatcher.isMatch(type, null)) {
+			throw new NotConstructableException();
+		}
+		return null;
+	}
+}

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/DefaultInstances.java

@@ -86,8 +86,17 @@ public class DefaultInstances implements InstanceProvider {
 	 * @param instaceProviders - array of instance providers.
 	 * @return An default instance generator.
 	 */
-	public static DefaultInstances fromArray(InstanceProvider... instaceProviders) {
+	public static DefaultInstances fromArray(InstanceProvider... instanceProviders) {
-		return new DefaultInstances(ImmutableList.copyOf(instaceProviders));
+		return new DefaultInstances(ImmutableList.copyOf(instanceProviders));
+	}
+	
+	/**
+	 * Construct a default instance generator using the given instance providers.
+	 * @param instaceProviders - collection of instance providers.
+	 * @return An default instance generator.
+	 */
+	public static DefaultInstances fromCollection(Collection<InstanceProvider> instanceProviders) {
+		return new DefaultInstances(ImmutableList.copyOf(instanceProviders));
 	}
 	
 	/**
@@ -241,12 +250,16 @@ public class DefaultInstances implements InstanceProvider {
 	private <T> T getDefaultInternal(Class<T> type, List<InstanceProvider> providers, int recursionLevel) {
 			
 		// The instance providiers should protect themselves against recursion
+		try {
 			for (InstanceProvider generator : providers) {
 				Object value = generator.create(type);
 				
 				if (value != null)
 					return (T) value;
 			}
+		} catch (NotConstructableException e) {
+			return null;
+		}
 
 		// Guard against recursion
 		if (recursionLevel >= maximumRecursion) {
@@ -276,7 +289,7 @@ public class DefaultInstances implements InstanceProvider {
 			}
 			
 		} catch (Exception e) {
-			// Nope, we couldn't create this type
+			// Nope, we couldn't create this type. Might for instance be NotConstructableException.
 		}
 		
 		// No suitable default value could be found

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/InstanceProvider.java

@@ -25,11 +25,11 @@ import javax.annotation.Nullable;
  * @author Kristian
  */
 public interface InstanceProvider {
-
 	/**
 	 * Create an instance given a type, if possible.
 	 * @param type - type to create.
 	 * @return The instance, or NULL if the type cannot be created.
+	 * @throws NotConstructableException Thrown to indicate that this type cannot or should never be constructed.
 	 */
 	public abstract Object create(@Nullable Class<?> type);
 }

ProtocolLib/src/main/java/com/comphenix/protocol/reflect/instances/NotConstructableException.java

@@ -0,0 +1,42 @@
+package com.comphenix.protocol.reflect.instances;
+
+/**
+ * Invoked when a instance provider indicates that a given type cannot or should not be 
+ * constructed under any circumstances.
+ * 
+ * @author Kristian
+ */
+public class NotConstructableException extends IllegalArgumentException {
+	/**
+	 * Generated by Eclipse.
+	 */
+	private static final long serialVersionUID = -1144171604744845463L;
+
+	/**
+	 * Construct a new not constructable exception.
+	 */
+	public NotConstructableException() {
+		super("This object should never be constructed.");
+	}
+	
+	/**
+	 * Construct a new not constructable exception with a custom message.
+	 */
+	public NotConstructableException(String message) {
+		super(message);
+	}
+	
+	/**
+	 * Construct a new not constructable exception with a custom message and cause.
+	 */
+	public NotConstructableException(String message, Throwable cause) {
+		super(message, cause);
+	}
+	
+	/**
+	 * Construct a new not constructable exception with a custom cause.
+	 */
+	public NotConstructableException(Throwable cause) {
+		super( cause);
+	}
+}

ProtocolLib/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java

@@ -25,6 +25,7 @@ import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.net.ServerSocket;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -587,6 +588,36 @@ public class MinecraftReflection {
 		}
 	}
 	
+	/**
+	 * Retrieve the Block (NMS) class.
+	 * @return Block (NMS) class.
+	 */
+	public static Class<?> getBlockClass() {
+		try {
+			return getMinecraftClass("Block");
+		} catch (RuntimeException e) {
+			FuzzyReflection reflect = FuzzyReflection.fromClass(getItemStackClass());
+			Set<Class<?>> candidates = new HashSet<Class<?>>();
+			
+			// Minecraft objects in the constructor
+			for (Constructor<?> constructor : reflect.getConstructors()) {
+				for (Class<?> clazz : constructor.getParameterTypes()) {
+					if (isMinecraftClass(clazz)) {
+						candidates.add(clazz);
+					}
+				}
+			}
+			
+			// Useful constructors
+			Method selected = 
+						reflect.getMethod(FuzzyMethodContract.newBuilder().
+							parameterMatches(FuzzyMatchers.matchAnyOf(candidates)).
+							returnTypeExact(float.class).
+						build());
+			return setMinecraftClass("Block", selected.getParameterTypes()[0]);
+		}
+	}
+		
 	/**
 	 * Retrieve the WorldType class.
 	 * @return The WorldType class.

ProtocolLib/src/main/java/com/comphenix/protocol/wrappers/WrappedDataWatcher.java

@@ -551,8 +551,17 @@ public class WrappedDataWatcher implements Iterable<WrappedWatchableObject> {
 		List<Method> candidates = fuzzy.getMethodListByParameters(Void.TYPE, 
 				  					new Class<?>[] { int.class, Object.class});
 		
-		for (Method method : candidates) {
+		// Load the get-method
+		try {
+			getKeyValueMethod = fuzzy.getMethodByParameters(
+					"getWatchableObject", MinecraftReflection.getWatchableObjectClass(), new Class[] { int.class });
+			getKeyValueMethod.setAccessible(true);
 			
+		} catch (IllegalArgumentException e) {
+			// Use the fallback method
+		}
+		
+		for (Method method : candidates) {
 			if (!method.getName().startsWith("watch")) {
 				createKeyValueMethod = method;
 			} else {
@@ -569,15 +578,21 @@ public class WrappedDataWatcher implements Iterable<WrappedWatchableObject> {
 			} else {
 				throw new IllegalStateException("Unable to find create and update watchable object. Update ProtocolLib.");
 			}
-		}
 			
-		// Load the get-method
+			// Be a little scientist - see if this in fact IS the right way around
 			try {
-			getKeyValueMethod = fuzzy.getMethodByParameters(
+				WrappedDataWatcher watcher = new WrappedDataWatcher();
-					"getWatchableObject", MinecraftReflection.getWatchableObjectClass(), new Class[] { int.class });
+				watcher.setObject(0, 0);
-			getKeyValueMethod.setAccessible(true);
+				watcher.setObject(0, 1);
-		} catch (IllegalArgumentException e) {
+				
-			// Use fallback method
+				if (watcher.getInteger(0) != 1) {
+					throw new IllegalStateException("This cannot be!");
+				}
+			} catch (Exception e) {
+				// Nope
+				updateKeyValueMethod = candidates.get(0);
+				createKeyValueMethod = candidates.get(1);
+			}
 		}
 	}