Fixed null packet handles in Bundle (#2328)

src/main/java/com/comphenix/protocol/events/PacketEvent.java

@@ -92,6 +92,9 @@ public class PacketEvent extends EventObject implements Cancellable {
 	private PacketEvent(Object source, PacketContainer packet, NetworkMarker marker, Player player, boolean serverPacket,
 			boolean filtered, @Nullable PacketEvent bundleEvent) {
 		super(source);
+		if(packet == null) {
+			throw new IllegalArgumentException("packet cannot be null");
+		}
 		this.packet = packet;
 		this.playerReference = new WeakReference<>(player);
 		this.networkMarker = marker;

src/main/java/com/comphenix/protocol/injector/PacketFilterManager.java

@@ -3,6 +3,7 @@ package com.comphenix.protocol.injector;
 import com.comphenix.protocol.AsynchronousManager;
 import com.comphenix.protocol.PacketType;
 import com.comphenix.protocol.PacketType.Sender;
+import com.comphenix.protocol.ProtocolLibrary;
 import com.comphenix.protocol.async.AsyncFilterManager;
 import com.comphenix.protocol.error.ErrorReporter;
 import com.comphenix.protocol.error.Report;
@@ -32,6 +33,8 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.logging.Level;
+
 import org.bukkit.Location;
 import org.bukkit.Server;
 import org.bukkit.World;
@@ -541,25 +544,29 @@ public class PacketFilterManager implements ListenerInvoker, InternalManager {
 	}
 
 	private void postPacketToListeners(SortedPacketListenerList listeners, PacketEvent event, boolean outbound) {
-		// append async marker if any async listener for the packet was registered
+		try {
-		if (this.asyncFilterManager.hasAsynchronousListeners(event)) {
+			// append async marker if any async listener for the packet was registered
-			event.setAsyncMarker(this.asyncFilterManager.createAsyncMarker());
+			if (this.asyncFilterManager.hasAsynchronousListeners(event)) {
-		}
+				event.setAsyncMarker(this.asyncFilterManager.createAsyncMarker());
+			}
 
-		// post to sync listeners
+			// post to sync listeners
-		if (outbound) {
+			if (outbound) {
-			listeners.invokePacketSending(this.reporter, event);
+				listeners.invokePacketSending(this.reporter, event);
-		} else {
+			} else {
-			listeners.invokePacketRecieving(this.reporter, event);
+				listeners.invokePacketRecieving(this.reporter, event);
-		}
+			}
 
-		// check if we need to post the packet to the async handler
+			// check if we need to post the packet to the async handler
-		if (!event.isCancelled() && event.getAsyncMarker() != null && !event.getAsyncMarker().isAsyncCancelled()) {
+			if (!event.isCancelled() && event.getAsyncMarker() != null && !event.getAsyncMarker().isAsyncCancelled()) {
-			this.asyncFilterManager.enqueueSyncPacket(event, event.getAsyncMarker());
+				this.asyncFilterManager.enqueueSyncPacket(event, event.getAsyncMarker());
 
-			// cancel the packet here for async processing (enqueueSyncPacket will create a copy of the event)
+				// cancel the packet here for async processing (enqueueSyncPacket will create a copy of the event)
-			event.setReadOnly(false);
+				event.setReadOnly(false);
-			event.setCancelled(true);
+				event.setCancelled(true);
+			}
+		} catch (Throwable t) {
+			plugin.getLogger().log(Level.WARNING, "Failed to process " + (outbound ? "outbound" : "inbound") + " packet event: " + event, t);
 		}
 	}
 

src/main/java/com/comphenix/protocol/injector/SortedPacketListenerList.java

@@ -20,16 +20,16 @@ package com.comphenix.protocol.injector;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.logging.Level;
 
 import com.comphenix.protocol.PacketType;
+import com.comphenix.protocol.ProtocolLibrary;
 import com.comphenix.protocol.concurrency.AbstractConcurrentListenerMultimap;
 import com.comphenix.protocol.error.ErrorReporter;
 import com.comphenix.protocol.events.ListenerPriority;
 import com.comphenix.protocol.events.PacketContainer;
 import com.comphenix.protocol.events.PacketEvent;
 import com.comphenix.protocol.events.PacketListener;
-import com.comphenix.protocol.injector.packet.PacketRegistry;
-import com.comphenix.protocol.reflect.StructureModifier;
 import com.comphenix.protocol.timing.TimedListenerManager;
 import com.comphenix.protocol.timing.TimedListenerManager.ListenerType;
 import com.comphenix.protocol.timing.TimedTracker;
@@ -152,12 +152,23 @@ public final class SortedPacketListenerList extends AbstractConcurrentListenerMu
 			Iterable<PacketContainer> packets = event.getPacket().getPacketBundles().read(0);
 			List<PacketContainer> outPackets = new ArrayList<>();
 			for (PacketContainer subPacket : packets) {
+				if(subPacket == null) {
+					ProtocolLibrary.getPlugin().getLogger().log(Level.WARNING, "Failed to invoke packet event " + (priorityFilter == null ? "" : ("with priority " + priorityFilter)) + " in bundle because bundle contains null packet: " + packets, new Throwable());
+					continue;
+				}
 				PacketEvent subPacketEvent = PacketEvent.fromServer(this, subPacket, event.getNetworkMarker(), event.getPlayer());
 				invokeUnpackedPacketSending(reporter, subPacketEvent, priorityFilter);
 
 				if (!subPacketEvent.isCancelled()) {
 					// if the packet event has been cancelled, the packet will be removed from the bundle
-					outPackets.add(subPacketEvent.getPacket());
+					PacketContainer packet = subPacketEvent.getPacket();
+					if(packet == null) {
+						ProtocolLibrary.getPlugin().getLogger().log(Level.WARNING, "null packet container returned for " + subPacketEvent, new Throwable());
+					} else if(packet.getHandle() == null) {
+						ProtocolLibrary.getPlugin().getLogger().log(Level.WARNING, "null packet handle returned for " + subPacketEvent, new Throwable());
+					} else {
+						outPackets.add(packet);
+					}
 				}
 			}
 

src/main/java/com/comphenix/protocol/wrappers/BukkitConverters.java

@@ -620,7 +620,7 @@ public class BukkitConverters {
 	}
 
 	public static EquivalentConverter<PacketContainer> getPacketContainerConverter() {
-		return handle(PacketContainer::getHandle, PacketContainer::fromPacket, PacketContainer.class);
+		return ignoreNull(handle(PacketContainer::getHandle, PacketContainer::fromPacket, PacketContainer.class));
 	}
 
 	/**

src/main/java/com/comphenix/protocol/wrappers/Converters.java

@@ -14,25 +14,21 @@
  */
 package com.comphenix.protocol.wrappers;
 
-import java.lang.reflect.Array;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.function.Function;
-import java.util.function.Supplier;
-
 import com.comphenix.protocol.reflect.EquivalentConverter;
 import com.comphenix.protocol.reflect.FuzzyReflection;
 import com.comphenix.protocol.reflect.accessors.Accessors;
 import com.comphenix.protocol.reflect.accessors.MethodAccessor;
 import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract;
 import com.comphenix.protocol.utility.MinecraftReflection;
-import org.checkerframework.checker.units.qual.C;
+
+import java.lang.reflect.Array;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.function.Supplier;
 
 /**
  * Utility class for converters