From 8c20d1933901149e8d526e0613b5ddc7b6797f77 Mon Sep 17 00:00:00 2001 From: Dan Mulloy Date: Wed, 7 Aug 2019 11:34:59 -0400 Subject: [PATCH] Try to fix a null pointer Addresses #665 --- .../protocol/async/AsyncFilterManager.java | 4 +-- .../concurrency/ConcurrentPlayerMap.java | 3 ++ .../protocol/events/PacketEvent.java | 34 ++++++++++--------- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/comphenix/protocol/async/AsyncFilterManager.java b/src/main/java/com/comphenix/protocol/async/AsyncFilterManager.java index dedd5577..abc21532 100644 --- a/src/main/java/com/comphenix/protocol/async/AsyncFilterManager.java +++ b/src/main/java/com/comphenix/protocol/async/AsyncFilterManager.java @@ -318,12 +318,12 @@ public class AsyncFilterManager implements AsynchronousManager { // The player is only be null when they're logged out, // so this should be a pretty safe check - Player player = newEvent.getPlayer(); + Player player = syncPacket.getPlayer(); if (player != null) { // Start the process getSendingQueue(syncPacket).enqueue(newEvent); - // We know this is occuring on the main thread, so pass TRUE + // We know this is occurring on the main thread, so pass TRUE getProcessingQueue(syncPacket).enqueue(newEvent, true); } } diff --git a/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java b/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java index 00b0261f..5e21f91d 100644 --- a/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java +++ b/src/main/java/com/comphenix/protocol/concurrency/ConcurrentPlayerMap.java @@ -3,6 +3,7 @@ package com.comphenix.protocol.concurrency; import com.comphenix.protocol.utility.SafeCacheBuilder; import com.comphenix.protocol.utility.Util; import com.google.common.base.Function; +import com.google.common.base.Preconditions; import com.google.common.cache.CacheLoader; import com.google.common.cache.RemovalListener; import com.google.common.collect.AbstractIterator; @@ -188,6 +189,8 @@ public class ConcurrentPlayerMap extends AbstractMap imp * @return The key. */ private Object cachePlayerKey(Player player) { + Preconditions.checkNotNull(player, "player cannot be null"); + Object key = keyMethod.apply(player); keyLookup.put(key, player); diff --git a/src/main/java/com/comphenix/protocol/events/PacketEvent.java b/src/main/java/com/comphenix/protocol/events/PacketEvent.java index bb4c0caf..177ab7b1 100644 --- a/src/main/java/com/comphenix/protocol/events/PacketEvent.java +++ b/src/main/java/com/comphenix/protocol/events/PacketEvent.java @@ -50,7 +50,7 @@ public class PacketEvent extends EventObject implements Cancellable { "Plugin %s changed packet type from %s to %s in packet listener. This is confusing for other plugins! (Not an error, though!)"); private static final SetMultimap CHANGE_WARNINGS = - Multimaps.synchronizedSetMultimap(HashMultimap.create()); + Multimaps.synchronizedSetMultimap(HashMultimap.create()); /** * Automatically generated by Eclipse. @@ -58,8 +58,7 @@ public class PacketEvent extends EventObject implements Cancellable { private static final long serialVersionUID = -5360289379097430620L; private transient WeakReference playerReference; - private transient Player offlinePlayer; - + private PacketContainer packet; private boolean serverPacket; private boolean cancel; @@ -90,7 +89,7 @@ public class PacketEvent extends EventObject implements Cancellable { private PacketEvent(Object source, PacketContainer packet, NetworkMarker marker, Player player, boolean serverPacket, boolean filtered) { super(source); this.packet = packet; - this.playerReference = new WeakReference(player); + this.playerReference = new WeakReference<>(player); this.networkMarker = marker; this.serverPacket = serverPacket; this.filtered = filtered; @@ -99,7 +98,7 @@ public class PacketEvent extends EventObject implements Cancellable { private PacketEvent(PacketEvent origial, AsyncMarker asyncMarker) { super(origial.source); this.packet = origial.packet; - this.playerReference = origial.playerReference; + this.playerReference = origial.getPlayerReference(); this.cancel = origial.cancel; this.serverPacket = origial.serverPacket; this.filtered = origial.filtered; @@ -317,24 +316,26 @@ public class PacketEvent extends EventObject implements Cancellable { this.cancel = cancel; } - /** - * Retrieves the player that has sent the packet or is recieving it. - * @return The player associated with this event. - */ - public Player getPlayer() { + private WeakReference getPlayerReference() { Player player = playerReference.get(); - // Check if the player has been updated so we can do more stuff if (player instanceof TemporaryPlayer) { Player updated = player.getPlayer(); if (updated != null && !(updated instanceof TemporaryPlayer)) { playerReference.clear(); playerReference = new WeakReference<>(updated); - return updated; } } - return player; + return playerReference; + } + + /** + * Retrieves the player that has sent the packet or is receiving it. + * @return The player associated with this event. + */ + public Player getPlayer() { + return getPlayerReference().get(); } /** @@ -467,7 +468,8 @@ public class PacketEvent extends EventObject implements Cancellable { output.defaultWriteObject(); // Write the name of the player (or NULL if it's not set) - output.writeObject(playerReference.get() != null ? new SerializedOfflinePlayer(playerReference.get()) : null); + Player player = getPlayer(); + output.writeObject(player != null ? new SerializedOfflinePlayer(player) : null); } private void readObject(ObjectInputStream input) throws ClassNotFoundException, IOException { @@ -479,8 +481,8 @@ public class PacketEvent extends EventObject implements Cancellable { // Better than nothing if (serialized != null) { // Store it, to prevent weak reference from cleaning up the reference - offlinePlayer = serialized.getPlayer(); - playerReference = new WeakReference(offlinePlayer); + Player offlinePlayer = serialized.getPlayer(); + playerReference = new WeakReference<>(offlinePlayer); } }