From f4f28023fa64e085a408f3628465138020bd201d Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Mon, 1 Oct 2012 06:04:31 +0200 Subject: [PATCH 01/23] Always create the NullPacketListener (unless otherwise stated). This functions like a crude "counter" that automatically unregisters the packet injectors when every asynchronous listener has been removed. --- .../protocol/AsynchronousManager.java | 1 - .../protocol/async/AsyncFilterManager.java | 33 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/AsynchronousManager.java b/ProtocolLib/src/com/comphenix/protocol/AsynchronousManager.java index eca8466a..55d75f99 100644 --- a/ProtocolLib/src/com/comphenix/protocol/AsynchronousManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/AsynchronousManager.java @@ -20,7 +20,6 @@ public interface AsynchronousManager { * Registers an asynchronous packet handler. *

* To start listening asynchronously, pass the getListenerLoop() runnable to a different thread. - * @param plugin - the plugin that is registering the handler. * @param listener - the packet listener that will recieve these asynchronous events. * @return An asynchrouns handler. */ diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java index 0862bed4..9e6e6886 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java @@ -68,28 +68,40 @@ public class AsyncFilterManager implements AsynchronousManager { @Override public AsyncListenerHandler registerAsyncHandler(PacketListener listener) { + return registerAsyncHandler(listener, true); + } + + /** + * Registers an asynchronous packet handler. + *

+ * To start listening asynchronously, pass the getListenerLoop() runnable to a different thread. + *

+ * Asynchronous events will only be executed if a synchronous listener with the same packets is registered. + * If you already have a synchronous event, call this method with autoInject set to FALSE. + * + * @param listener - the packet listener that will recieve these asynchronous events. + * @param autoInject - whether or not to automatically create the corresponding synchronous listener, + * @return An asynchrouns handler. + */ + public AsyncListenerHandler registerAsyncHandler(PacketListener listener, boolean autoInject) { AsyncListenerHandler handler = new AsyncListenerHandler(mainThread, this, listener); ListeningWhitelist sendingWhitelist = listener.getSendingWhitelist(); ListeningWhitelist receivingWhitelist = listener.getReceivingWhitelist(); - // We need a synchronized listener to get the ball rolling - boolean hasListener = true; - // Add listener to either or both processing queue if (hasValidWhitelist(sendingWhitelist)) { PacketFilterManager.verifyWhitelist(listener, sendingWhitelist); serverProcessingQueue.addListener(handler, sendingWhitelist); - hasListener &= hasPacketListener(sendingWhitelist); } if (hasValidWhitelist(receivingWhitelist)) { PacketFilterManager.verifyWhitelist(listener, receivingWhitelist); clientProcessingQueue.addListener(handler, receivingWhitelist); - hasListener &= hasPacketListener(receivingWhitelist); } - if (!hasListener) { + // We need a synchronized listener to get the ball rolling + if (autoInject) { handler.setNullPacketListener(new NullPacketListener(listener)); manager.addPacketListener(handler.getNullPacketListener()); } @@ -97,15 +109,6 @@ public class AsyncFilterManager implements AsynchronousManager { return handler; } - /** - * Determine if the given packets are represented. - * @param whitelist - list of packets. - * @return TRUE if they are all registered, FALSE otherwise. - */ - private boolean hasPacketListener(ListeningWhitelist whitelist) { - return manager.getSendingFilters().containsAll(whitelist.getWhitelist()); - } - private boolean hasValidWhitelist(ListeningWhitelist whitelist) { return whitelist != null && whitelist.getWhitelist().size() > 0; } From 7b35dd954c6b9f94bdd8cd24c0853f4313258918 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Mon, 1 Oct 2012 20:30:56 +0200 Subject: [PATCH 02/23] Switching to the network server object. --- .../com/comphenix/protocol/injector/PacketFilterManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index e9009586..aac0b9ad 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -92,7 +92,7 @@ public final class PacketFilterManager implements ProtocolManager { private Map playerInjection = new HashMap(); // Player injection type - private PlayerInjectHooks playerHook = PlayerInjectHooks.NETWORK_HANDLER_FIELDS; + private PlayerInjectHooks playerHook = PlayerInjectHooks.NETWORK_SERVER_OBJECT; // Packet injection private PacketInjector packetInjector; From fe55bb2e56afdbecf796dee729191b3087b42f06 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Mon, 1 Oct 2012 21:47:19 +0200 Subject: [PATCH 03/23] Processed packets is now sorted by the sending index. --- .../protocol/async/AsyncFilterManager.java | 1 + .../protocol/async/PacketEventHolder.java | 40 ++++ .../protocol/async/PacketProcessingQueue.java | 29 ++- .../protocol/async/PacketSendingQueue.java | 33 ++- .../protocol/async/Synchronization.java | 211 ++++++++++++++++++ 5 files changed, 285 insertions(+), 29 deletions(-) create mode 100644 ProtocolLib/src/com/comphenix/protocol/async/PacketEventHolder.java create mode 100644 ProtocolLib/src/com/comphenix/protocol/async/Synchronization.java diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java index 9e6e6886..ecc841d0 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncFilterManager.java @@ -53,6 +53,7 @@ public class AsyncFilterManager implements AsynchronousManager { // Server packets are synchronized already this.serverQueue = new PacketSendingQueue(false); + // Client packets must be synchronized this.clientQueue = new PacketSendingQueue(true); diff --git a/ProtocolLib/src/com/comphenix/protocol/async/PacketEventHolder.java b/ProtocolLib/src/com/comphenix/protocol/async/PacketEventHolder.java new file mode 100644 index 00000000..429d5caf --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/async/PacketEventHolder.java @@ -0,0 +1,40 @@ +package com.comphenix.protocol.async; + +import com.comphenix.protocol.events.PacketEvent; +import com.google.common.base.Preconditions; +import com.google.common.collect.ComparisonChain; + +/** + * Provides a comparable to a packet event. + * + * @author Kristian + */ +class PacketEventHolder implements Comparable { + + private PacketEvent event; + + /** + * A wrapper that ensures the packet event is ordered by sending index. + * @param event - packet event to wrap. + */ + public PacketEventHolder(PacketEvent event) { + this.event = Preconditions.checkNotNull(event, "Event must be non-null"); + } + + /** + * Retrieve the stored event. + * @return The stored event. + */ + public PacketEvent getEvent() { + return event; + } + + @Override + public int compareTo(PacketEventHolder other) { + AsyncMarker marker = other != null ? other.getEvent().getAsyncMarker() : null; + + return ComparisonChain.start(). + compare(event.getAsyncMarker(), marker). + result(); + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java b/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java index 37349e83..02a3f93f 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java @@ -2,12 +2,14 @@ package com.comphenix.protocol.async; import java.util.Collection; import java.util.Iterator; -import java.util.concurrent.ArrayBlockingQueue; +import java.util.Queue; import java.util.concurrent.Semaphore; import com.comphenix.protocol.concurrency.AbstractConcurrentListenerMultimap; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.injector.PrioritizedListener; +import com.google.common.collect.MinMaxPriorityQueue; + /** * Handles the processing of every packet type. @@ -16,6 +18,9 @@ import com.comphenix.protocol.injector.PrioritizedListener; */ class PacketProcessingQueue extends AbstractConcurrentListenerMultimap { + // Initial number of elements + public static final int INITIAL_CAPACITY = 64; + /** * Default maximum number of packets to process concurrently. */ @@ -33,18 +38,23 @@ class PacketProcessingQueue extends AbstractConcurrentListenerMultimap processingQueue; + private Queue processingQueue; // Packets for sending private PacketSendingQueue sendingQueue; public PacketProcessingQueue(PacketSendingQueue sendingQueue) { - this(sendingQueue, DEFAULT_QUEUE_LIMIT, DEFAULT_MAXIMUM_CONCURRENCY); + this(sendingQueue, INITIAL_CAPACITY, DEFAULT_QUEUE_LIMIT, DEFAULT_MAXIMUM_CONCURRENCY); } - public PacketProcessingQueue(PacketSendingQueue sendingQueue, int queueLimit, int maximumConcurrency) { + public PacketProcessingQueue(PacketSendingQueue sendingQueue, int initialSize, int maximumSize, int maximumConcurrency) { super(); - this.processingQueue = new ArrayBlockingQueue(queueLimit); + + this.processingQueue = Synchronization.queue(MinMaxPriorityQueue. + expectedSize(initialSize). + maximumSize(maximumSize). + create(), null); + this.maximumConcurrency = maximumConcurrency; this.concurrentProcessing = new Semaphore(maximumConcurrency); this.sendingQueue = sendingQueue; @@ -58,7 +68,7 @@ class PacketProcessingQueue extends AbstractConcurrentListenerMultimap> list = getListener(packet.getPacketID()); + if (holder != null) { + PacketEvent packet = holder.getEvent(); AsyncMarker marker = packet.getAsyncMarker(); + Collection> list = getListener(packet.getPacketID()); // Yes, removing the marker will cause the chain to stop if (list != null) { diff --git a/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java b/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java index 500cf2c3..d2e9861e 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java @@ -1,7 +1,6 @@ package com.comphenix.protocol.async; import java.io.IOException; -import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -9,7 +8,6 @@ import java.util.concurrent.PriorityBlockingQueue; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.reflect.FieldAccessException; -import com.google.common.collect.ComparisonChain; /** * Represents packets ready to be transmitted to a client. @@ -17,9 +15,9 @@ import com.google.common.collect.ComparisonChain; */ class PacketSendingQueue { - private static final int INITIAL_CAPACITY = 64; + public static final int INITIAL_CAPACITY = 64; - private PriorityBlockingQueue sendingQueue; + private PriorityBlockingQueue sendingQueue; // Whether or not packet transmission can only occur on the main thread private final boolean synchronizeMain; @@ -29,16 +27,8 @@ class PacketSendingQueue { * @param synchronizeMain - whether or not to synchronize with the main thread. */ public PacketSendingQueue(boolean synchronizeMain) { + this.sendingQueue = new PriorityBlockingQueue(INITIAL_CAPACITY); this.synchronizeMain = synchronizeMain; - this.sendingQueue = new PriorityBlockingQueue(INITIAL_CAPACITY, new Comparator() { - // Compare using the async marker - @Override - public int compare(PacketEvent o1, PacketEvent o2) { - return ComparisonChain.start(). - compare(o1.getAsyncMarker(), o2.getAsyncMarker()). - result(); - } - }); } /** @@ -46,7 +36,7 @@ class PacketSendingQueue { * @param packet */ public void enqueue(PacketEvent packet) { - sendingQueue.add(packet); + sendingQueue.add(new PacketEventHolder(packet)); } /** @@ -70,7 +60,9 @@ class PacketSendingQueue { Set lookup = new HashSet(packetsRemoved); // Note that this is O(n), so it might be expensive - for (PacketEvent event : sendingQueue) { + for (PacketEventHolder holder : sendingQueue) { + PacketEvent event = holder.getEvent(); + if (lookup.contains(event.getPacketID())) { event.getAsyncMarker().setProcessed(true); } @@ -88,9 +80,10 @@ class PacketSendingQueue { // Transmit as many packets as we can while (true) { - PacketEvent current = sendingQueue.peek(); + PacketEventHolder holder = sendingQueue.peek(); - if (current != null) { + if (holder != null) { + PacketEvent current = holder.getEvent(); AsyncMarker marker = current.getAsyncMarker(); // Abort if we're not on the main thread @@ -129,10 +122,10 @@ class PacketSendingQueue { */ private void forceSend() { while (true) { - PacketEvent current = sendingQueue.poll(); + PacketEventHolder holder = sendingQueue.poll(); - if (current != null) { - sendPacket(current); + if (holder != null) { + sendPacket(holder.getEvent()); } else { break; } diff --git a/ProtocolLib/src/com/comphenix/protocol/async/Synchronization.java b/ProtocolLib/src/com/comphenix/protocol/async/Synchronization.java new file mode 100644 index 00000000..71e949e7 --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/async/Synchronization.java @@ -0,0 +1,211 @@ +package com.comphenix.protocol.async; + +import java.io.Serializable; +import java.util.Collection; +import java.util.Iterator; +import java.util.Queue; + +import javax.annotation.Nullable; + +import com.google.common.base.Preconditions; + +/** + * Synchronization views copied from Google Guava. + * + * @author Kristian + */ +class Synchronization { + + /** + * Create a synchronized wrapper for the given queue. + *

+ * This wrapper cannot synchronize the iterator(). Callers are expected + * to synchronize iterators manually. + * @param queue - the queue to synchronize. + * @param mutex - synchronization mutex, or NULL to use the queue. + * @return A synchronization wrapper. + */ + public static Queue queue(Queue queue, @Nullable Object mutex) { + return (queue instanceof SynchronizedQueue) ? + queue : + new SynchronizedQueue(queue, mutex); + } + + private static class SynchronizedObject implements Serializable { + private static final long serialVersionUID = -4408866092364554628L; + + final Object delegate; + final Object mutex; + + SynchronizedObject(Object delegate, @Nullable Object mutex) { + this.delegate = Preconditions.checkNotNull(delegate); + this.mutex = (mutex == null) ? this : mutex; + } + + Object delegate() { + return delegate; + } + + // No equals and hashCode; see ForwardingObject for details. + + @Override + public String toString() { + synchronized (mutex) { + return delegate.toString(); + } + } + } + + private static class SynchronizedCollection extends SynchronizedObject implements Collection { + private static final long serialVersionUID = 5440572373531285692L; + + private SynchronizedCollection(Collection delegate, + @Nullable Object mutex) { + super(delegate, mutex); + } + + @SuppressWarnings("unchecked") + @Override + Collection delegate() { + return (Collection) super.delegate(); + } + + @Override + public boolean add(E e) { + synchronized (mutex) { + return delegate().add(e); + } + } + + @Override + public boolean addAll(Collection c) { + synchronized (mutex) { + return delegate().addAll(c); + } + } + + @Override + public void clear() { + synchronized (mutex) { + delegate().clear(); + } + } + + @Override + public boolean contains(Object o) { + synchronized (mutex) { + return delegate().contains(o); + } + } + + @Override + public boolean containsAll(Collection c) { + synchronized (mutex) { + return delegate().containsAll(c); + } + } + + @Override + public boolean isEmpty() { + synchronized (mutex) { + return delegate().isEmpty(); + } + } + + @Override + public Iterator iterator() { + return delegate().iterator(); // manually synchronized + } + + @Override + public boolean remove(Object o) { + synchronized (mutex) { + return delegate().remove(o); + } + } + + @Override + public boolean removeAll(Collection c) { + synchronized (mutex) { + return delegate().removeAll(c); + } + } + + @Override + public boolean retainAll(Collection c) { + synchronized (mutex) { + return delegate().retainAll(c); + } + } + + @Override + public int size() { + synchronized (mutex) { + return delegate().size(); + } + } + + @Override + public Object[] toArray() { + synchronized (mutex) { + return delegate().toArray(); + } + } + + @Override + public T[] toArray(T[] a) { + synchronized (mutex) { + return delegate().toArray(a); + } + } + + } + + private static class SynchronizedQueue extends SynchronizedCollection implements Queue { + private static final long serialVersionUID = 1961791630386791902L; + + SynchronizedQueue(Queue delegate, @Nullable Object mutex) { + super(delegate, mutex); + } + + @Override + Queue delegate() { + return (Queue) super.delegate(); + } + + @Override + public E element() { + synchronized (mutex) { + return delegate().element(); + } + } + + @Override + public boolean offer(E e) { + synchronized (mutex) { + return delegate().offer(e); + } + } + + @Override + public E peek() { + synchronized (mutex) { + return delegate().peek(); + } + } + + @Override + public E poll() { + synchronized (mutex) { + return delegate().poll(); + } + } + + @Override + public E remove() { + synchronized (mutex) { + return delegate().remove(); + } + } + } +} From 0470b2335c7faf1c36500f448e06702831393bcb Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Mon, 1 Oct 2012 21:47:36 +0200 Subject: [PATCH 04/23] Bumping to 1.2.1 --- ProtocolLib/src/plugin.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ProtocolLib/src/plugin.yml b/ProtocolLib/src/plugin.yml index f0fd2022..b1d48571 100644 --- a/ProtocolLib/src/plugin.yml +++ b/ProtocolLib/src/plugin.yml @@ -1,5 +1,5 @@ name: ProtocolLib -version: 1.2.0 +version: 1.2.1 description: Provides read/write access to the Minecraft protocol. author: Comphenix website: http://www.comphenix.net/ProtocolLib From 2fceaa803e2b99c4426db24da83a7b0aa50bf24e Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 2 Oct 2012 03:54:50 +0200 Subject: [PATCH 05/23] Fixed the "moved too quickly" error that would cause players to fall out into the void on 1.3.2. --- .../injector/InjectedServerConnection.java | 19 +++++- .../injector/NetworkServerInjector.java | 32 ++-------- .../protocol/injector/ObjectCloner.java | 64 +++++++++++++++++++ .../protocol/injector/ReplacedArrayList.java | 25 ++++++-- 4 files changed, 107 insertions(+), 33 deletions(-) create mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java b/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java index ea2576db..2b92a150 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java @@ -7,6 +7,8 @@ import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; +import net.sf.cglib.proxy.Factory; + import org.bukkit.Server; import com.comphenix.protocol.reflect.FieldUtils; @@ -134,18 +136,33 @@ class InjectedServerConnection { if (list instanceof ReplacedArrayList) { replacedLists.add((ReplacedArrayList) list); } else { - replacedLists.add(new ReplacedArrayList(list)); + replacedLists.add(createReplacement(list)); listFieldRef.setValue(replacedLists.get(0)); listFields.add(listFieldRef); } } + // Hack to avoid the "moved to quickly" error + private ReplacedArrayList createReplacement(List list) { + return new ReplacedArrayList(list) { + @Override + protected void onReplacing(Object inserting, Object replacement) { + // Is this a normal Minecraft object? + if (!(inserting instanceof Factory)) { + // If so, copy the content of the old element to the new + ObjectCloner.copyTo(inserting, replacement, inserting.getClass()); + } + } + }; + } + /** * Replace the server handler instance kept by the "keep alive" object. * @param oldHandler - old server handler. * @param newHandler - new, proxied server handler. */ public void replaceServerHandler(Object oldHandler, Object newHandler) { + if (!hasAttempted) { injectList(); } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java index fb901a1a..5ea5bf0b 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java @@ -13,10 +13,8 @@ import net.sf.cglib.proxy.MethodProxy; import org.bukkit.entity.Player; import com.comphenix.protocol.events.PacketListener; -import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; -import com.comphenix.protocol.reflect.StructureModifier; import com.comphenix.protocol.reflect.instances.CollectionGenerator; import com.comphenix.protocol.reflect.instances.DefaultInstances; import com.comphenix.protocol.reflect.instances.ExistingGenerator; @@ -30,8 +28,6 @@ import com.comphenix.protocol.reflect.instances.PrimitiveGenerator; public class NetworkServerInjector extends PlayerInjector { private static Method sendPacketMethod; - - private StructureModifier serverHandlerModifier; private InjectedServerConnection serverInjection; public NetworkServerInjector(Player player, PacketFilterManager manager, @@ -48,8 +44,6 @@ public class NetworkServerInjector extends PlayerInjector { if (hasInitialized) { if (sendPacketMethod == null) sendPacketMethod = FuzzyReflection.fromObject(serverHandler).getMethodByName("sendPacket.*"); - if (serverHandlerModifier == null) - serverHandlerModifier = new StructureModifier(serverHandler.getClass(), null, false); } } @@ -119,41 +113,23 @@ public class NetworkServerInjector extends PlayerInjector { CollectionGenerator.INSTANCE); Object proxyObject = serverInstances.forEnhancer(ex).getDefault(serverClass); - serverInjection.replaceServerHandler(serverHandler, proxyObject); // Inject it now if (proxyObject != null) { - copyTo(serverHandler, proxyObject); + // This will be done by InjectedServerConnection instead + //copyTo(serverHandler, proxyObject); + serverInjection.replaceServerHandler(serverHandler, proxyObject); serverHandlerRef.setValue(proxyObject); } else { throw new RuntimeException( "Cannot hook player: Unable to find a valid constructor for the NetServerHandler object."); } } - - /** - * Copy every field in server handler A to server handler B. - * @param source - fields to copy. - * @param destination - fields to copy to. - */ - private void copyTo(Object source, Object destination) { - StructureModifier modifierSource = serverHandlerModifier.withTarget(source); - StructureModifier modifierDest = serverHandlerModifier.withTarget(destination); - // Copy every field - try { - for (int i = 0; i < modifierSource.size(); i++) { - modifierDest.write(i, modifierSource.read(i)); - } - } catch (FieldAccessException e) { - throw new RuntimeException("Unable to copy fields from NetServerHandler.", e); - } - } - @Override public void cleanupAll() { if (serverHandlerRef != null && serverHandlerRef.isCurrentSet()) { - copyTo(serverHandlerRef.getValue(), serverHandlerRef.getOldValue()); + ObjectCloner.copyTo(serverHandlerRef.getValue(), serverHandlerRef.getOldValue(), serverHandler.getClass()); serverHandlerRef.revertValue(); } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java b/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java new file mode 100644 index 00000000..c3cec1a3 --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java @@ -0,0 +1,64 @@ +package com.comphenix.protocol.injector; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import com.comphenix.protocol.reflect.FieldAccessException; +import com.comphenix.protocol.reflect.StructureModifier; + +/** + * Can copy an object field by field. + * + * @author Kristian + */ +class ObjectCloner { + + // Cache structure modifiers + @SuppressWarnings("rawtypes") + private static ConcurrentMap> cache = + new ConcurrentHashMap>(); + + /** + * Copy every field in object A to object B. + *

+ * The two objects must have the same number of fields of the same type. + * @param source - fields to copy. + * @param destination - fields to copy to. + * @param commonType - type containing each field to copy. + */ + public static void copyTo(Object source, Object destination, Class commonType) { + + if (source == null) + throw new IllegalArgumentException("Source cannot be NULL"); + if (destination == null) + throw new IllegalArgumentException("Destination cannot be NULL"); + + StructureModifier modifier = cache.get(commonType); + + // Create the structure modifier if we haven't already + if (modifier == null) { + StructureModifier value = new StructureModifier(commonType, null, false); + modifier = cache.putIfAbsent(commonType, value); + + if (modifier == null) + modifier = value; + } + + // Add target + StructureModifier modifierSource = modifier.withTarget(source); + StructureModifier modifierDest = modifier.withTarget(destination); + + // Copy every field + try { + for (int i = 0; i < modifierSource.size(); i++) { + Object value = modifierSource.read(i); + modifierDest.write(i, value); + + // System.out.println(String.format("Writing value %s to %s", + // value, modifier.getFields().get(i).getName())); + } + } catch (FieldAccessException e) { + throw new RuntimeException("Unable to copy fields from " + commonType.getName(), e); + } + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java b/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java index 64e3b0de..2923785c 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java @@ -24,10 +24,21 @@ class ReplacedArrayList extends ForwardingList { this.underlyingList = underlyingList; } + /** + * Invoked when a element inserted is replaced. + * @param inserting - the element inserted. + * @param replacement - the element that it should replace. + */ + protected void onReplacing(TKey inserting, TKey replacement) { + // Default is to do nothing. + } + @Override public boolean add(TKey element) { if (replaceMap.containsKey(element)) { - return super.add(replaceMap.get(element)); + TKey replacement = replaceMap.get(element); + onReplacing(element, replacement); + return super.add(replacement); } else { return super.add(element); } @@ -36,7 +47,9 @@ class ReplacedArrayList extends ForwardingList { @Override public void add(int index, TKey element) { if (replaceMap.containsKey(element)) { - super.add(index, replaceMap.get(element)); + TKey replacement = replaceMap.get(element); + onReplacing(element, replacement); + super.add(index, replacement); } else { super.add(index, element); } @@ -101,8 +114,10 @@ class ReplacedArrayList extends ForwardingList { */ public synchronized void replaceAll(TKey find, TKey replace) { for (int i = 0; i < underlyingList.size(); i++) { - if (Objects.equal(underlyingList.get(i), find)) + if (Objects.equal(underlyingList.get(i), find)) { + onReplacing(find, replace); underlyingList.set(i, replace); + } } } @@ -121,7 +136,9 @@ class ReplacedArrayList extends ForwardingList { TKey replaced = underlyingList.get(i); if (inverse.containsKey(replaced)) { - underlyingList.set(i, inverse.get(replaced)); + TKey original = inverse.get(replaced); + onReplacing(replaced, original); + underlyingList.set(i, original); } } From e4e4581717ddb51255d8cc77cc5617830ce3b02e Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 2 Oct 2012 03:55:07 +0200 Subject: [PATCH 06/23] Bumping to 1.2.2 --- ProtocolLib/src/plugin.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ProtocolLib/src/plugin.yml b/ProtocolLib/src/plugin.yml index b1d48571..19931803 100644 --- a/ProtocolLib/src/plugin.yml +++ b/ProtocolLib/src/plugin.yml @@ -1,5 +1,5 @@ name: ProtocolLib -version: 1.2.1 +version: 1.2.2 description: Provides read/write access to the Minecraft protocol. author: Comphenix website: http://www.comphenix.net/ProtocolLib From 48cedd20d46bde66ecca4a976dadff8ad0af81fb Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 2 Oct 2012 04:02:54 +0200 Subject: [PATCH 07/23] Add some size methods. --- .../protocol/async/PacketProcessingQueue.java | 10 +++++++++- .../comphenix/protocol/async/PacketSendingQueue.java | 8 ++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java b/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java index 02a3f93f..08ab06ad 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/PacketProcessingQueue.java @@ -69,7 +69,7 @@ class PacketProcessingQueue extends AbstractConcurrentListenerMultimap Date: Tue, 2 Oct 2012 04:36:47 +0200 Subject: [PATCH 08/23] Speed up the proxy object by using a NoOp filter. --- .../injector/NetworkServerInjector.java | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java index 5ea5bf0b..8325d659 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java @@ -5,10 +5,13 @@ import java.lang.reflect.Method; import java.util.Set; import net.minecraft.server.Packet; +import net.sf.cglib.proxy.Callback; +import net.sf.cglib.proxy.CallbackFilter; import net.sf.cglib.proxy.Enhancer; import net.sf.cglib.proxy.Factory; import net.sf.cglib.proxy.MethodInterceptor; import net.sf.cglib.proxy.MethodProxy; +import net.sf.cglib.proxy.NoOp; import org.bukkit.entity.Player; @@ -80,29 +83,38 @@ public class NetworkServerInjector extends PlayerInjector { Class serverClass = serverHandler.getClass(); Enhancer ex = new Enhancer(); - ex.setClassLoader(manager.getClassLoader()); - ex.setSuperclass(serverClass); - ex.setCallback(new MethodInterceptor() { + Callback sendPacketCallback = new MethodInterceptor() { @Override public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { - // The send packet method! - if (method.equals(sendPacketMethod)) { - Packet packet = (Packet) args[0]; + Packet packet = (Packet) args[0]; + + if (packet != null) { + packet = handlePacketRecieved(packet); - if (packet != null) { - packet = handlePacketRecieved(packet); - - // A NULL packet indicate cancelling - if (packet != null) - args[0] = packet; - else - return null; - } + // A NULL packet indicate cancelling + if (packet != null) + args[0] = packet; + else + return null; } // Call the method directly return proxy.invokeSuper(obj, args); + }; + }; + Callback noOpCallback = NoOp.INSTANCE; + + ex.setClassLoader(manager.getClassLoader()); + ex.setSuperclass(serverClass); + ex.setCallbacks(new Callback[] { sendPacketCallback, noOpCallback }); + ex.setCallbackFilter(new CallbackFilter() { + @Override + public int accept(Method method) { + if (method.equals(sendPacketMethod)) + return 0; + else + return 1; } }); From 3f1b5d49e928948d1778d8d73c6b8b871d6719ee Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 2 Oct 2012 23:05:31 +0200 Subject: [PATCH 09/23] Ignore offline players when sending overdue packets. --- .../protocol/async/PacketSendingQueue.java | 11 +++++- .../injector/InjectedServerConnection.java | 1 + .../injector/NetworkServerInjector.java | 1 + .../injector/PacketFilterManager.java | 15 +++++++- .../injector/PlayerLoggedOutException.java | 36 +++++++++++++++++++ .../{injector => reflect}/ObjectCloner.java | 6 ++-- 6 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java rename ProtocolLib/src/com/comphenix/protocol/{injector => reflect}/ObjectCloner.java (88%) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java b/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java index 027c0931..0899ed58 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/PacketSendingQueue.java @@ -6,6 +6,8 @@ import java.util.List; import java.util.Set; import java.util.concurrent.PriorityBlockingQueue; +import org.bukkit.entity.Player; + import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.reflect.FieldAccessException; @@ -112,7 +114,10 @@ class PacketSendingQueue { if (marker.isProcessed() || marker.hasExpired()) { if (marker.isProcessed() && !current.isCancelled()) { - sendPacket(current); + // Silently skip players that have logged out + if (isOnline(current.getPlayer())) { + sendPacket(current); + } } sendingQueue.poll(); @@ -125,6 +130,10 @@ class PacketSendingQueue { } } + private boolean isOnline(Player player) { + return player != null && player.isOnline(); + } + /** * Send every packet, regardless of the processing state. */ diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java b/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java index 2b92a150..ea987263 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java @@ -13,6 +13,7 @@ import org.bukkit.Server; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; +import com.comphenix.protocol.reflect.ObjectCloner; import com.comphenix.protocol.reflect.VolatileField; /** diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java index 8325d659..3c053311 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java @@ -18,6 +18,7 @@ import org.bukkit.entity.Player; 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; diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index aac0b9ad..3a3f6ea7 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -403,6 +403,8 @@ public final class PacketFilterManager implements ProtocolManager { if (packet == null) throw new IllegalArgumentException("packet cannot be NULL."); + sender.is + PlayerInjector injector = getInjector(sender); Packet mcPacket = packet.getHandle(); @@ -503,10 +505,21 @@ public final class PacketFilterManager implements ProtocolManager { try { injector = getHookInstance(player, currentHook); injector.injectManager(); + + DataInputStream inputStream = injector.getInputStream(false); + + if (!player.isOnline() || inputStream == null) { + throw new PlayerLoggedOutException(); + } + playerInjection.put(player, injector); - connectionLookup.put(injector.getInputStream(false), player); + connectionLookup.put(inputStream, player); break; + + } catch (PlayerLoggedOutException e) { + throw e; + } catch (Exception e) { // Mark this injection attempt as a failure diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java new file mode 100644 index 00000000..9d2f0c2b --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java @@ -0,0 +1,36 @@ +package com.comphenix.protocol.injector; + +/** + * Invoked when attempting to use a player that has already logged out. + * + * @author Kristian + */ +public class PlayerLoggedOutException extends RuntimeException { + + public PlayerLoggedOutException() { + // Default error message + super("Cannot inject a player that has already logged out."); + } + + public PlayerLoggedOutException(String message, Throwable cause) { + super(message, cause); + } + + public PlayerLoggedOutException(String message) { + super(message); + } + + public PlayerLoggedOutException(Throwable cause) { + super(cause); + } + + /** + * Construct an exception from a formatted message. + * @param message - the message to format. + * @param params - parameters. + * @return The formated exception + */ + public static PlayerLoggedOutException fromFormat(String message, Object... params) { + return new PlayerLoggedOutException(String.format(message, params)); + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java b/ProtocolLib/src/com/comphenix/protocol/reflect/ObjectCloner.java similarity index 88% rename from ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java rename to ProtocolLib/src/com/comphenix/protocol/reflect/ObjectCloner.java index c3cec1a3..65902cbd 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/ObjectCloner.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/ObjectCloner.java @@ -1,17 +1,15 @@ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.reflect; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import com.comphenix.protocol.reflect.FieldAccessException; -import com.comphenix.protocol.reflect.StructureModifier; /** * Can copy an object field by field. * * @author Kristian */ -class ObjectCloner { +public class ObjectCloner { // Cache structure modifiers @SuppressWarnings("rawtypes") From eb8ac33a3ec36d95ee78c1a1943055a7ccb76e6e Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Tue, 2 Oct 2012 23:07:03 +0200 Subject: [PATCH 10/23] Removed uncompleted code. --- .../com/comphenix/protocol/injector/PacketFilterManager.java | 2 -- .../protocol/injector/PlayerLoggedOutException.java | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index 3a3f6ea7..6c025328 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -403,8 +403,6 @@ public final class PacketFilterManager implements ProtocolManager { if (packet == null) throw new IllegalArgumentException("packet cannot be NULL."); - sender.is - PlayerInjector injector = getInjector(sender); Packet mcPacket = packet.getHandle(); diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java index 9d2f0c2b..5cdd569a 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerLoggedOutException.java @@ -7,6 +7,11 @@ package com.comphenix.protocol.injector; */ public class PlayerLoggedOutException extends RuntimeException { + /** + * Generated by Eclipse. + */ + private static final long serialVersionUID = 4889257862160145234L; + public PlayerLoggedOutException() { // Default error message super("Cannot inject a player that has already logged out."); From c6fb01e1e172b08b06adef60752fbf20ba05396c Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Wed, 3 Oct 2012 23:10:35 +0200 Subject: [PATCH 11/23] Adding the ability to use multiple worker threads. --- .../protocol/async/AsyncListenerHandler.java | 79 +++++++++++-------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java index e4cd15fc..047fb120 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java @@ -28,7 +28,7 @@ public class AsyncListenerHandler { private volatile boolean cancelled; // If we've started the listener loop before - private volatile boolean started; + private volatile int started; // The packet listener private PacketListener listener; @@ -78,18 +78,26 @@ public class AsyncListenerHandler { return nullPacketListener; } + private String getPluginName() { + return PacketAdapter.getPluginName(listener); + } + + /** + * Retrieve the plugin associated with this async listener. + * @return The plugin. + */ + public Plugin getPlugin() { + return listener != null ? listener.getPlugin() : null; + } + /** * Cancel the handler. */ public void cancel() { // Remove the listener as quickly as possible close(); - - // Poison Pill Shutdown - queuedPackets.clear(); - queuedPackets.add(INTERUPT_PACKET); } - + /** * Queue a packet for processing. * @param packet - a packet for processing. @@ -116,19 +124,36 @@ public class AsyncListenerHandler { }; } + /** + * Start a singler worker thread handling the asynchronous. + */ + public void start() { + if (listener.getPlugin() == null) + throw new IllegalArgumentException("Cannot start task without a valid plugin."); + + filterManager.scheduleAsyncTask(listener.getPlugin(), getListenerLoop()); + } + + /** + * Start multiple worker threads for this listener. + * @param count - number of worker threads to start. + */ + public void start(int count) { + for (int i = 0; i < count; i++) + start(); + } + // DO NOT call this method from the main thread private void listenerLoop() { // Danger, danger! if (Thread.currentThread().getId() == mainThread.getId()) throw new IllegalStateException("Do not call this method from the main thread."); - if (started) - throw new IllegalStateException("A listener cannot be run by multiple threads. Create a new listener instead."); if (cancelled) throw new IllegalStateException("Listener has been cancelled. Create a new listener instead."); // Proceed - started = true; + started++; try { mainLoop: @@ -171,40 +196,32 @@ public class AsyncListenerHandler { } catch (InterruptedException e) { // We're done + } finally { + // Clean up + started--; + close(); } - - // Clean up - close(); } - private void close() { + private synchronized void close() { // Remove the listener itself if (!cancelled) { filterManager.unregisterAsyncHandlerInternal(this); cancelled = true; - started = false; + + // Tell every uncancelled thread to end + stopThreads(); } } - private String getPluginName() { - return PacketAdapter.getPluginName(listener); - } - /** - * Retrieve the plugin associated with this async listener. - * @return The plugin. + * Use the poision pill method to stop every worker thread. */ - public Plugin getPlugin() { - return listener != null ? listener.getPlugin() : null; - } - - /** - * Start the asynchronous listener using the Bukkit scheduler. - */ - public void start() { - if (listener.getPlugin() == null) - throw new IllegalArgumentException("Cannot start task without a valid plugin."); + private void stopThreads() { + // Poison Pill Shutdown + queuedPackets.clear(); - filterManager.scheduleAsyncTask(listener.getPlugin(), getListenerLoop()); + for (int i = 0; i < started; i++) + queuedPackets.add(INTERUPT_PACKET); } } From 558eab225335fe7d961b44e8fdefc487256c7b9e Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Wed, 3 Oct 2012 23:13:05 +0200 Subject: [PATCH 12/23] Use an atomic integer instead of a volatile field. Incrementing and decrementing is not thread-safe on volatile variables. --- .../comphenix/protocol/async/AsyncListenerHandler.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java index 047fb120..5668647f 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java @@ -1,6 +1,7 @@ package com.comphenix.protocol.async; import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import org.bukkit.plugin.Plugin; @@ -28,7 +29,7 @@ public class AsyncListenerHandler { private volatile boolean cancelled; // If we've started the listener loop before - private volatile int started; + private AtomicInteger started = new AtomicInteger(); // The packet listener private PacketListener listener; @@ -153,7 +154,7 @@ public class AsyncListenerHandler { throw new IllegalStateException("Listener has been cancelled. Create a new listener instead."); // Proceed - started++; + started.incrementAndGet(); try { mainLoop: @@ -198,7 +199,7 @@ public class AsyncListenerHandler { // We're done } finally { // Clean up - started--; + started.decrementAndGet(); close(); } } @@ -221,7 +222,7 @@ public class AsyncListenerHandler { // Poison Pill Shutdown queuedPackets.clear(); - for (int i = 0; i < started; i++) + for (int i = 0; i < started.get(); i++) queuedPackets.add(INTERUPT_PACKET); } } From 8a5e5e849bc1e47ff5f440c9a405b691b624222b Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 00:58:05 +0200 Subject: [PATCH 13/23] Attempt to work around existing injected proxies. This didn't work for Orebfuscator though. --- .../protocol/injector/PlayerInjector.java | 60 +++++++++++++++++-- .../protocol/reflect/FuzzyReflection.java | 4 +- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java index 2ae2976d..7d88e25f 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java @@ -22,9 +22,11 @@ import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Set; +import java.util.logging.Level; import net.minecraft.server.EntityPlayer; import net.minecraft.server.Packet; +import net.sf.cglib.proxy.Factory; import org.bukkit.craftbukkit.entity.CraftPlayer; import org.bukkit.entity.Player; @@ -40,7 +42,9 @@ import com.comphenix.protocol.reflect.VolatileField; abstract class PlayerInjector { // Cache previously retrieved fields - protected static Field serverHandlerField; + private static Field serverHandlerField; + private static Field proxyServerField; + protected static Field networkManagerField; protected static Field inputField; protected static Field netHandlerField; @@ -96,17 +100,26 @@ abstract class PlayerInjector { hasInitialized = true; // Retrieve the server handler - if (serverHandlerField == null) + if (serverHandlerField == null) { serverHandlerField = FuzzyReflection.fromObject(notchEntity).getFieldByType(".*NetServerHandler"); - serverHandlerRef = new VolatileField(serverHandlerField, notchEntity); - serverHandler = serverHandlerRef.getValue(); + proxyServerField = getProxyField(notchEntity, serverHandlerField); + } + // Yo dawg + if (proxyServerField != null) { + Object container = FieldUtils.readField(serverHandlerField, notchEntity, true); + serverHandlerRef = new VolatileField(proxyServerField, container); + } else { + serverHandlerRef = new VolatileField(serverHandlerField, notchEntity); + } + serverHandler = serverHandlerRef.getValue(); + // Next, get the network manager if (networkManagerField == null) networkManagerField = FuzzyReflection.fromObject(serverHandler).getFieldByType(".*NetworkManager"); networkManagerRef = new VolatileField(networkManagerField, serverHandler); networkManager = networkManagerRef.getValue(); - + // Create the network manager modifier from the actual object type if (networkManager != null && networkModifier == null) networkModifier = new StructureModifier(networkManager.getClass(), null, false); @@ -123,6 +136,38 @@ abstract class PlayerInjector { } } + private Field getProxyField(EntityPlayer notchEntity, Field serverField) { + + try { + Object handler = FieldUtils.readField(serverHandlerField, notchEntity, true); + + // Is this a Minecraft hook? + if (handler != null && !handler.getClass().getName().startsWith("net.minecraft.server")) { + + // This is our proxy object + if (handler instanceof Factory) + return null; + + // No? Is it a Proxy type? + try { + FuzzyReflection reflection = FuzzyReflection.fromObject(handler, true); + + // It might be + return reflection.getFieldByType(".*NetServerHandler"); + + } catch (RuntimeException e) { + manager.getLogger().log(Level.WARNING, "Server handler is a proxy type.", e); + } + } + + } catch (IllegalAccessException e) { + manager.getLogger().warning("Unable to load server handler from proxy type."); + } + + // Nope, just go with it + return null; + } + /** * Retrieves the current net handler for this player. * @return Current net handler. @@ -246,6 +291,11 @@ abstract class PlayerInjector { * @return The player's input stream. */ public DataInputStream getInputStream(boolean cache) { + if (inputField == null) + throw new IllegalStateException("Input field is NULL."); + if (networkManager == null) + throw new IllegalStateException("Network manager is NULL."); + // Get the associated input stream try { if (cache && cachedInput != null) diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/FuzzyReflection.java b/ProtocolLib/src/com/comphenix/protocol/reflect/FuzzyReflection.java index 448507f7..d9f0bad7 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/FuzzyReflection.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/FuzzyReflection.java @@ -292,7 +292,9 @@ public class FuzzyReflection { // Like above, only here we test the field type for (Field field : getFields()) { - if (match.matcher(field.getType().getName()).matches()) { + String name = field.getType().getName(); + + if (match.matcher(name).matches()) { return field; } } From e729583d74812caea8691fd56de75204e86ebc26 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 03:20:09 +0200 Subject: [PATCH 14/23] Make the manual asynchronous worker closable. Added a good deal of synchronization to deal with closing a specific worker. This overhead can be avoided by simply closing any given worker. --- .../protocol/async/AsyncListenerHandler.java | 179 ++++++++++++++++-- .../protocol/async/AsyncRunnable.java | 23 +++ 2 files changed, 185 insertions(+), 17 deletions(-) create mode 100644 ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java index 5668647f..f5385fad 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java @@ -1,6 +1,9 @@ package com.comphenix.protocol.async; +import java.util.HashSet; +import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; @@ -18,18 +21,26 @@ import com.comphenix.protocol.events.PacketListener; public class AsyncListenerHandler { /** - * Signal an end to the packet processing. + * Signal an end to packet processing. */ private static final PacketEvent INTERUPT_PACKET = new PacketEvent(new Object()); + /** + * Called when the threads have to wake up for something important. + */ + private static final PacketEvent WAKEUP_PACKET = new PacketEvent(new Object()); + // Default queue capacity private static int DEFAULT_CAPACITY = 1024; // Cancel the async handler private volatile boolean cancelled; - // If we've started the listener loop before - private AtomicInteger started = new AtomicInteger(); + // Number of worker threads + private final AtomicInteger started = new AtomicInteger(); + + // Unique local worker ID + private final AtomicInteger nextID = new AtomicInteger(); // The packet listener private PacketListener listener; @@ -41,6 +52,10 @@ public class AsyncListenerHandler { // List of queued packets private ArrayBlockingQueue queuedPackets = new ArrayBlockingQueue(DEFAULT_CAPACITY); + // List of cancelled tasks + private final Set stoppedTasks = new HashSet(); + private final Object stopLock = new Object(); + // Minecraft main thread private Thread mainThread; @@ -112,15 +127,57 @@ public class AsyncListenerHandler { } /** - * Create a runnable that will initiate the listener loop. + * Create a worker that will initiate the listener loop. Note that using stop() to + * close a specific worker is less efficient than stopping an arbitrary worker. *

* Warning: Never call the run() method in the main thread. */ public Runnable getListenerLoop() { - return new Runnable() { + return new AsyncRunnable() { + + private final AtomicBoolean running = new AtomicBoolean(); + private volatile int id; + @Override public void run() { - listenerLoop(); + // Careful now + if (running.compareAndSet(false, true)) { + id = nextID.incrementAndGet(); + listenerLoop(id); + + synchronized (stopLock) { + stoppedTasks.remove(id); + notifyAll(); + running.set(false); + } + + } else { + throw new IllegalStateException( + "This listener loop has already been started. Create a new instead."); + } + } + + @Override + public boolean stop() throws InterruptedException { + synchronized (stopLock) { + if (!running.get()) + return false; + + stoppedTasks.add(id); + + // Wake up threads - we have a listener to stop + for (int i = 0; i < getWorkers(); i++) { + queuedPackets.offer(WAKEUP_PACKET); + } + + waitForStops(); + return true; + } + } + + @Override + public boolean isRunning() { + return running.get(); } }; } @@ -128,9 +185,11 @@ public class AsyncListenerHandler { /** * Start a singler worker thread handling the asynchronous. */ - public void start() { + public synchronized void start() { if (listener.getPlugin() == null) throw new IllegalArgumentException("Cannot start task without a valid plugin."); + if (cancelled) + throw new IllegalStateException("Cannot start a worker when the listener is closing."); filterManager.scheduleAsyncTask(listener.getPlugin(), getListenerLoop()); } @@ -139,32 +198,115 @@ public class AsyncListenerHandler { * Start multiple worker threads for this listener. * @param count - number of worker threads to start. */ - public void start(int count) { + public synchronized void start(int count) { for (int i = 0; i < count; i++) start(); } + /** + * Stop a worker thread. + */ + public synchronized void stop() { + queuedPackets.add(INTERUPT_PACKET); + } + + /** + * Stop the given amount of worker threads. + * @param count - number of threads to stop. + */ + public synchronized void stop(int count) { + for (int i = 0; i < count; i++) + stop(); + } + + /** + * Set the current number of workers. + *

+ * This method can only be called with a count of zero when the listener is closing. + * @param count - new number of workers. + */ + public synchronized void setWorkers(int count) { + if (count < 0) + throw new IllegalArgumentException("Number of workers cannot be less than zero."); + if (count > DEFAULT_CAPACITY) + throw new IllegalArgumentException("Cannot initiate more than " + DEFAULT_CAPACITY + " workers"); + if (cancelled && count > 0) + throw new IllegalArgumentException("Cannot add workers when the listener is closing."); + + long time = System.currentTimeMillis(); + + // Try to get to the correct count + while (started.get() != count) { + if (started.get() < count) + start(); + else + stop(); + + // May happen if another thread is doing something similar to "setWorkers" + if ((System.currentTimeMillis() - time) > 1000) + throw new RuntimeException("Failed to set worker count."); + } + } + + /** + * Retrieve the current number of registered workers. + *

+ * Note that the returned value may be out of data. + * @return Number of registered workers. + */ + public synchronized int getWorkers() { + return started.get(); + } + + /** + * Wait until every tasks scheduled to stop has actually stopped. + * @return TRUE if the current listener should stop, FALSE otherwise. + * @throws InterruptedException - If the current thread was interrupted. + */ + private boolean waitForStops() throws InterruptedException { + while (stoppedTasks.size() > 0 && !cancelled) { + wait(); + } + return cancelled; + } + // DO NOT call this method from the main thread - private void listenerLoop() { + private void listenerLoop(int taskID) { // Danger, danger! if (Thread.currentThread().getId() == mainThread.getId()) throw new IllegalStateException("Do not call this method from the main thread."); if (cancelled) throw new IllegalStateException("Listener has been cancelled. Create a new listener instead."); - - // Proceed - started.incrementAndGet(); - + try { + // Wait if certain threads are stopping + synchronized (stopLock) { + if (waitForStops()) + return; + } + + // Proceed + started.incrementAndGet(); + mainLoop: while (!cancelled) { PacketEvent packet = queuedPackets.take(); AsyncMarker marker = packet.getAsyncMarker(); // Handle cancel requests - if (packet == null || marker == null || !packet.isAsynchronous()) { - break; + if (packet == null || marker == null || packet == INTERUPT_PACKET) { + return; + + } else if (packet == WAKEUP_PACKET) { + // This is a bit slow, but it should be safe + synchronized (stopLock) { + // Are we the one who is supposed to stop? + if (stoppedTasks.contains(taskID)) + return; + if (waitForStops()) + return; + } } // Here's the core of the asynchronous processing @@ -221,8 +363,11 @@ public class AsyncListenerHandler { private void stopThreads() { // Poison Pill Shutdown queuedPackets.clear(); + stop(started.get()); - for (int i = 0; i < started.get(); i++) - queuedPackets.add(INTERUPT_PACKET); + // Individual shut down is irrelevant now + synchronized (stopLock) { + notifyAll(); + } } } diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java new file mode 100644 index 00000000..281b71be --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java @@ -0,0 +1,23 @@ +package com.comphenix.protocol.async; + +/** + * A runnable representing a asynchronous event listener. + * + * @author Kristian + */ +public interface AsyncRunnable extends Runnable { + + /** + * Stop the given runnable. + *

+ * This may not occur right away. + * @return TRUE if the thread was stopped, FALSE if it was already stopped. + */ + public boolean stop() throws InterruptedException; + + /** + * Determine if we're running or not. + * @return TRUE if we're running, FALSE otherwise. + */ + public boolean isRunning(); +} From db8db1fba198fe1426641eabdca3a8e7b8f03e38 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 05:14:17 +0200 Subject: [PATCH 15/23] Make it possible to identify the current worker. --- .../protocol/async/AsyncListenerHandler.java | 20 ++++++--- .../comphenix/protocol/async/AsyncMarker.java | 44 ++++++++++++++++++- .../protocol/async/AsyncRunnable.java | 6 +++ 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java index f5385fad..137327b8 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java @@ -30,6 +30,9 @@ public class AsyncListenerHandler { */ private static final PacketEvent WAKEUP_PACKET = new PacketEvent(new Object()); + // Unique worker ID + private static final AtomicInteger nextID = new AtomicInteger(); + // Default queue capacity private static int DEFAULT_CAPACITY = 1024; @@ -39,9 +42,6 @@ public class AsyncListenerHandler { // Number of worker threads private final AtomicInteger started = new AtomicInteger(); - // Unique local worker ID - private final AtomicInteger nextID = new AtomicInteger(); - // The packet listener private PacketListener listener; @@ -132,12 +132,17 @@ public class AsyncListenerHandler { *

* Warning: Never call the run() method in the main thread. */ - public Runnable getListenerLoop() { + public AsyncRunnable getListenerLoop() { return new AsyncRunnable() { private final AtomicBoolean running = new AtomicBoolean(); private volatile int id; + @Override + public int getID() { + return id; + } + @Override public void run() { // Careful now @@ -271,7 +276,7 @@ public class AsyncListenerHandler { } // DO NOT call this method from the main thread - private void listenerLoop(int taskID) { + private void listenerLoop(int workerID) { // Danger, danger! if (Thread.currentThread().getId() == mainThread.getId()) @@ -302,7 +307,7 @@ public class AsyncListenerHandler { // This is a bit slow, but it should be safe synchronized (stopLock) { // Are we the one who is supposed to stop? - if (stoppedTasks.contains(taskID)) + if (stoppedTasks.contains(workerID)) return; if (waitForStops()) return; @@ -311,6 +316,9 @@ public class AsyncListenerHandler { // Here's the core of the asynchronous processing try { + marker.setListenerHandler(this); + marker.setWorkerID(workerID); + if (packet.isServerPacket()) listener.onPacketSending(packet); else diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncMarker.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncMarker.java index 8cecb1c8..a5e15f0e 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncMarker.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncMarker.java @@ -67,6 +67,10 @@ public class AsyncMarker implements Serializable, Comparable { // Whether or not the asynchronous processing itself should be cancelled private volatile boolean asyncCancelled; + // Used to identify the asynchronous worker + private AsyncListenerHandler listenerHandler; + private int workerID; + // Determine if Minecraft processes this packet asynchronously private static Method isMinecraftAsync; private static boolean alwaysSync; @@ -214,12 +218,48 @@ public class AsyncMarker implements Serializable, Comparable { public void setAsyncCancelled(boolean asyncCancelled) { this.asyncCancelled = asyncCancelled; } - + + /** + * Retrieve the current asynchronous listener handler. + * @return Asychronous listener handler, or NULL if this packet is not asynchronous. + */ + public AsyncListenerHandler getListenerHandler() { + return listenerHandler; + } + + /** + * Set the current asynchronous listener handler. + *

+ * Used by the worker to update the value. + * @param listenerHandler - new listener handler. + */ + void setListenerHandler(AsyncListenerHandler listenerHandler) { + this.listenerHandler = listenerHandler; + } + + /** + * Retrieve the current worker ID. + * @return Current worker ID. + */ + public int getWorkerID() { + return workerID; + } + + /** + * Set the current worker ID. + *

+ * Used by the worker. + * @param workerID - new worker ID. + */ + void setWorkerID(int workerID) { + this.workerID = workerID; + } + /** * Retrieve iterator for the next listener in line. * @return Next async packet listener iterator. */ - public Iterator> getListenerTraversal() { + Iterator> getListenerTraversal() { return listenerTraversal; } diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java index 281b71be..7e56a6b6 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java @@ -7,6 +7,12 @@ package com.comphenix.protocol.async; */ public interface AsyncRunnable extends Runnable { + /** + * Retrieve a unique worker ID. + * @return Unique worker ID. + */ + public int getID(); + /** * Stop the given runnable. *

From af2d692c5953f9cd1beee70a2aa9b2e45a7862b6 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 06:13:19 +0200 Subject: [PATCH 16/23] Wait and notify on the correct lock. --- .../protocol/async/AsyncListenerHandler.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java index 137327b8..8e729875 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java @@ -152,7 +152,7 @@ public class AsyncListenerHandler { synchronized (stopLock) { stoppedTasks.remove(id); - notifyAll(); + stopLock.notifyAll(); running.set(false); } @@ -269,10 +269,12 @@ public class AsyncListenerHandler { * @throws InterruptedException - If the current thread was interrupted. */ private boolean waitForStops() throws InterruptedException { - while (stoppedTasks.size() > 0 && !cancelled) { - wait(); + synchronized (stopLock) { + while (stoppedTasks.size() > 0 && !cancelled) { + stopLock.wait(); + } + return cancelled; } - return cancelled; } // DO NOT call this method from the main thread @@ -286,10 +288,8 @@ public class AsyncListenerHandler { try { // Wait if certain threads are stopping - synchronized (stopLock) { - if (waitForStops()) - return; - } + if (waitForStops()) + return; // Proceed started.incrementAndGet(); @@ -375,7 +375,7 @@ public class AsyncListenerHandler { // Individual shut down is irrelevant now synchronized (stopLock) { - notifyAll(); + stopLock.notifyAll(); } } } From debf8c4d889426da298130f637f0c31535f22763 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 06:23:27 +0200 Subject: [PATCH 17/23] Create the worker ID before it is run. In addition, we'll prevent reuse of worker objects. --- .../protocol/async/AsyncListenerHandler.java | 26 +++++++++++++------ .../protocol/async/AsyncRunnable.java | 6 +++++ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java index 8e729875..87c09e26 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncListenerHandler.java @@ -135,8 +135,9 @@ public class AsyncListenerHandler { public AsyncRunnable getListenerLoop() { return new AsyncRunnable() { - private final AtomicBoolean running = new AtomicBoolean(); - private volatile int id; + private final AtomicBoolean firstRun = new AtomicBoolean(); + private final AtomicBoolean finished = new AtomicBoolean(); + private final int id = nextID.incrementAndGet(); @Override public int getID() { @@ -146,18 +147,21 @@ public class AsyncListenerHandler { @Override public void run() { // Careful now - if (running.compareAndSet(false, true)) { - id = nextID.incrementAndGet(); + if (firstRun.compareAndSet(false, true)) { listenerLoop(id); synchronized (stopLock) { stoppedTasks.remove(id); stopLock.notifyAll(); - running.set(false); + finished.set(true); } } else { - throw new IllegalStateException( + if (finished.get()) + throw new IllegalStateException( + "This listener has already been run. Create a new instead."); + else + throw new IllegalStateException( "This listener loop has already been started. Create a new instead."); } } @@ -165,7 +169,7 @@ public class AsyncListenerHandler { @Override public boolean stop() throws InterruptedException { synchronized (stopLock) { - if (!running.get()) + if (!isRunning()) return false; stoppedTasks.add(id); @@ -175,6 +179,7 @@ public class AsyncListenerHandler { queuedPackets.offer(WAKEUP_PACKET); } + finished.set(true); waitForStops(); return true; } @@ -182,7 +187,12 @@ public class AsyncListenerHandler { @Override public boolean isRunning() { - return running.get(); + return firstRun.get() && !finished.get(); + } + + @Override + public boolean isFinished() { + return finished.get(); } }; } diff --git a/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java b/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java index 7e56a6b6..2c0004d9 100644 --- a/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java +++ b/ProtocolLib/src/com/comphenix/protocol/async/AsyncRunnable.java @@ -26,4 +26,10 @@ public interface AsyncRunnable extends Runnable { * @return TRUE if we're running, FALSE otherwise. */ public boolean isRunning(); + + /** + * Determine if this runnable has already run its course. + * @return TRUE if it has been stopped, FALSE otherwise. + */ + boolean isFinished(); } From f0651f7170d3ae14419b6b621e0302dd5a2e497e Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 09:01:33 +0200 Subject: [PATCH 18/23] Don't blow up the fallback method just because a plugin isn't compatible. --- .../comphenix/protocol/injector/PacketFilterManager.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index 6c025328..68333b7d 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -166,7 +166,11 @@ public final class PacketFilterManager implements ProtocolManager { // Make sure the current listeners are compatible if (lastSuccessfulHook != null) { for (PacketListener listener : packetListeners) { - checkListener(listener); + try { + checkListener(listener); + } catch (IllegalStateException e) { + logger.log(Level.WARNING, "Unsupported listener.", e); + } } } } From eb12808483e753430e8f82d256bdb1dcb1f9b342 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 17:43:46 +0200 Subject: [PATCH 19/23] 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. --- .../injector/NetworkServerInjector.java | 40 +++++----- .../reflect/instances/DefaultInstances.java | 73 ++++++++++++++++--- 2 files changed, 83 insertions(+), 30 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java index 3c053311..50f311ad 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java +++ b/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), - PrimitiveGenerator.INSTANCE, - CollectionGenerator.INSTANCE); - + ExistingGenerator.fromObjectFields(serverHandler)); + serverInstances.setNonNull(true); + 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(); - } - - serverInjection.revertServerHandler(serverHandler); - - try { - if (getNetHandler() != null) { - // Restore packet listener - try { - FieldUtils.writeField(netHandlerField, networkManager, serverHandlerRef.getOldValue(), true); - } catch (IllegalAccessException e) { - // Oh well - e.printStackTrace(); + + try { + if (getNetHandler() != null) { + // Restore packet listener + try { + FieldUtils.writeField(netHandlerField, networkManager, serverHandlerRef.getOldValue(), true); + } catch (IllegalAccessException e) { + // Oh well + e.printStackTrace(); + } } + } catch (IllegalAccessException e) { + e.printStackTrace(); } - } catch (IllegalAccessException e) { - e.printStackTrace(); } + + serverInjection.revertServerHandler(serverHandler); } @Override diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java index 7b846077..8c3b8ebc 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java +++ b/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 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. *

@@ -158,12 +207,8 @@ public class DefaultInstances { @SuppressWarnings("unchecked") private T getDefaultInternal(Class type, List providers, int recursionLevel) { - - // Guard against recursion - if (recursionLevel > MAXIMUM_RECURSION) { - return null; - } - + + // The instance providiers should protect themselves against recursion 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 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 createInstance(Class type, Constructor constructor, Class[] types, Object[] params) { From af3e278e06850b19b8ae02d6213d672fdde32a55 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Thu, 4 Oct 2012 21:56:39 +0200 Subject: [PATCH 20/23] Moved all player related injection methods into a separate package. This should make the code a little bit clearer. --- ProtocolLib/.classpath | 1 + .../protocol/injector/ListenerInvoker.java | 27 ++ .../injector/PacketFilterManager.java | 245 +++----------- .../protocol/injector/PacketInjector.java | 24 +- .../{ => player}/InjectedArrayList.java | 6 +- .../InjectedServerConnection.java | 2 +- .../{ => player}/NetworkFieldInjector.java | 32 +- .../{ => player}/NetworkObjectInjector.java | 22 +- .../{ => player}/NetworkServerInjector.java | 35 +- .../injector/player/NetworkSpoutHook.java | 60 ++++ .../player/PlayerInjectionHandler.java | 306 ++++++++++++++++++ .../injector/{ => player}/PlayerInjector.java | 51 ++- .../{ => player}/ReplacedArrayList.java | 2 +- 13 files changed, 572 insertions(+), 241 deletions(-) create mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/ListenerInvoker.java rename ProtocolLib/src/com/comphenix/protocol/injector/{ => player}/InjectedArrayList.java (93%) rename ProtocolLib/src/com/comphenix/protocol/injector/{ => player}/InjectedServerConnection.java (95%) rename ProtocolLib/src/com/comphenix/protocol/injector/{ => player}/NetworkFieldInjector.java (79%) rename ProtocolLib/src/com/comphenix/protocol/injector/{ => player}/NetworkObjectInjector.java (81%) rename ProtocolLib/src/com/comphenix/protocol/injector/{ => player}/NetworkServerInjector.java (81%) create mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java create mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java rename ProtocolLib/src/com/comphenix/protocol/injector/{ => player}/PlayerInjector.java (84%) rename ProtocolLib/src/com/comphenix/protocol/injector/{ => player}/ReplacedArrayList.java (94%) diff --git a/ProtocolLib/.classpath b/ProtocolLib/.classpath index 0bc481ca..408271fe 100644 --- a/ProtocolLib/.classpath +++ b/ProtocolLib/.classpath @@ -10,5 +10,6 @@ + diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ListenerInvoker.java b/ProtocolLib/src/com/comphenix/protocol/injector/ListenerInvoker.java new file mode 100644 index 00000000..e5bc0e4b --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/injector/ListenerInvoker.java @@ -0,0 +1,27 @@ +package com.comphenix.protocol.injector; + +import net.minecraft.server.Packet; + +import com.comphenix.protocol.events.PacketEvent; + +public interface ListenerInvoker { + + /** + * Invokes the given packet event for every registered listener. + * @param event - the packet event to invoke. + */ + public abstract void invokePacketRecieving(PacketEvent event); + + /** + * Invokes the given packet event for every registered listener. + * @param event - the packet event to invoke. + */ + public abstract void invokePacketSending(PacketEvent event); + + /** + * Retrieve the associated ID of a packet. + * @param packet - the packet. + * @return The packet ID. + */ + public abstract int getPacketID(Packet packet); +} \ No newline at end of file diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index 68333b7d..3d4c4c0e 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -17,13 +17,10 @@ package com.comphenix.protocol.injector; -import java.io.DataInputStream; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; @@ -51,12 +48,13 @@ import com.comphenix.protocol.ProtocolManager; import com.comphenix.protocol.async.AsyncFilterManager; import com.comphenix.protocol.async.AsyncMarker; import com.comphenix.protocol.events.*; +import com.comphenix.protocol.injector.player.PlayerInjectionHandler; import com.comphenix.protocol.reflect.FieldAccessException; import com.comphenix.protocol.reflect.FuzzyReflection; import com.google.common.base.Objects; import com.google.common.collect.ImmutableSet; -public final class PacketFilterManager implements ProtocolManager { +public final class PacketFilterManager implements ProtocolManager, ListenerInvoker { /** * Sets the inject hook type. Different types allow for maximum compatibility. @@ -86,22 +84,12 @@ public final class PacketFilterManager implements ProtocolManager { // Create a concurrent set private Set packetListeners = Collections.newSetFromMap(new ConcurrentHashMap()); - - // Player injection - private Map connectionLookup = new ConcurrentHashMap(); - private Map playerInjection = new HashMap(); - - // Player injection type - private PlayerInjectHooks playerHook = PlayerInjectHooks.NETWORK_SERVER_OBJECT; - + // Packet injection private PacketInjector packetInjector; - // Server connection injection - private InjectedServerConnection serverInjection; - - // Enabled packet filters - private Set sendingFilters = Collections.newSetFromMap(new ConcurrentHashMap()); + // Player injection + private PlayerInjectionHandler playerInjection; // The two listener containers private SortedPacketListenerList recievedListeners = new SortedPacketListenerList(); @@ -112,10 +100,7 @@ public final class PacketFilterManager implements ProtocolManager { // The default class loader private ClassLoader classLoader; - - // The last successful player hook - private PlayerInjector lastSuccessfulHook; - + // Error logger private Logger logger; @@ -135,9 +120,9 @@ public final class PacketFilterManager implements ProtocolManager { // Initialize values this.classLoader = classLoader; this.logger = logger; - this.packetInjector = new PacketInjector(classLoader, this, connectionLookup); + this.playerInjection = new PlayerInjectionHandler(classLoader, logger, this, server); + this.packetInjector = new PacketInjector(classLoader, this, playerInjection); this.asyncFilterManager = new AsyncFilterManager(logger, server.getScheduler(), this); - this.serverInjection = new InjectedServerConnection(logger, server); } catch (IllegalAccessException e) { logger.log(Level.SEVERE, "Unable to initialize packet injector.", e); } @@ -153,7 +138,7 @@ public final class PacketFilterManager implements ProtocolManager { * @return Injection method for reading server packets. */ public PlayerInjectHooks getPlayerHook() { - return playerHook; + return playerInjection.getPlayerHook(); } /** @@ -161,18 +146,10 @@ public final class PacketFilterManager implements ProtocolManager { * @param playerHook - the new injection method for reading server packets. */ public void setPlayerHook(PlayerInjectHooks playerHook) { - this.playerHook = playerHook; + playerInjection.setPlayerHook(playerHook); // Make sure the current listeners are compatible - if (lastSuccessfulHook != null) { - for (PacketListener listener : packetListeners) { - try { - checkListener(listener); - } catch (IllegalStateException e) { - logger.log(Level.WARNING, "Unsupported listener.", e); - } - } - } + playerInjection.checkListener(packetListeners); } public Logger getLogger() { @@ -204,14 +181,14 @@ public final class PacketFilterManager implements ProtocolManager { verifyWhitelist(listener, sending); sendingListeners.addListener(listener, sending); enablePacketFilters(ConnectionSide.SERVER_SIDE, sending.getWhitelist()); + + // Make sure this is possible + playerInjection.checkListener(listener); } if (hasReceiving) { verifyWhitelist(listener, receiving); recievedListeners.addListener(listener, receiving); enablePacketFilters(ConnectionSide.CLIENT_SIDE, receiving.getWhitelist()); - - // We don't know if we've hooked any players yet - checkListener(listener); } // Inform our injected hooks @@ -234,21 +211,7 @@ public final class PacketFilterManager implements ProtocolManager { } } } - - /** - * Determine if a listener is valid or not. - * @param listener - listener to check. - * @throws IllegalStateException If the given listener's whitelist cannot be fulfilled. - */ - public void checkListener(PacketListener listener) { - try { - if (lastSuccessfulHook != null) - lastSuccessfulHook.checkListener(listener); - } catch (Exception e) { - throw new IllegalStateException("Registering listener " + PacketAdapter.getPluginName(listener) + " failed", e); - } - } - + @Override public void removePacketListener(PacketListener listener) { if (listener == null) @@ -292,18 +255,12 @@ public final class PacketFilterManager implements ProtocolManager { asyncFilterManager.unregisterAsyncHandlers(plugin); } - /** - * Invokes the given packet event for every registered listener. - * @param event - the packet event to invoke. - */ + @Override public void invokePacketRecieving(PacketEvent event) { handlePacket(recievedListeners, event, false); } - - /** - * Invokes the given packet event for every registered listener. - * @param event - the packet event to invoke. - */ + + @Override public void invokePacketSending(PacketEvent event) { handlePacket(sendingListeners, event, true); } @@ -356,8 +313,8 @@ public final class PacketFilterManager implements ProtocolManager { for (int packetID : packets) { if (side.isForServer()) - sendingFilters.add(packetID); - if (side.isForClient() && packetInjector != null) + playerInjection.addPacketHandler(packetID); + if (side.isForClient() && packetInjector != null) packetInjector.addPacketHandler(packetID); } } @@ -373,7 +330,7 @@ public final class PacketFilterManager implements ProtocolManager { for (int packetID : packets) { if (side.isForServer()) - sendingFilters.remove(packetID); + playerInjection.removePacketHandler(packetID); if (side.isForClient() && packetInjector != null) packetInjector.removePacketHandler(packetID); } @@ -391,7 +348,7 @@ public final class PacketFilterManager implements ProtocolManager { if (packet == null) throw new IllegalArgumentException("packet cannot be NULL."); - getInjector(reciever).sendServerPacket(packet.getHandle(), filters); + playerInjection.sendServerPacket(reciever, packet, filters); } @Override @@ -407,17 +364,21 @@ public final class PacketFilterManager implements ProtocolManager { if (packet == null) throw new IllegalArgumentException("packet cannot be NULL."); - PlayerInjector injector = getInjector(sender); Packet mcPacket = packet.getHandle(); // Make sure the packet isn't cancelled packetInjector.undoCancel(packet.getID(), mcPacket); if (filters) { - mcPacket = injector.handlePacketRecieved(mcPacket); + PacketEvent event = packetInjector.packetRecieved(packet, sender); + + if (!event.isCancelled()) + mcPacket = event.getPacket().getHandle(); + else + return; } - injector.processPacket(mcPacket); + playerInjection.processPacket(sender, mcPacket); } @Override @@ -448,7 +409,7 @@ public final class PacketFilterManager implements ProtocolManager { @Override public Set getSendingFilters() { - return ImmutableSet.copyOf(sendingFilters); + return playerInjection.getSendingFilters(); } @Override @@ -467,94 +428,9 @@ public final class PacketFilterManager implements ProtocolManager { */ public void initializePlayers(Player[] players) { for (Player player : players) - injectPlayer(player); + playerInjection.injectPlayer(player); } - - /** - * Used to construct a player hook. - * @param player - the player to hook. - * @param hook - the hook type. - * @return A new player hoook - * @throws IllegalAccessException Unable to do our reflection magic. - */ - protected PlayerInjector getHookInstance(Player player, PlayerInjectHooks hook) throws IllegalAccessException { - // Construct the correct player hook - switch (hook) { - case NETWORK_HANDLER_FIELDS: - return new NetworkFieldInjector(player, this, sendingFilters); - case NETWORK_MANAGER_OBJECT: - return new NetworkObjectInjector(player, this, sendingFilters); - case NETWORK_SERVER_OBJECT: - return new NetworkServerInjector(player, this, sendingFilters, serverInjection); - default: - throw new IllegalArgumentException("Cannot construct a player injector."); - } - } - - /** - * Initialize a player hook, allowing us to read server packets. - * @param player - player to hook. - */ - protected void injectPlayer(Player player) { - PlayerInjector injector = null; - PlayerInjectHooks currentHook = playerHook; - boolean firstPlayer = lastSuccessfulHook == null; - - // Don't inject if the class has closed - if (!hasClosed && player != null && !playerInjection.containsKey(player)) { - while (true) { - try { - injector = getHookInstance(player, currentHook); - injector.injectManager(); - - DataInputStream inputStream = injector.getInputStream(false); - - if (!player.isOnline() || inputStream == null) { - throw new PlayerLoggedOutException(); - } - - playerInjection.put(player, injector); - connectionLookup.put(inputStream, player); - break; - - - } catch (PlayerLoggedOutException e) { - throw e; - - } catch (Exception e) { - - // Mark this injection attempt as a failure - logger.log(Level.SEVERE, "Player hook " + currentHook.toString() + " failed.", e); - - // Clean up as much as possible - try { - if (injector != null) - injector.cleanupAll(); - } catch (Exception e2) { - logger.log(Level.WARNING, "Cleaing up after player hook failed.", e); - } - - if (currentHook.ordinal() > 0) { - // Choose the previous player hook type - currentHook = PlayerInjectHooks.values()[currentHook.ordinal() - 1]; - logger.log(Level.INFO, "Switching to " + currentHook.toString() + " instead."); - } else { - // UTTER FAILURE - playerInjection.put(player, null); - return; - } - } - } - - // Update values - if (injector != null) - lastSuccessfulHook = injector; - if (currentHook != playerHook || firstPlayer) - setPlayerHook(currentHook); - } - } - /** * Register this protocol manager on Bukkit. * @param manager - Bukkit plugin manager that provides player join/leave events. @@ -567,12 +443,12 @@ public final class PacketFilterManager implements ProtocolManager { @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onPlayerJoin(PlayerJoinEvent event) { - injectPlayer(event.getPlayer()); + playerInjection.injectPlayer(event.getPlayer()); } @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) public void onPlayerQuit(PlayerQuitEvent event) { - uninjectPlayer(event.getPlayer()); + playerInjection.uninjectPlayer(event.getPlayer()); } @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) @@ -591,6 +467,14 @@ public final class PacketFilterManager implements ProtocolManager { } } + @Override + public int getPacketID(Packet packet) { + if (packet == null) + throw new IllegalArgumentException("Packet cannot be NULL."); + + return MinecraftRegistry.getPacketToID().get(packet.getClass()); + } + // Yes, this is crazy. @SuppressWarnings({ "unchecked", "rawtypes" }) private void registerOld(PluginManager manager, Plugin plugin) { @@ -632,9 +516,9 @@ public final class PacketFilterManager implements ProtocolManager { // Check for the correct event if (event instanceof PlayerJoinEvent) - injectPlayer(((PlayerJoinEvent) event).getPlayer()); + playerInjection.injectPlayer(((PlayerJoinEvent) event).getPlayer()); else if (event instanceof PlayerQuitEvent) - uninjectPlayer(((PlayerQuitEvent) event).getPlayer()); + playerInjection.uninjectPlayer(((PlayerQuitEvent) event).getPlayer()); } return null; } @@ -676,37 +560,7 @@ public final class PacketFilterManager implements ProtocolManager { e.printStackTrace(); } } - - private void uninjectPlayer(Player player) { - if (!hasClosed && player != null) { - - PlayerInjector injector = playerInjection.get(player); - - if (injector != null) { - DataInputStream input = injector.getInputStream(true); - injector.cleanupAll(); - - playerInjection.remove(player); - connectionLookup.remove(input); - } - } - } - - private PlayerInjector getInjector(Player player) { - if (!playerInjection.containsKey(player)) { - // What? Try to inject again. - injectPlayer(player); - } - PlayerInjector injector = playerInjection.get(player); - - // Check that the injector was sucessfully added - if (injector != null) - return injector; - else - throw new IllegalArgumentException("Player has no injected handler."); - } - /** * Retrieves the current plugin class loader. * @return Class loader. @@ -722,28 +576,19 @@ public final class PacketFilterManager implements ProtocolManager { public void close() { // Guard - if (hasClosed || playerInjection == null) + if (hasClosed) return; - // Remove everything - for (PlayerInjector injection : playerInjection.values()) { - if (injection != null) { - injection.cleanupAll(); - } - } - // Remove packet handlers if (packetInjector != null) packetInjector.cleanupAll(); // Remove server handler - serverInjection.cleanupAll(); + playerInjection.close(); hasClosed = true; // Remove listeners packetListeners.clear(); - playerInjection.clear(); - connectionLookup.clear(); // Clean up async handlers. We have to do this last. asyncFilterManager.cleanupAll(); diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java index 998eb5a7..9e042885 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketInjector.java @@ -33,6 +33,7 @@ import net.sf.cglib.proxy.Enhancer; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; +import com.comphenix.protocol.injector.player.PlayerInjectionHandler; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; @@ -48,10 +49,10 @@ class PacketInjector { private static Object intHashMap; // The packet filter manager - private PacketFilterManager manager; + private ListenerInvoker manager; // Allows us to determine the sender - private Map playerLookup; + private PlayerInjectionHandler playerInjection; // Allows us to look up read packet injectors private Map readModifier; @@ -59,12 +60,12 @@ class PacketInjector { // Class loader private ClassLoader classLoader; - public PacketInjector(ClassLoader classLoader, PacketFilterManager manager, - Map playerLookup) throws IllegalAccessException { + public PacketInjector(ClassLoader classLoader, ListenerInvoker manager, + PlayerInjectionHandler playerInjection) throws IllegalAccessException { this.classLoader = classLoader; this.manager = manager; - this.playerLookup = playerLookup; + this.playerInjection = playerInjection; this.readModifier = new ConcurrentHashMap(); initialize(); } @@ -194,7 +195,18 @@ class PacketInjector { // Called from the ReadPacketModified monitor PacketEvent packetRecieved(PacketContainer packet, DataInputStream input) { - Player client = playerLookup.get(input); + Player client = playerInjection.getPlayerByConnection(input); + return packetRecieved(packet, client); + } + + /** + * Let the packet listeners process the given packet. + * @param packet - a packet to process. + * @param client - the client that sent the packet. + * @return The resulting packet event. + */ + public PacketEvent packetRecieved(PacketContainer packet, Player client) { + PacketEvent event = PacketEvent.fromClient((Object) manager, packet, client); manager.invokePacketRecieving(event); diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedArrayList.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/InjectedArrayList.java similarity index 93% rename from ProtocolLib/src/com/comphenix/protocol/injector/InjectedArrayList.java rename to ProtocolLib/src/com/comphenix/protocol/injector/player/InjectedArrayList.java index 5273ce57..4562d2c5 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedArrayList.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/InjectedArrayList.java @@ -1,17 +1,17 @@ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.injector.player; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Set; +import com.comphenix.protocol.injector.player.NetworkFieldInjector.FakePacket; + import net.minecraft.server.Packet; import net.sf.cglib.proxy.Enhancer; import net.sf.cglib.proxy.MethodInterceptor; import net.sf.cglib.proxy.MethodProxy; -import com.comphenix.protocol.injector.NetworkFieldInjector.FakePacket; - /** * The array list that notifies when packets are sent by the server. * diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/InjectedServerConnection.java similarity index 95% rename from ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java rename to ProtocolLib/src/com/comphenix/protocol/injector/player/InjectedServerConnection.java index ea987263..4ed76815 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/InjectedServerConnection.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/InjectedServerConnection.java @@ -1,4 +1,4 @@ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.injector.player; import java.lang.reflect.Field; import java.lang.reflect.Method; diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkFieldInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkFieldInjector.java similarity index 79% rename from ProtocolLib/src/com/comphenix/protocol/injector/NetworkFieldInjector.java rename to ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkFieldInjector.java index 56903e35..35395725 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkFieldInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkFieldInjector.java @@ -1,4 +1,4 @@ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.injector.player; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; @@ -7,12 +7,14 @@ import java.util.Collections; import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Logger; import org.bukkit.entity.Player; import com.comphenix.protocol.Packets; import com.comphenix.protocol.events.ListeningWhitelist; import com.comphenix.protocol.events.PacketListener; +import com.comphenix.protocol.injector.ListenerInvoker; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.StructureModifier; @@ -45,13 +47,28 @@ class NetworkFieldInjector extends PlayerInjector { // Sync field private static Field syncField; private Object syncObject; + + // Determine if we're listening + private Set sendingFilters; + + // Used to construct proxy objects + private ClassLoader classLoader; - public NetworkFieldInjector(Player player, PacketFilterManager manager, Set sendingFilters) throws IllegalAccessException { - super(player, manager, sendingFilters); + public NetworkFieldInjector(ClassLoader classLoader, Logger logger, Player player, + ListenerInvoker manager, Set sendingFilters) throws IllegalAccessException { + + super(logger, player, manager); + this.classLoader = classLoader; + this.sendingFilters = sendingFilters; } @Override - protected synchronized void initialize() throws IllegalAccessException { + protected boolean hasListener(int packetID) { + return sendingFilters.contains(packetID); + } + + @Override + public synchronized void initialize() throws IllegalAccessException { super.initialize(); // Get the sync field as well @@ -112,7 +129,7 @@ class NetworkFieldInjector extends PlayerInjector { synchronized(syncObject) { // The list we'll be inserting - List hackedList = new InjectedArrayList(manager.getClassLoader(), this, ignoredPackets); + List hackedList = new InjectedArrayList(classLoader, this, ignoredPackets); // Add every previously stored packet for (Packet packet : minecraftList) { @@ -154,4 +171,9 @@ class NetworkFieldInjector extends PlayerInjector { } overridenLists.clear(); } + + @Override + public boolean canInject() { + return true; + } } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkObjectInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkObjectInjector.java similarity index 81% rename from ProtocolLib/src/com/comphenix/protocol/injector/NetworkObjectInjector.java rename to ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkObjectInjector.java index 9091390d..6d6bba12 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkObjectInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkObjectInjector.java @@ -1,4 +1,4 @@ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.injector.player; import java.lang.reflect.InvocationTargetException; @@ -8,12 +8,14 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.Proxy; import java.lang.reflect.Method; import java.util.Set; +import java.util.logging.Logger; import org.bukkit.entity.Player; import com.comphenix.protocol.Packets; import com.comphenix.protocol.events.ListeningWhitelist; import com.comphenix.protocol.events.PacketListener; +import com.comphenix.protocol.injector.ListenerInvoker; /** * Injection method that overrides the NetworkHandler itself, and it's sendPacket-method. @@ -21,10 +23,19 @@ import com.comphenix.protocol.events.PacketListener; * @author Kristian */ class NetworkObjectInjector extends PlayerInjector { - public NetworkObjectInjector(Player player, PacketFilterManager manager, Set sendingFilters) throws IllegalAccessException { - super(player, manager, sendingFilters); + // Determine if we're listening + private Set sendingFilters; + + public NetworkObjectInjector(Logger logger, Player player, ListenerInvoker invoker, Set sendingFilters) throws IllegalAccessException { + super(logger, player, invoker); + this.sendingFilters = sendingFilters; } + @Override + protected boolean hasListener(int packetID) { + return sendingFilters.contains(packetID); + } + @Override public void sendServerPacket(Packet packet, boolean filtered) throws InvocationTargetException { Object networkDelegate = filtered ? networkManagerRef.getValue() : networkManagerRef.getOldValue(); @@ -107,4 +118,9 @@ class NetworkObjectInjector extends PlayerInjector { // Clean up networkManagerRef.revertValue(); } + + @Override + public boolean canInject() { + return true; + } } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java similarity index 81% rename from ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java rename to ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java index 50f311ad..243c0fc7 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java @@ -1,8 +1,9 @@ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.injector.player; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Set; +import java.util.logging.Logger; import net.minecraft.server.Packet; import net.sf.cglib.proxy.Callback; @@ -16,6 +17,7 @@ import net.sf.cglib.proxy.NoOp; import org.bukkit.entity.Player; import com.comphenix.protocol.events.PacketListener; +import com.comphenix.protocol.injector.ListenerInvoker; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.ObjectCloner; @@ -32,14 +34,30 @@ public class NetworkServerInjector extends PlayerInjector { private static Method sendPacketMethod; private InjectedServerConnection serverInjection; - public NetworkServerInjector(Player player, PacketFilterManager manager, - Set sendingFilters, InjectedServerConnection serverInjection) throws IllegalAccessException { - super(player, manager, sendingFilters); + // Determine if we're listening + private Set sendingFilters; + + // Used to create proxy objects + private ClassLoader classLoader; + + public NetworkServerInjector( + ClassLoader classLoader, Logger logger, Player player, + ListenerInvoker invoker, Set sendingFilters, + InjectedServerConnection serverInjection) throws IllegalAccessException { + + super(logger, player, invoker); + this.classLoader = classLoader; + this.sendingFilters = sendingFilters; this.serverInjection = serverInjection; } @Override - protected void initialize() throws IllegalAccessException { + protected boolean hasListener(int packetID) { + return sendingFilters.contains(packetID); + } + + @Override + public void initialize() throws IllegalAccessException { super.initialize(); // Get the send packet method! @@ -104,7 +122,7 @@ public class NetworkServerInjector extends PlayerInjector { }; Callback noOpCallback = NoOp.INSTANCE; - ex.setClassLoader(manager.getClassLoader()); + ex.setClassLoader(classLoader); ex.setSuperclass(serverClass); ex.setCallbacks(new Callback[] { sendPacketCallback, noOpCallback }); ex.setCallbackFilter(new CallbackFilter() { @@ -165,4 +183,9 @@ public class NetworkServerInjector extends PlayerInjector { public void checkListener(PacketListener listener) { // We support everything } + + @Override + public boolean canInject() { + return true; + } } diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java new file mode 100644 index 00000000..e71bacef --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java @@ -0,0 +1,60 @@ +package com.comphenix.protocol.injector.player; + +import java.lang.reflect.InvocationTargetException; +import java.util.logging.Logger; + +import net.minecraft.server.Packet; + +import org.bukkit.Bukkit; +import org.bukkit.entity.Player; +import org.bukkit.plugin.Plugin; + +import com.comphenix.protocol.events.PacketListener; +import com.comphenix.protocol.injector.ListenerInvoker; + +public class NetworkSpoutHook extends PlayerInjector { + + public NetworkSpoutHook(Logger logger, Player player, ListenerInvoker invoker) throws IllegalAccessException { + super(logger, player, invoker); + } + + @Override + protected boolean hasListener(int packetID) { + return false; + } + + @Override + public boolean canInject() { + return getSpout() != null; + } + + private Plugin getSpout() { + // Spout must be loaded + try { + return Bukkit.getServer().getPluginManager().getPlugin("Spout"); + } catch (Throwable e) { + return null; + } + } + + @Override + public void injectManager() { + + } + + @Override + public void sendServerPacket(Packet packet, boolean filtered) throws InvocationTargetException { + + } + + @Override + public void cleanupAll() { + // TODO Auto-generated method stub + + } + + @Override + public void checkListener(PacketListener listener) { + // We support everything Spout does + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java new file mode 100644 index 00000000..1ceb0725 --- /dev/null +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java @@ -0,0 +1,306 @@ +package com.comphenix.protocol.injector.player; + +import java.io.DataInputStream; +import java.lang.reflect.InvocationTargetException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Level; +import java.util.logging.Logger; + +import net.minecraft.server.Packet; + +import org.bukkit.Server; +import org.bukkit.entity.Player; + +import com.comphenix.protocol.events.PacketAdapter; +import com.comphenix.protocol.events.PacketContainer; +import com.comphenix.protocol.events.PacketListener; +import com.comphenix.protocol.injector.ListenerInvoker; +import com.comphenix.protocol.injector.PlayerLoggedOutException; +import com.comphenix.protocol.injector.PacketFilterManager.PlayerInjectHooks; +import com.google.common.collect.ImmutableSet; + +public class PlayerInjectionHandler { + + // Server connection injection + private InjectedServerConnection serverInjection; + + // The last successful player hook + private PlayerInjector lastSuccessfulHook; + + // Player injection + private Map connectionLookup = new ConcurrentHashMap(); + private Map playerInjection = new HashMap(); + + // Player injection type + private PlayerInjectHooks playerHook = PlayerInjectHooks.NETWORK_SERVER_OBJECT; + + // Error logger + private Logger logger; + + // Whether or not we're closing + private boolean hasClosed; + + // Used to invoke events + private ListenerInvoker invoker; + + // Enabled packet filters + private Set sendingFilters = Collections.newSetFromMap(new ConcurrentHashMap()); + + // The class loader we're using + private ClassLoader classLoader; + + public PlayerInjectionHandler(ClassLoader classLoader, Logger logger, ListenerInvoker invoker, Server server) { + this.classLoader = classLoader; + this.logger = logger; + this.invoker = invoker; + this.serverInjection = new InjectedServerConnection(logger, server); + } + + /** + * Retrieves how the server packets are read. + * @return Injection method for reading server packets. + */ + public PlayerInjectHooks getPlayerHook() { + return playerHook; + } + + /** + * Add an underlying packet handler of the given ID. + * @param packetID - packet ID to register. + */ + public void addPacketHandler(int packetID) { + sendingFilters.add(packetID); + } + + /** + * Remove an underlying packet handler of ths ID. + * @param packetID - packet ID to unregister. + */ + public void removePacketHandler(int packetID) { + sendingFilters.remove(packetID); + } + + /** + * Sets how the server packets are read. + * @param playerHook - the new injection method for reading server packets. + */ + public void setPlayerHook(PlayerInjectHooks playerHook) { + this.playerHook = playerHook; + } + + /** + * Used to construct a player hook. + * @param player - the player to hook. + * @param hook - the hook type. + * @return A new player hoook + * @throws IllegalAccessException Unable to do our reflection magic. + */ + private PlayerInjector getHookInstance(Player player, PlayerInjectHooks hook) throws IllegalAccessException { + // Construct the correct player hook + switch (hook) { + case NETWORK_HANDLER_FIELDS: + return new NetworkFieldInjector(classLoader, logger, player, invoker, sendingFilters); + case NETWORK_MANAGER_OBJECT: + return new NetworkObjectInjector(logger, player, invoker, sendingFilters); + case NETWORK_SERVER_OBJECT: + return new NetworkServerInjector(classLoader, logger, player, invoker, sendingFilters, serverInjection); + default: + throw new IllegalArgumentException("Cannot construct a player injector."); + } + } + + public Player getPlayerByConnection(DataInputStream inputStream) { + return connectionLookup.get(inputStream); + } + + /** + * Initialize a player hook, allowing us to read server packets. + * @param manager - the main packet filter manager. + * @param player - player to hook. + */ + public void injectPlayer(Player player) { + + PlayerInjector injector = null; + PlayerInjectHooks currentHook = playerHook; + boolean firstPlayer = lastSuccessfulHook == null; + + // Don't inject if the class has closed + if (!hasClosed && player != null && !playerInjection.containsKey(player)) { + while (true) { + try { + injector = getHookInstance(player, currentHook); + injector.initialize(); + injector.injectManager(); + + DataInputStream inputStream = injector.getInputStream(false); + + if (!player.isOnline() || inputStream == null) { + throw new PlayerLoggedOutException(); + } + + playerInjection.put(player, injector); + connectionLookup.put(inputStream, player); + break; + + + } catch (PlayerLoggedOutException e) { + throw e; + + } catch (Exception e) { + + // Mark this injection attempt as a failure + logger.log(Level.SEVERE, "Player hook " + currentHook.toString() + " failed.", e); + + // Clean up as much as possible + try { + if (injector != null) + injector.cleanupAll(); + } catch (Exception e2) { + logger.log(Level.WARNING, "Cleaing up after player hook failed.", e); + } + + if (currentHook.ordinal() > 0) { + + // Choose the previous player hook type + currentHook = PlayerInjectHooks.values()[currentHook.ordinal() - 1]; + logger.log(Level.INFO, "Switching to " + currentHook.toString() + " instead."); + } else { + // UTTER FAILURE + playerInjection.put(player, null); + return; + } + } + } + + // Update values + if (injector != null) + lastSuccessfulHook = injector; + if (currentHook != playerHook || firstPlayer) + setPlayerHook(currentHook); + } + } + + /** + * Unregisters the given player. + * @param player - player to unregister. + */ + public void uninjectPlayer(Player player) { + if (!hasClosed && player != null) { + + PlayerInjector injector = playerInjection.get(player); + + if (injector != null) { + DataInputStream input = injector.getInputStream(true); + injector.cleanupAll(); + + playerInjection.remove(player); + connectionLookup.remove(input); + } + } + } + + public void sendServerPacket(Player reciever, PacketContainer packet, boolean filters) throws InvocationTargetException { + getInjector(reciever).sendServerPacket(packet.getHandle(), filters); + } + + private PlayerInjector getInjector(Player player) { + if (!playerInjection.containsKey(player)) { + // What? Try to inject again. + injectPlayer(player); + } + + PlayerInjector injector = playerInjection.get(player); + + // Check that the injector was sucessfully added + if (injector != null) + return injector; + else + throw new IllegalArgumentException("Player has no injected handler."); + } + + /** + * Determine if the given listeners are valid. + * @param listeners - listeners to check. + */ + public void checkListener(Set listeners) { + // Make sure the current listeners are compatible + if (lastSuccessfulHook != null) { + for (PacketListener listener : listeners) { + try { + checkListener(listener); + } catch (IllegalStateException e) { + logger.log(Level.WARNING, "Unsupported listener.", e); + } + } + } + } + + /** + * Determine if a listener is valid or not. + * @param listener - listener to check. + * @throws IllegalStateException If the given listener's whitelist cannot be fulfilled. + */ + public void checkListener(PacketListener listener) { + try { + if (lastSuccessfulHook != null) + lastSuccessfulHook.checkListener(listener); + } catch (Exception e) { + throw new IllegalStateException("Registering listener " + PacketAdapter.getPluginName(listener) + " failed", e); + } + } + + /** + * Process a packet as if it were sent by the given player. + * @param player - the sender. + * @param mcPacket - the packet to process. + * @throws IllegalAccessException If the reflection machinery failed. + * @throws InvocationTargetException If the underlying method caused an error. + */ + public void processPacket(Player player, Packet mcPacket) throws IllegalAccessException, InvocationTargetException { + + PlayerInjector injector = getInjector(player); + injector.processPacket(mcPacket); + } + + /** + * Retrieve the current list of registered sending listeners. + * @return List of the sending listeners's packet IDs. + */ + public Set getSendingFilters() { + return ImmutableSet.copyOf(sendingFilters); + } + + /** + * Retrieve the current logger. + * @return Error logger. + */ + public Logger getLogger() { + return logger; + } + + public void close() { + + // Guard + if (hasClosed || playerInjection == null) + return; + + // Remove everything + for (PlayerInjector injection : playerInjection.values()) { + if (injection != null) { + injection.cleanupAll(); + } + } + + // Remove server handler + serverInjection.cleanupAll(); + hasClosed = true; + + playerInjection.clear(); + connectionLookup.clear(); + invoker = null; + } +} diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java similarity index 84% rename from ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java rename to ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java index 7d88e25f..9a1779ec 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PlayerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java @@ -15,14 +15,14 @@ * 02111-1307 USA */ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.injector.player; import java.io.DataInputStream; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.Set; import java.util.logging.Level; +import java.util.logging.Logger; import net.minecraft.server.EntityPlayer; import net.minecraft.server.Packet; @@ -34,6 +34,7 @@ import org.bukkit.entity.Player; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.events.PacketListener; +import com.comphenix.protocol.injector.ListenerInvoker; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.StructureModifier; @@ -69,17 +70,18 @@ abstract class PlayerInjector { protected Object netHandler; // The packet manager and filters - protected PacketFilterManager manager; - protected Set sendingFilters; + protected ListenerInvoker invoker; // Previous data input protected DataInputStream cachedInput; + + // Handle errors + protected Logger logger; - public PlayerInjector(Player player, PacketFilterManager manager, Set sendingFilters) throws IllegalAccessException { + public PlayerInjector(Logger logger, Player player, ListenerInvoker invoker) throws IllegalAccessException { + this.logger = logger; this.player = player; - this.manager = manager; - this.sendingFilters = sendingFilters; - initialize(); + this.invoker = invoker; } /** @@ -91,7 +93,11 @@ abstract class PlayerInjector { return craft.getHandle(); } - protected void initialize() throws IllegalAccessException { + /** + * Initialize all fields for this player injector, if it hasn't already. + * @throws IllegalAccessException An error has occured. + */ + public void initialize() throws IllegalAccessException { EntityPlayer notchEntity = getEntityPlayer(); @@ -156,12 +162,12 @@ abstract class PlayerInjector { return reflection.getFieldByType(".*NetServerHandler"); } catch (RuntimeException e) { - manager.getLogger().log(Level.WARNING, "Server handler is a proxy type.", e); + logger.log(Level.WARNING, "Server handler is a proxy type.", e); } } } catch (IllegalAccessException e) { - manager.getLogger().warning("Unable to load server handler from proxy type."); + logger.warning("Unable to load server handler from proxy type."); } // Nope, just go with it @@ -250,6 +256,12 @@ abstract class PlayerInjector { */ public abstract void cleanupAll(); + /** + * Determine if this inject method can even be attempted. + * @return TRUE if can be attempted, though possibly with failure, FALSE otherwise. + */ + public abstract boolean canInject(); + /** * Invoked before a new listener is registered. *

@@ -263,16 +275,16 @@ abstract class PlayerInjector { * @param packet - packet to recieve. * @return The given packet, or the packet replaced by the listeners. */ - Packet handlePacketRecieved(Packet packet) { + public Packet handlePacketRecieved(Packet packet) { // Get the packet ID too - Integer id = MinecraftRegistry.getPacketToID().get(packet.getClass()); + Integer id = invoker.getPacketID(packet); // Make sure we're listening - if (id != null && sendingFilters.contains(id)) { + if (id != null && hasListener(id)) { // A packet has been sent guys! PacketContainer container = new PacketContainer(id, packet); - PacketEvent event = PacketEvent.fromServer(manager, container, player); - manager.invokePacketSending(event); + PacketEvent event = PacketEvent.fromServer(invoker, container, player); + invoker.invokePacketSending(event); // Cancelling is pretty simple. Just ignore the packet. if (event.isCancelled()) @@ -285,6 +297,13 @@ abstract class PlayerInjector { return packet; } + /** + * Determine if the given injector is listening for this packet ID. + * @param packetID - packet ID to check. + * @return TRUE if it is, FALSE oterhwise. + */ + protected abstract boolean hasListener(int packetID); + /** * Retrieve the current player's input stream. * @param cache - whether or not to cache the result of this method. diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/ReplacedArrayList.java similarity index 94% rename from ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java rename to ProtocolLib/src/com/comphenix/protocol/injector/player/ReplacedArrayList.java index 2923785c..ed239f3c 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/ReplacedArrayList.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/ReplacedArrayList.java @@ -1,4 +1,4 @@ -package com.comphenix.protocol.injector; +package com.comphenix.protocol.injector.player; import java.util.Collection; import java.util.List; From 18ef06ea21b2399fc61b1e55853a1a72ebfbc48a Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 5 Oct 2012 01:41:17 +0200 Subject: [PATCH 21/23] Adding "support" for Spout by proxying it's NetServerHandler. While it may seem better to use a Spout PacketListener, we can't prevent other spout listeners from stopping the listener chain. For instance, Orebfuscator does supports Spout, but does this by cancelling every chunk packet and sending them to be processed and sent by Orebfuscator only. This can never be made compatible with other plugins. So, we choose to add some overhead and inject our proxy onto Spout. Fortunately, Spout injects the proxy using a player listener on LOWEST, so we get to override Spout again with our HIGHEST. The proxy method should be generic enough to handle most proxy types. --- ProtocolLib/.classpath | 1 - .../injector/PacketFilterManager.java | 8 +-- .../player/NetworkServerInjector.java | 20 ++++++- .../injector/player/NetworkSpoutHook.java | 60 ------------------- .../player/PlayerInjectionHandler.java | 5 ++ .../injector/player/PlayerInjector.java | 15 ++++- .../protocol/reflect/ObjectCloner.java | 8 +++ .../reflect/instances/DefaultInstances.java | 4 +- .../reflect/instances/ExistingGenerator.java | 28 ++++++++- 9 files changed, 78 insertions(+), 71 deletions(-) delete mode 100644 ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java diff --git a/ProtocolLib/.classpath b/ProtocolLib/.classpath index 408271fe..0bc481ca 100644 --- a/ProtocolLib/.classpath +++ b/ProtocolLib/.classpath @@ -10,6 +10,5 @@ - diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java index 3d4c4c0e..7a1c612d 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/PacketFilterManager.java @@ -441,17 +441,17 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok try { manager.registerEvents(new Listener() { - @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) public void onPlayerJoin(PlayerJoinEvent event) { playerInjection.injectPlayer(event.getPlayer()); } - @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) public void onPlayerQuit(PlayerQuitEvent event) { playerInjection.uninjectPlayer(event.getPlayer()); } - @EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true) + @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true) public void onPluginDisabled(PluginDisableEvent event) { // Clean up in case the plugin forgets if (event.getPlugin() != plugin) { @@ -487,7 +487,7 @@ public final class PacketFilterManager implements ProtocolManager, ListenerInvok Class eventPriority = loader.loadClass("org.bukkit.event.Event$Priority"); // Get the priority - Object priorityNormal = Enum.valueOf(eventPriority, "Normal"); + Object priorityNormal = Enum.valueOf(eventPriority, "Highest"); // Get event types Object playerJoinType = Enum.valueOf(eventTypes, "PLAYER_JOIN"); diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java index 243c0fc7..55ebd698 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java @@ -136,8 +136,17 @@ public class NetworkServerInjector extends PlayerInjector { }); // Use the existing field values when we create our copy - DefaultInstances serverInstances = DefaultInstances.fromArray( + DefaultInstances serverInstances = null; + + if (hasProxyServerHandler()) { + Class minecraftSuperClass = getFirstMinecraftSuperClass(serverHandler.getClass()); + serverInstances = DefaultInstances.fromArray( + ExistingGenerator.fromObjectFields(serverHandler, minecraftSuperClass)); + } else { + serverInstances = DefaultInstances.fromArray( ExistingGenerator.fromObjectFields(serverHandler)); + } + serverInstances.setNonNull(true); serverInstances.setMaximumRecursion(1); @@ -154,6 +163,15 @@ public class NetworkServerInjector extends PlayerInjector { "Cannot hook player: Unable to find a valid constructor for the NetServerHandler object."); } } + + private Class getFirstMinecraftSuperClass(Class clazz) { + if (clazz.getName().startsWith("net.minecraft")) + return clazz; + else if (clazz.equals(Object.class)) + return clazz; + else + return clazz.getSuperclass(); + } @Override public void cleanupAll() { diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java deleted file mode 100644 index e71bacef..00000000 --- a/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkSpoutHook.java +++ /dev/null @@ -1,60 +0,0 @@ -package com.comphenix.protocol.injector.player; - -import java.lang.reflect.InvocationTargetException; -import java.util.logging.Logger; - -import net.minecraft.server.Packet; - -import org.bukkit.Bukkit; -import org.bukkit.entity.Player; -import org.bukkit.plugin.Plugin; - -import com.comphenix.protocol.events.PacketListener; -import com.comphenix.protocol.injector.ListenerInvoker; - -public class NetworkSpoutHook extends PlayerInjector { - - public NetworkSpoutHook(Logger logger, Player player, ListenerInvoker invoker) throws IllegalAccessException { - super(logger, player, invoker); - } - - @Override - protected boolean hasListener(int packetID) { - return false; - } - - @Override - public boolean canInject() { - return getSpout() != null; - } - - private Plugin getSpout() { - // Spout must be loaded - try { - return Bukkit.getServer().getPluginManager().getPlugin("Spout"); - } catch (Throwable e) { - return null; - } - } - - @Override - public void injectManager() { - - } - - @Override - public void sendServerPacket(Packet packet, boolean filtered) throws InvocationTargetException { - - } - - @Override - public void cleanupAll() { - // TODO Auto-generated method stub - - } - - @Override - public void checkListener(PacketListener listener) { - // We support everything Spout does - } -} diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java index 1ceb0725..d10bfb64 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjectionHandler.java @@ -23,6 +23,11 @@ import com.comphenix.protocol.injector.PlayerLoggedOutException; import com.comphenix.protocol.injector.PacketFilterManager.PlayerInjectHooks; import com.google.common.collect.ImmutableSet; +/** + * Responsible for injecting into a player's sendPacket method. + * + * @author Kristian + */ public class PlayerInjectionHandler { // Server connection injection diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java index 9a1779ec..f80b8e49 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java @@ -50,6 +50,9 @@ abstract class PlayerInjector { protected static Field inputField; protected static Field netHandlerField; + // Whether or not we're using a proxy type + private static boolean hasProxyType; + // To add our injected array lists protected static StructureModifier networkModifier; @@ -142,6 +145,14 @@ abstract class PlayerInjector { } } + /** + * Retrieve whether or not the server handler is a proxy object. + * @return TRUE if it is, FALSE otherwise. + */ + protected boolean hasProxyServerHandler() { + return hasProxyType; + } + private Field getProxyField(EntityPlayer notchEntity, Field serverField) { try { @@ -154,6 +165,8 @@ abstract class PlayerInjector { if (handler instanceof Factory) return null; + hasProxyType = true; + // No? Is it a Proxy type? try { FuzzyReflection reflection = FuzzyReflection.fromObject(handler, true); @@ -162,7 +175,7 @@ abstract class PlayerInjector { return reflection.getFieldByType(".*NetServerHandler"); } catch (RuntimeException e) { - logger.log(Level.WARNING, "Server handler is a proxy type.", e); + logger.log(Level.WARNING, "Detected server handler proxy type by another plugin. Conflict may occur!"); } } diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/ObjectCloner.java b/ProtocolLib/src/com/comphenix/protocol/reflect/ObjectCloner.java index 65902cbd..e64c32a8 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/ObjectCloner.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/ObjectCloner.java @@ -55,6 +55,14 @@ public class ObjectCloner { // System.out.println(String.format("Writing value %s to %s", // value, modifier.getFields().get(i).getName())); } + + // Copy private fields underneath + Class superclass = commonType.getSuperclass(); + + if (!superclass.equals(Object.class)) { + copyTo(source, destination, superclass); + } + } catch (FieldAccessException e) { throw new RuntimeException("Unable to copy fields from " + commonType.getName(), e); } diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java index 8c3b8ebc..fad95379 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java @@ -235,8 +235,9 @@ public class DefaultInstances { params[i] = getDefaultInternal(types[i], providers, recursionLevel + 1); // Did we break the non-null contract? - if (params[i] == null && nonNull) + if (params[i] == null && nonNull) { return null; + } } return createInstance(type, minimum, types, params); @@ -244,7 +245,6 @@ public class DefaultInstances { } catch (Exception e) { // Nope, we couldn't create this type - e.printStackTrace(); } // No suitable default value could be found diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java index 97bad0a6..25f78a03 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java @@ -33,13 +33,37 @@ public class ExistingGenerator implements InstanceProvider { * @return The instance generator. */ public static ExistingGenerator fromObjectFields(Object object) { + if (object == null) + throw new IllegalArgumentException("Object cannot be NULL."); + + return fromObjectFields(object, object.getClass()); + } + + /** + * Automatically create an instance provider from a objects public and private fields. + *

+ * If two or more fields share the same type, the last declared non-null field will take + * precedent. + * @param object - object to create an instance generator from. + * @param type - the type to cast the object. + * @return The instance generator. + */ + public static ExistingGenerator fromObjectFields(Object object, Class type) { ExistingGenerator generator = new ExistingGenerator(); + // Possible errors + if (object == null) + throw new IllegalArgumentException("Object cannot be NULL."); + if (type == null) + throw new IllegalArgumentException("Type cannot be NULL."); + if (!type.isAssignableFrom(object.getClass())) + throw new IllegalArgumentException("Type must be a superclass or be the same type."); + // Read instances from every field. - for (Field field : FuzzyReflection.fromObject(object, true).getFields()) { + for (Field field : FuzzyReflection.fromClass(type, true).getFields()) { try { Object value = FieldUtils.readField(field, object, true); - + // Use the type of the field, not the object itself if (value != null) generator.addObject(field.getType(), value); From 0e76d8ea2b11da17dd036849dc3fe789b470002d Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 5 Oct 2012 03:12:35 +0200 Subject: [PATCH 22/23] Make the proxy creation even more flexible. Now we even support Orebfuscator without using Spout. --- .../player/NetworkServerInjector.java | 64 +++++++++++++++---- .../injector/player/PlayerInjector.java | 14 ++-- .../reflect/instances/DefaultInstances.java | 39 +++++++++-- .../reflect/instances/ExistingGenerator.java | 14 ++-- 4 files changed, 98 insertions(+), 33 deletions(-) diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java index 55ebd698..72de1223 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/NetworkServerInjector.java @@ -21,6 +21,7 @@ import com.comphenix.protocol.injector.ListenerInvoker; import com.comphenix.protocol.reflect.FieldUtils; import com.comphenix.protocol.reflect.FuzzyReflection; import com.comphenix.protocol.reflect.ObjectCloner; +import com.comphenix.protocol.reflect.VolatileField; import com.comphenix.protocol.reflect.instances.DefaultInstances; import com.comphenix.protocol.reflect.instances.ExistingGenerator; @@ -97,6 +98,29 @@ public class NetworkServerInjector extends PlayerInjector { if (serverHandlerRef.getValue() instanceof Factory) return; + if (!tryInjectManager()) { + + // Try to override the proxied object + if (proxyServerField != null) { + serverHandlerRef = new VolatileField(proxyServerField, serverHandler, true); + serverHandler = serverHandlerRef.getValue(); + + if (serverHandler == null) + throw new RuntimeException("Cannot hook player: Inner proxy object is NULL."); + + // Try again + if (tryInjectManager()) { + // It worked - probably + return; + } + } + + throw new RuntimeException( + "Cannot hook player: Unable to find a valid constructor for the NetServerHandler object."); + } + } + + private boolean tryInjectManager() { Class serverClass = serverHandler.getClass(); Enhancer ex = new Enhancer(); @@ -135,18 +159,22 @@ public class NetworkServerInjector extends PlayerInjector { } }); - // Use the existing field values when we create our copy + // Find the Minecraft NetServerHandler superclass + Class minecraftSuperClass = getFirstMinecraftSuperClass(serverHandler.getClass()); + ExistingGenerator generator = ExistingGenerator.fromObjectFields(serverHandler, minecraftSuperClass); DefaultInstances serverInstances = null; - if (hasProxyServerHandler()) { - Class minecraftSuperClass = getFirstMinecraftSuperClass(serverHandler.getClass()); - serverInstances = DefaultInstances.fromArray( - ExistingGenerator.fromObjectFields(serverHandler, minecraftSuperClass)); + // Maybe the proxy instance can help? + Object proxyInstance = getProxyServerHandler(); + + // Use the existing server proxy when we create one + if (proxyInstance != null && proxyInstance != serverHandler) { + serverInstances = DefaultInstances.fromArray(generator, + ExistingGenerator.fromObjectArray(new Object[] { proxyInstance })); } else { - serverInstances = DefaultInstances.fromArray( - ExistingGenerator.fromObjectFields(serverHandler)); + serverInstances = DefaultInstances.fromArray(generator); } - + serverInstances.setNonNull(true); serverInstances.setMaximumRecursion(1); @@ -158,19 +186,31 @@ public class NetworkServerInjector extends PlayerInjector { //copyTo(serverHandler, proxyObject); serverInjection.replaceServerHandler(serverHandler, proxyObject); serverHandlerRef.setValue(proxyObject); + return true; } else { - throw new RuntimeException( - "Cannot hook player: Unable to find a valid constructor for the NetServerHandler object."); + return false; } } + private Object getProxyServerHandler() { + if (proxyServerField != null && !proxyServerField.equals(serverHandlerRef.getField())) { + try { + return FieldUtils.readField(proxyServerField, serverHandler, true); + } catch (Throwable e) { + // Oh well + } + } + + return null; + } + private Class getFirstMinecraftSuperClass(Class clazz) { - if (clazz.getName().startsWith("net.minecraft")) + if (clazz.getName().startsWith("net.minecraft.server.")) return clazz; else if (clazz.equals(Object.class)) return clazz; else - return clazz.getSuperclass(); + return getFirstMinecraftSuperClass(clazz.getSuperclass()); } @Override diff --git a/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java index f80b8e49..62e01f33 100644 --- a/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java +++ b/ProtocolLib/src/com/comphenix/protocol/injector/player/PlayerInjector.java @@ -43,8 +43,8 @@ import com.comphenix.protocol.reflect.VolatileField; abstract class PlayerInjector { // Cache previously retrieved fields - private static Field serverHandlerField; - private static Field proxyServerField; + protected static Field serverHandlerField; + protected static Field proxyServerField; protected static Field networkManagerField; protected static Field inputField; @@ -115,12 +115,7 @@ abstract class PlayerInjector { } // Yo dawg - if (proxyServerField != null) { - Object container = FieldUtils.readField(serverHandlerField, notchEntity, true); - serverHandlerRef = new VolatileField(proxyServerField, container); - } else { - serverHandlerRef = new VolatileField(serverHandlerField, notchEntity); - } + serverHandlerRef = new VolatileField(serverHandlerField, notchEntity); serverHandler = serverHandlerRef.getValue(); // Next, get the network manager @@ -166,6 +161,7 @@ abstract class PlayerInjector { return null; hasProxyType = true; + logger.log(Level.WARNING, "Detected server handler proxy type by another plugin. Conflict may occur!"); // No? Is it a Proxy type? try { @@ -175,7 +171,7 @@ abstract class PlayerInjector { return reflection.getFieldByType(".*NetServerHandler"); } catch (RuntimeException e) { - logger.log(Level.WARNING, "Detected server handler proxy type by another plugin. Conflict may occur!"); + // Damn } } diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java index fad95379..7dd03cf7 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/DefaultInstances.java @@ -156,9 +156,12 @@ public class DefaultInstances { * @param type - type to construct. * @return A constructor with the fewest number of parameters, or NULL if the type has no constructors. */ - @SuppressWarnings("unchecked") public Constructor getMinimumConstructor(Class type) { - + return getMinimumConstructor(type, registered, 0); + } + + @SuppressWarnings("unchecked") + private Constructor getMinimumConstructor(Class type, List providers, int recursionLevel) { Constructor minimum = null; int lastCount = Integer.MAX_VALUE; @@ -170,6 +173,13 @@ public class DefaultInstances { // require itself in the constructor. if (types.length < lastCount) { if (!contains(types, type)) { + if (nonNull) { + // Make sure all of these types are non-null + if (isAnyNull(types, providers, recursionLevel)) { + continue; + } + } + minimum = (Constructor) candidate; lastCount = types.length; @@ -183,6 +193,27 @@ public class DefaultInstances { return minimum; } + /** + * Determine if any of the given types will be NULL once created. + *

+ * Recursion level is the number of times the default method has been called. + * @param types - types to check. + * @param providers - instance providers. + * @param recursionLevel - current recursion level. + * @return + */ + private boolean isAnyNull(Class[] types, List providers, int recursionLevel) { + // Just check if any of them are NULL + for (Class type : types) { + if (getDefaultInternal(type, providers, recursionLevel) == null) { + System.out.println(type.getName() + " is NULL!"); + return true; + } + } + + return false; + } + /** * Retrieves a default instance or value that is assignable to this type. *

@@ -198,7 +229,7 @@ public class DefaultInstances { * * * @param type - the type to construct a default value. - * @param providers - instance providers used during the + * @param providers - instance providers used during the construction. * @return A default value/instance, or NULL if not possible. */ public T getDefault(Class type, List providers) { @@ -221,7 +252,7 @@ public class DefaultInstances { return null; } - Constructor minimum = getMinimumConstructor(type); + Constructor minimum = getMinimumConstructor(type, providers, recursionLevel + 1); // Create the type with this constructor using default values. This might fail, though. try { diff --git a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java index 25f78a03..d183ad95 100644 --- a/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java +++ b/ProtocolLib/src/com/comphenix/protocol/reflect/instances/ExistingGenerator.java @@ -17,8 +17,7 @@ import com.comphenix.protocol.reflect.FuzzyReflection; */ public class ExistingGenerator implements InstanceProvider { - @SuppressWarnings("rawtypes") - private Map existingValues = new HashMap(); + private Map existingValues = new HashMap(); private ExistingGenerator() { // Only accessible to the constructors @@ -72,7 +71,7 @@ public class ExistingGenerator implements InstanceProvider { // Yes, swallow it. No, really. } } - + return generator; } @@ -94,19 +93,18 @@ public class ExistingGenerator implements InstanceProvider { if (value == null) throw new IllegalArgumentException("Value cannot be NULL."); - existingValues.put(value.getClass(), value); + existingValues.put(value.getClass().getName(), value); } private void addObject(Class type, Object value) { - existingValues.put(type, value); + existingValues.put(type.getName(), value); } - @Override public Object create(@Nullable Class type) { - Object value = existingValues.get(type); - + Object value = existingValues.get(type.getName()); + // NULL values indicate that the generator failed return value; } From 88da4e327200d90c734597347931216ad7fe3931 Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 5 Oct 2012 03:50:28 +0200 Subject: [PATCH 23/23] Bumping to version 1.3.0 --- ProtocolLib/src/plugin.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ProtocolLib/src/plugin.yml b/ProtocolLib/src/plugin.yml index 19931803..3901dcf0 100644 --- a/ProtocolLib/src/plugin.yml +++ b/ProtocolLib/src/plugin.yml @@ -1,5 +1,5 @@ name: ProtocolLib -version: 1.2.2 +version: 1.3.0 description: Provides read/write access to the Minecraft protocol. author: Comphenix website: http://www.comphenix.net/ProtocolLib