Skip to content

Commit 0ee93ac

Browse files
authored
Fixed null packet handles in Bundle (#2328)
1 parent d83dd9a commit 0ee93ac

File tree

5 files changed

+48
-31
lines changed

5 files changed

+48
-31
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ private PacketEvent(Object source, PacketContainer packet, Player player, boolea
9292
private PacketEvent(Object source, PacketContainer packet, NetworkMarker marker, Player player, boolean serverPacket,
9393
boolean filtered, @Nullable PacketEvent bundleEvent) {
9494
super(source);
95+
if(packet == null) {
96+
throw new IllegalArgumentException("packet cannot be null");
97+
}
9598
this.packet = packet;
9699
this.playerReference = new WeakReference<>(player);
97100
this.networkMarker = marker;

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

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.comphenix.protocol.AsynchronousManager;
44
import com.comphenix.protocol.PacketType;
55
import com.comphenix.protocol.PacketType.Sender;
6+
import com.comphenix.protocol.ProtocolLibrary;
67
import com.comphenix.protocol.async.AsyncFilterManager;
78
import com.comphenix.protocol.error.ErrorReporter;
89
import com.comphenix.protocol.error.Report;
@@ -32,6 +33,8 @@
3233
import java.util.List;
3334
import java.util.Objects;
3435
import java.util.Set;
36+
import java.util.logging.Level;
37+
3538
import org.bukkit.Location;
3639
import org.bukkit.Server;
3740
import org.bukkit.World;
@@ -541,25 +544,29 @@ public PacketType getPacketType(Object packet) {
541544
}
542545

543546
private void postPacketToListeners(SortedPacketListenerList listeners, PacketEvent event, boolean outbound) {
544-
// append async marker if any async listener for the packet was registered
545-
if (this.asyncFilterManager.hasAsynchronousListeners(event)) {
546-
event.setAsyncMarker(this.asyncFilterManager.createAsyncMarker());
547-
}
547+
try {
548+
// append async marker if any async listener for the packet was registered
549+
if (this.asyncFilterManager.hasAsynchronousListeners(event)) {
550+
event.setAsyncMarker(this.asyncFilterManager.createAsyncMarker());
551+
}
548552

549-
// post to sync listeners
550-
if (outbound) {
551-
listeners.invokePacketSending(this.reporter, event);
552-
} else {
553-
listeners.invokePacketRecieving(this.reporter, event);
554-
}
553+
// post to sync listeners
554+
if (outbound) {
555+
listeners.invokePacketSending(this.reporter, event);
556+
} else {
557+
listeners.invokePacketRecieving(this.reporter, event);
558+
}
555559

556-
// check if we need to post the packet to the async handler
557-
if (!event.isCancelled() && event.getAsyncMarker() != null && !event.getAsyncMarker().isAsyncCancelled()) {
558-
this.asyncFilterManager.enqueueSyncPacket(event, event.getAsyncMarker());
560+
// check if we need to post the packet to the async handler
561+
if (!event.isCancelled() && event.getAsyncMarker() != null && !event.getAsyncMarker().isAsyncCancelled()) {
562+
this.asyncFilterManager.enqueueSyncPacket(event, event.getAsyncMarker());
559563

560-
// cancel the packet here for async processing (enqueueSyncPacket will create a copy of the event)
561-
event.setReadOnly(false);
562-
event.setCancelled(true);
564+
// cancel the packet here for async processing (enqueueSyncPacket will create a copy of the event)
565+
event.setReadOnly(false);
566+
event.setCancelled(true);
567+
}
568+
} catch (Throwable t) {
569+
plugin.getLogger().log(Level.WARNING, "Failed to process " + (outbound ? "outbound" : "inbound") + " packet event: " + event, t);
563570
}
564571
}
565572

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,16 @@
2020
import java.util.ArrayList;
2121
import java.util.Collection;
2222
import java.util.List;
23+
import java.util.logging.Level;
2324

2425
import com.comphenix.protocol.PacketType;
26+
import com.comphenix.protocol.ProtocolLibrary;
2527
import com.comphenix.protocol.concurrency.AbstractConcurrentListenerMultimap;
2628
import com.comphenix.protocol.error.ErrorReporter;
2729
import com.comphenix.protocol.events.ListenerPriority;
2830
import com.comphenix.protocol.events.PacketContainer;
2931
import com.comphenix.protocol.events.PacketEvent;
3032
import com.comphenix.protocol.events.PacketListener;
31-
import com.comphenix.protocol.injector.packet.PacketRegistry;
32-
import com.comphenix.protocol.reflect.StructureModifier;
3333
import com.comphenix.protocol.timing.TimedListenerManager;
3434
import com.comphenix.protocol.timing.TimedListenerManager.ListenerType;
3535
import com.comphenix.protocol.timing.TimedTracker;
@@ -152,12 +152,23 @@ public void invokePacketSending(ErrorReporter reporter, PacketEvent event, @Null
152152
Iterable<PacketContainer> packets = event.getPacket().getPacketBundles().read(0);
153153
List<PacketContainer> outPackets = new ArrayList<>();
154154
for (PacketContainer subPacket : packets) {
155+
if(subPacket == null) {
156+
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());
157+
continue;
158+
}
155159
PacketEvent subPacketEvent = PacketEvent.fromServer(this, subPacket, event.getNetworkMarker(), event.getPlayer());
156160
invokeUnpackedPacketSending(reporter, subPacketEvent, priorityFilter);
157161

158162
if (!subPacketEvent.isCancelled()) {
159163
// if the packet event has been cancelled, the packet will be removed from the bundle
160-
outPackets.add(subPacketEvent.getPacket());
164+
PacketContainer packet = subPacketEvent.getPacket();
165+
if(packet == null) {
166+
ProtocolLibrary.getPlugin().getLogger().log(Level.WARNING, "null packet container returned for " + subPacketEvent, new Throwable());
167+
} else if(packet.getHandle() == null) {
168+
ProtocolLibrary.getPlugin().getLogger().log(Level.WARNING, "null packet handle returned for " + subPacketEvent, new Throwable());
169+
} else {
170+
outPackets.add(packet);
171+
}
161172
}
162173
}
163174

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ public static EquivalentConverter<WrappedLevelChunkData.LightData> getWrappedLig
620620
}
621621

622622
public static EquivalentConverter<PacketContainer> getPacketContainerConverter() {
623-
return handle(PacketContainer::getHandle, PacketContainer::fromPacket, PacketContainer.class);
623+
return ignoreNull(handle(PacketContainer::getHandle, PacketContainer::fromPacket, PacketContainer.class));
624624
}
625625

626626
/**

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,22 @@
1414
*/
1515
package com.comphenix.protocol.wrappers;
1616

17+
import com.comphenix.protocol.reflect.EquivalentConverter;
18+
import com.comphenix.protocol.reflect.FuzzyReflection;
19+
import com.comphenix.protocol.reflect.accessors.Accessors;
20+
import com.comphenix.protocol.reflect.accessors.MethodAccessor;
21+
import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract;
22+
import com.comphenix.protocol.utility.MinecraftReflection;
23+
1724
import java.lang.reflect.Array;
18-
import java.lang.reflect.Method;
1925
import java.lang.reflect.Modifier;
2026
import java.util.ArrayList;
2127
import java.util.Collection;
2228
import java.util.List;
2329
import java.util.Optional;
24-
import java.util.concurrent.ConcurrentHashMap;
25-
import java.util.concurrent.ConcurrentMap;
2630
import java.util.function.Function;
2731
import java.util.function.Supplier;
2832

29-
import com.comphenix.protocol.reflect.EquivalentConverter;
30-
import com.comphenix.protocol.reflect.FuzzyReflection;
31-
import com.comphenix.protocol.reflect.accessors.Accessors;
32-
import com.comphenix.protocol.reflect.accessors.MethodAccessor;
33-
import com.comphenix.protocol.reflect.fuzzy.FuzzyMethodContract;
34-
import com.comphenix.protocol.utility.MinecraftReflection;
35-
import org.checkerframework.checker.units.qual.C;
36-
3733
/**
3834
* Utility class for converters
3935
* @author dmulloy2

0 commit comments

Comments
 (0)