fix: attempt to retrieve online from offline player for perm attachments (#3182)

* fix: attempt to retrieve online from offline player for perm attachments

* Use synchonized WeakHashMap to be sure

* We don't always need the permission attachment, check for null

Author: dordsor21

worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/BukkitPermissionAttachmentManager.java

@@ -1,27 +1,33 @@
 package com.fastasyncworldedit.bukkit;
 
 import com.sk89q.worldedit.bukkit.WorldEditPlugin;
+import org.bukkit.OfflinePlayer;
 import org.bukkit.entity.Player;
 import org.bukkit.permissions.PermissibleBase;
 import org.bukkit.permissions.PermissionAttachment;
 
 import javax.annotation.Nullable;
+import java.util.Collections;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.WeakHashMap;
 
 public class BukkitPermissionAttachmentManager {
 
     private final WorldEditPlugin plugin;
-    private final Map<Player, PermissionAttachment> attachments = new ConcurrentHashMap<>();
+    private final Map<Player, PermissionAttachment> attachments = Collections.synchronizedMap(new WeakHashMap<>());
     private PermissionAttachment noopAttachment;
 
     public BukkitPermissionAttachmentManager(WorldEditPlugin plugin) {
         this.plugin = plugin;
     }
 
-    public PermissionAttachment getOrAddAttachment(@Nullable final Player p) {
-        if (p == null) {
-            return null;
+    @Nullable
+    public PermissionAttachment getOrAddAttachment(@Nullable Player p) {
+        if (p instanceof OfflinePlayer offline) {
+            p = offline.getPlayer();
+        }
+        if (p == null || !p.isOnline()) {
+            return null; // The attachment is only used for setting permissions (e.g. when toggling bypass) so null is acceptable
         }
         if (p.hasMetadata("NPC")) {
             if (this.noopAttachment == null) {

worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/BukkitPlayer.java

@@ -33,6 +33,7 @@
 import com.sk89q.worldedit.extent.Extent;
 import com.sk89q.worldedit.extent.inventory.BlockBag;
 import com.sk89q.worldedit.internal.cui.CUIEvent;
+import com.sk89q.worldedit.internal.util.LogManagerCompat;
 import com.sk89q.worldedit.math.BlockVector3;
 import com.sk89q.worldedit.math.Vector3;
 import com.sk89q.worldedit.session.SessionKey;
@@ -51,6 +52,7 @@
 import com.sk89q.worldedit.world.block.BlockTypes;
 import com.sk89q.worldedit.world.gamemode.GameMode;
 import com.sk89q.worldedit.world.gamemode.GameModes;
+import org.apache.logging.log4j.Logger;
 import org.bukkit.Bukkit;
 import org.bukkit.Location;
 import org.bukkit.Material;
@@ -73,10 +75,12 @@
 
 public class BukkitPlayer extends AbstractPlayerActor {
 
+    private static final Logger LOGGER = LogManagerCompat.getLogger();
+
     private final Player player;
     private final WorldEditPlugin plugin;
     //FAWE start
-    private final PermissionAttachment permAttachment;
+    private PermissionAttachment permAttachment = null;
 
     /**
      * This constructs a new {@link BukkitPlayer} for the given {@link Player}.
@@ -89,7 +93,6 @@ public BukkitPlayer(@Nullable Player player) {
         super(player != null ? getExistingMap(WorldEditPlugin.getInstance(), player) : new ConcurrentHashMap<>());
         this.plugin = WorldEditPlugin.getInstance();
         this.player = player;
-        this.permAttachment = plugin.getPermissionAttachmentManager().getOrAddAttachment(player);
     }
     //FAWE end
 
@@ -105,7 +108,6 @@ public BukkitPlayer(@Nonnull WorldEditPlugin plugin, @Nullable Player player) {
         this.plugin = plugin;
         this.player = player;
         //FAWE start
-        this.permAttachment = plugin.getPermissionAttachmentManager().getOrAddAttachment(player);
         if (player != null && Settings.settings().CLIPBOARD.USE_DISK) {
             BukkitPlayer cached = WorldEditPlugin.getInstance().getCachedPlayer(player);
             if (cached == null) {
@@ -297,6 +299,17 @@ public void setPermission(String permission, boolean value) {
             }
         }
         if (usesuperperms) {
+            if (this.permAttachment == null) {
+                this.permAttachment = plugin.getPermissionAttachmentManager().getOrAddAttachment(player);
+            }
+            if (this.permAttachment == null) {
+                LOGGER.warn(
+                        "Attempted to set permission for offline player `{}`, UUID: `{}`?!",
+                        player.getName(),
+                        player.getUniqueId()
+                );
+                return;
+            }
             permAttachment.setPermission(permission, value);
         }
     }