Prevent method #3 from creating uneeded proxy objects.

This would happen if the NetServerHandler is already a proxy, and
the injection method is unable to create an instance of the proxy
type. 

It's best to fail instead of polluting (due to constructors with
side-effects) Minecraft with failed proxy objects.

ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java

@@ -19,10 +19,8 @@ import com.comphenix.protocol.events.PacketListener;
 import com.comphenix.protocol.reflect.FieldUtils;
 import com.comphenix.protocol.reflect.FuzzyReflection;
 import com.comphenix.protocol.reflect.ObjectCloner;
-import com.comphenix.protocol.reflect.instances.CollectionGenerator;
 import com.comphenix.protocol.reflect.instances.DefaultInstances;
 import com.comphenix.protocol.reflect.instances.ExistingGenerator;
-import com.comphenix.protocol.reflect.instances.PrimitiveGenerator;
 
 /**
  * Represents a player hook into the NetServerHandler class. 
@@ -87,7 +85,7 @@ public class NetworkServerInjector extends PlayerInjector {
 		Callback sendPacketCallback = new MethodInterceptor() {
 			@Override
 			public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable {
-				
+
 				Packet packet = (Packet) args[0];
 				
 				if (packet != null) {
@@ -121,10 +119,10 @@ public class NetworkServerInjector extends PlayerInjector {
 		
 		// Use the existing field values when we create our copy
 		DefaultInstances serverInstances = DefaultInstances.fromArray(
-				ExistingGenerator.fromObjectFields(serverHandler),
+				ExistingGenerator.fromObjectFields(serverHandler));
-				PrimitiveGenerator.INSTANCE, 
+		serverInstances.setNonNull(true);
-				CollectionGenerator.INSTANCE);
+		serverInstances.setMaximumRecursion(1);
-
+		
 		Object proxyObject = serverInstances.forEnhancer(ex).getDefault(serverClass);
 		
 		// Inject it now
@@ -144,23 +142,23 @@ public class NetworkServerInjector extends PlayerInjector {
 		if (serverHandlerRef != null && serverHandlerRef.isCurrentSet()) {
 			ObjectCloner.copyTo(serverHandlerRef.getValue(), serverHandlerRef.getOldValue(), serverHandler.getClass());
 			serverHandlerRef.revertValue();
-		}
+			
-		
+			try {
-		serverInjection.revertServerHandler(serverHandler);
+				if (getNetHandler() != null) {
-		
+					// Restore packet listener
-		try {
+					try {
-			if (getNetHandler() != null) {
+						FieldUtils.writeField(netHandlerField, networkManager, serverHandlerRef.getOldValue(), true);
-				// Restore packet listener
+					} catch (IllegalAccessException e) {
-				try {
+						// Oh well
-					FieldUtils.writeField(netHandlerField, networkManager, serverHandlerRef.getOldValue(), true);
+						e.printStackTrace();
-				} catch (IllegalAccessException e) {
+					}
-					// Oh well
-					e.printStackTrace();
 				}
+			} catch (IllegalAccessException e) {
+				e.printStackTrace();
 			}
-		} catch (IllegalAccessException e) {
-			e.printStackTrace();
 		}
+
+		serverInjection.revertServerHandler(serverHandler);
 	}
 	
 	@Override

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

@@ -41,13 +41,18 @@ public class DefaultInstances {
 	/**
 	 * The maximum height of the hierachy of creates types. Used to prevent cycles.
 	 */
-	private final static int MAXIMUM_RECURSION = 20;
+	private int maximumRecursion = 20;
 	
 	/**
 	 * Ordered list of instance provider, from highest priority to lowest.
 	 */
 	private ImmutableList<InstanceProvider> registered;
 	
+	/**
+	 * Whether or not the constructor must be non-null.
+	 */
+	private boolean nonNull;
+	
 	/**
 	 * Construct a default instance generator using the given instance providers.
 	 * @param registered - list of instance providers.
@@ -56,6 +61,16 @@ public class DefaultInstances {
 		this.registered = registered;
 	}
 	
+	/**
+	 * Copy a given instance provider.
+	 * @param other - instance provider to copy.
+	 */
+	public DefaultInstances(DefaultInstances other) {
+		this.nonNull = other.nonNull;
+		this.maximumRecursion = other.maximumRecursion;
+		this.registered = other.registered;
+	}
+	
 	/**
 	 * Construct a default instance generator using the given instance providers.
 	 * @param instaceProviders - array of instance providers.
@@ -81,6 +96,40 @@ public class DefaultInstances {
 		return registered;
 	}
 	
+	/**
+	 * Retrieve whether or not the constructor's parameters must be non-null.
+	 * @return TRUE if they must be non-null, FALSE otherwise.
+	 */
+	public boolean isNonNull() {
+		return nonNull;
+	}
+
+	/**
+	 * Set whether or not the constructor's parameters must be non-null.
+	 * @param nonNull - TRUE if they must be non-null, FALSE otherwise.
+	 */
+	public void setNonNull(boolean nonNull) {
+		this.nonNull = nonNull;
+	}
+	
+	/**
+	 * Retrieve the the maximum height of the hierachy of creates types. 
+	 * @return Maximum height.
+	 */
+	public int getMaximumRecursion() {
+		return maximumRecursion;
+	}
+
+	/**
+	 * Set the maximum height of the hierachy of creates types. Used to prevent cycles.
+	 * @param maximumRecursion - maximum recursion height.
+	 */
+	public void setMaximumRecursion(int maximumRecursion) {
+		if (maximumRecursion < 1)
+			throw new IllegalArgumentException("Maxmimum recursion height must be one or higher.");
+		this.maximumRecursion = maximumRecursion;
+	}
+
 	/**
 	 * Retrieves a default instance or value that is assignable to this type.
 	 * <p>
@@ -158,12 +207,8 @@ public class DefaultInstances {
 	
 	@SuppressWarnings("unchecked")
 	private <T> T getDefaultInternal(Class<T> type, List<InstanceProvider> providers, int recursionLevel) {
-		
+			
-		// Guard against recursion
+		// The instance providiers should protect themselves against recursion
-		if (recursionLevel > MAXIMUM_RECURSION) {
-			return null;
-		}
-		
 		for (InstanceProvider generator : providers) {
 			Object value = generator.create(type);
 			
@@ -171,6 +216,11 @@ public class DefaultInstances {
 				return (T) value;
 		}
 
+		// Guard against recursion
+		if (recursionLevel >= maximumRecursion) {
+			return null;
+		}
+		
 		Constructor<T> minimum = getMinimumConstructor(type);
 
 		// Create the type with this constructor using default values. This might fail, though.
@@ -183,13 +233,18 @@ public class DefaultInstances {
 				// Fill out 
 				for (int i = 0; i < parameterCount; i++) {
 					params[i] = getDefaultInternal(types[i], providers, recursionLevel + 1);
+					
+					// Did we break the non-null contract?
+					if (params[i] == null && nonNull)
+						return null;
 				}
-				
+
 				return createInstance(type, minimum, types, params);
 			}
 			
 		} catch (Exception e) {
 			// Nope, we couldn't create this type
+			e.printStackTrace();
 		}
 		
 		// No suitable default value could be found
@@ -204,7 +259,7 @@ public class DefaultInstances {
 	public DefaultInstances forEnhancer(Enhancer enhancer) {
 		final Enhancer ex = enhancer;
 		
-		return new DefaultInstances(registered) {
+		return new DefaultInstances(this) {
 			@SuppressWarnings("unchecked")
 			@Override
 			protected <T> T createInstance(Class<T> type, Constructor<T> constructor, Class<?>[] types, Object[] params) {