GH-406 fix: audit findings

eternalcombat-plugin/src/main/java/com/eternalcode/combat/CombatPlugin.java

@@ -128,7 +128,7 @@
         this.dropService = new DropServiceImpl();
         this.dropKeepInventoryService = new DropKeepInventoryServiceImpl();
 
         UpdaterService updaterService = new UpdaterService(this.getDescription());
 
         MiniMessage miniMessage = MiniMessage.builder()
             .postProcessor(new AdventureLegacyColorPostProcessor())
@@ -208,8 +208,7 @@
             new CommandsBlocker(this.fightManager, noticeService, pluginConfig),
             new ElytraBlocker(this.fightManager, pluginConfig),
             new ElytraEquipBlocker(this.fightManager, noticeService, pluginConfig, server),
-            new FlyingBlocker(this.fightManager, pluginConfig, server),
-            new PlaceBlockBlocker(this.fightManager, noticeService, pluginConfig)
+            new FlyingBlocker(this.fightManager, pluginConfig, server)
         );
 
         new KnockbackMountController(noticeService, this.regionProvider, this.fightManager).register(this);

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/FightManagerImpl.java

@@ -27,13 +27,9 @@ public FightManagerImpl(EventManager eventManager) {
 
     @Override
     public boolean isInCombat(UUID player) {
-        if (!this.fights.containsKey(player)) {
-            return false;
-        }
-
         FightTag fightTag = this.fights.get(player);
 
-        return !fightTag.isExpired();
+        return fightTag != null && !fightTag.isExpired();
     }
 
     @Override

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/FightTagCommand.java

@@ -89,8 +89,9 @@ void tagMultiple(@Context CommandSender sender, @Arg Player firstTarget, @Arg Pl
             return;
         }
 
+        boolean firstAlreadyInCombat = this.fightManager.isInCombat(firstTarget.getUniqueId());
+
         FightTagEvent firstTagEvent = this.fightManager.tag(firstTarget.getUniqueId(), combatTime, CauseOfTag.COMMAND);
-        FightTagEvent secondTagEvent = this.fightManager.tag(secondTarget.getUniqueId(), combatTime, CauseOfTag.COMMAND);
 
         if (firstTagEvent.isCancelled()) {
             CancelTagReason cancelReason = firstTagEvent.getCancelReason();
@@ -100,15 +101,19 @@ void tagMultiple(@Context CommandSender sender, @Arg Player firstTarget, @Arg Pl
             return;
         }
 
+        FightTagEvent secondTagEvent = this.fightManager.tag(secondTarget.getUniqueId(), combatTime, CauseOfTag.COMMAND);
+
         if (secondTagEvent.isCancelled()) {
             CancelTagReason cancelReason = secondTagEvent.getCancelReason();
 
-            this.tagoutReasonHandler(sender, cancelReason, messagesSettings);
+            // Only roll back the first tag when this command actually created it;
+            // never untag a player who was already in combat beforehand.
+            if (!firstAlreadyInCombat) {
+                this.fightManager.untag(firstTarget.getUniqueId(), CauseOfUnTag.COMMAND);
+            }
 
-            return;
-        }
+            this.tagoutReasonHandler(sender, cancelReason, messagesSettings);
 
-        if (firstTagEvent.isCancelled() && secondTagEvent.isCancelled()) {
             return;
         }
 

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/FightTask.java

@@ -38,7 +38,7 @@ public void run() {
 
             if (fightTag.isExpired()) {
                 this.fightManager.untag(playerUniqueId, CauseOfUnTag.TIME_EXPIRED);
-                return;
+                continue;
             }
 
             Duration remaining = fightTag.getRemainingDuration();

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/blocker/CommandsBlocker.java

@@ -32,10 +32,12 @@ void onPlayerCommandPreprocess(PlayerCommandPreprocessEvent event) {
             return;
         }
 
-        String command = event.getMessage().substring(1);
+        String original = event.getMessage().substring(1).stripLeading();
+        String command = normalizeCommand(original);
 
         boolean isAnyMatch = this.config.commands.restrictedCommands.stream()
-            .anyMatch(restrictedCommand -> StringUtil.startsWithIgnoreCase(command, restrictedCommand));
+            .anyMatch(restrictedCommand -> StringUtil.startsWithIgnoreCase(original, restrictedCommand)
+                || StringUtil.startsWithIgnoreCase(command, restrictedCommand));
 
         WhitelistBlacklistMode mode = this.config.commands.commandRestrictionMode;
 
@@ -51,4 +53,21 @@ void onPlayerCommandPreprocess(PlayerCommandPreprocessEvent event) {
         }
     }
 
+    // Trim leading spaces and strip any namespace prefix (e.g. "minecraft:tp" -> "tp")
+    // from the command label so blacklisted commands cannot be bypassed with those forms.
+    private static String normalizeCommand(String rawCommand) {
+        String command = rawCommand.stripLeading();
+
+        int spaceIndex = command.indexOf(' ');
+        String label = spaceIndex == -1 ? command : command.substring(0, spaceIndex);
+
+        int colonIndex = label.indexOf(':');
+        if (colonIndex == -1) {
+            return command;
+        }
+
+        String rest = spaceIndex == -1 ? "" : command.substring(spaceIndex);
+        return label.substring(colonIndex + 1) + rest;
+    }
+
 }

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/death/DeathFlareController.java

@@ -102,7 +102,7 @@ private void spawnFlare(Player player) {
     }
 
     private void scheduleParticles(Firework flare, World world) {
-        this.scheduler.runLaterAsync(
+        this.scheduler.runLater(
             () -> {
                 if (flare.isDead() || !flare.isValid()) {
                     return;

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/drop/DropController.java

@@ -4,6 +4,7 @@
 import com.eternalcode.combat.fight.FightManager;
 import com.eternalcode.commons.adventure.AdventureUtil;
 import net.kyori.adventure.text.minimessage.MiniMessage;
+import org.bukkit.Location;
 import org.bukkit.Material;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.entity.Player;
@@ -16,6 +17,7 @@
 import org.bukkit.inventory.meta.SkullMeta;
 
 import java.util.List;
+import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.ThreadLocalRandom;
 
@@ -85,8 +87,8 @@
             return false;
         }
 
-        if (this.dropSettings.headDropOnlyInCombat && inCombat) {
-            return true;
+        if (this.dropSettings.headDropOnlyInCombat && !inCombat) {
+            return false;
         }
 
         if (this.dropSettings.headDropChance <= 0.0) {
@@ -106,7 +108,7 @@
                 .replace("{KILLER}", killerName);
 
             meta.setOwningPlayer(player);
             meta.setDisplayName(AdventureUtil.SECTION_SERIALIZER.serialize(miniMessage.deserialize(displayName)));
 
             if (!this.dropSettings.headDropLore.isEmpty()) {
                 List<String> lore = this.dropSettings.headDropLore.stream()
@@ -116,7 +118,7 @@
                     .map(line -> AdventureUtil.SECTION_SERIALIZER.serialize(miniMessage.deserialize(line)))
                     .toList();
 
                 meta.setLore(lore);
             }
 
             meta.addEnchant(Enchantment.LURE, 1, true);
@@ -139,7 +141,13 @@
             ItemStack[] itemsToGive = this.keepInventoryManager.nextItems(playerUniqueId)
                 .toArray(new ItemStack[0]);
 
-            playerInventory.addItem(itemsToGive);
+            // The player is not teleported yet during PlayerRespawnEvent, so
+            // player.getLocation() would still be the death location.
+            Location respawnLocation = event.getRespawnLocation();
+
+            Map<Integer, ItemStack> leftover = playerInventory.addItem(itemsToGive);
+            leftover.values().forEach(item ->
+                respawnLocation.getWorld().dropItemNaturally(respawnLocation, item));
         }
     }
 }

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/knockback/KnockbackRegionController.java

@@ -135,7 +135,7 @@ void onVehicleMove(VehicleMoveEvent event) {
     void onTag(FightTagEvent event) {
         Player player = this.server.getPlayer(event.getPlayer());
         if (player == null) {
-            throw new IllegalStateException("Player cannot be null!");
+            return;
         }
 
         Optional<Region> regionOptional = this.regionProvider.getRegion(player.getLocation());

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/pearl/PearlServiceImpl.java

@@ -59,7 +59,7 @@ public void handleDelay(Player player) {
                 this.scheduler.runLater(
                     () -> player.setCooldown(
                         Material.ENDER_PEARL,
-                        (int) this.pluginConfig.pearl.pearlThrowDelay.toMillis() / 50
+                        (int) (this.pluginConfig.pearl.pearlThrowDelay.toMillis() / 50)
                     ), Duration.ofMillis(50)
                 );
 

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/tagout/FightTagOutServiceImpl.java

@@ -1,14 +1,34 @@
 package com.eternalcode.combat.fight.tagout;
 
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.Expiry;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.UUID;
 
 public class FightTagOutServiceImpl implements FightTagOutService {
 
-    private final Map<UUID, Instant> tagOuts = new HashMap<>();
+    // Self-expiring cache so entries are evicted at their end time instead of
+    // accumulating for every player that has ever tagged out.
+    private final Cache<UUID, Instant> tagOuts = Caffeine.newBuilder()
+        .expireAfter(new Expiry<UUID, Instant>() {
+            @Override
+            public long expireAfterCreate(UUID key, Instant endTime, long currentTime) {
+                return Math.max(0, Duration.between(Instant.now(), endTime).toNanos());
+            }
+
+            @Override
+            public long expireAfterUpdate(UUID key, Instant endTime, long currentTime, long currentDuration) {
+                return this.expireAfterCreate(key, endTime, currentTime);
+            }
+
+            @Override
+            public long expireAfterRead(UUID key, Instant endTime, long currentTime, long currentDuration) {
+                return currentDuration;
+            }
+        })
+        .build();
 
     @Override
     public void tagOut(UUID player, Duration duration) {
@@ -19,12 +39,12 @@ public void tagOut(UUID player, Duration duration) {
 
     @Override
     public void unTagOut(UUID player) {
-        this.tagOuts.remove(player);
+        this.tagOuts.invalidate(player);
     }
 
     @Override
     public boolean isTaggedOut(UUID player) {
-        Instant endTime = this.tagOuts.get(player);
+        Instant endTime = this.tagOuts.getIfPresent(player);
 
         if (endTime == null) {
             return false;

eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/trident/TridentController.java

@@ -119,9 +119,15 @@ public void onRiptide(PlayerRiptideEvent event) {
 
     @EventHandler(ignoreCancelled = true)
     public void onUntag(FightUntagEvent event) {
-        Player player = server.getPlayer(event.getPlayer());
+        UUID playerId = event.getPlayer();
+        this.tridentService.removeDelay(playerId);
+
+        Player player = server.getPlayer(playerId);
+        if (player == null) {
+            return;
+        }
+
         player.setCooldown(Material.TRIDENT, 0);
-        this.tridentService.removeDelay(player.getUniqueId());
     }
 
     private boolean isRiptideInteract(PlayerInteractEvent event) {

eternalcombat-plugin/src/main/java/com/eternalcode/combat/util/InventoryUtil.java

@@ -24,23 +24,46 @@ public static RemoveItemResult removeRandomItems(List<ItemStack> list, int items
         List<ItemStack> currentItems = new ArrayList<>(list);
         List<ItemStack> removedItems = new ArrayList<>();
 
-        int currentItemsToDelete = itemsToDelete;
-        while (currentItemsToDelete > 0) {
-            int randomIndex = RANDOM.nextInt(list.size());
+        int totalAvailable = 0;
+        for (ItemStack item : currentItems) {
+            if (item != null) {
+                totalAvailable += Math.max(0, item.getAmount());
+            }
+        }
+
+        // Never try to remove more than is actually available, otherwise the loop
+        // could never reach zero and would spin forever.
+        int currentItemsToDelete = Math.min(itemsToDelete, totalAvailable);
+
+        while (currentItemsToDelete > 0 && !currentItems.isEmpty()) {
+            int randomIndex = RANDOM.nextInt(currentItems.size());
             ItemStack currentItem = currentItems.get(randomIndex);
 
-            int amount = currentItem.getAmount();
-            int randomAmount = RANDOM.nextInt(0, Math.min(currentItemsToDelete, amount) + 1);
+            // Bukkit inventory lists can contain null entries for empty slots.
+            if (currentItem == null) {
+                currentItems.remove(randomIndex);
+                continue;
+            }
 
-            if (amount <= 0 || randomAmount <= 0) {
+            int amount = currentItem.getAmount();
+            if (amount <= 0) {
+                currentItems.remove(randomIndex);
                 continue;
             }
 
+            int maxRemovable = Math.min(currentItemsToDelete, amount);
+            int randomAmount = RANDOM.nextInt(1, maxRemovable + 1);
+
             ItemStack removedItem = currentItem.clone();
             removedItem.setAmount(randomAmount);
             removedItems.add(removedItem);
 
-            currentItem.setAmount(amount - randomAmount);
+            int remaining = amount - randomAmount;
+            if (remaining <= 0) {
+                currentItems.remove(randomIndex);
+            } else {
+                currentItem.setAmount(remaining);
+            }
 
             currentItemsToDelete -= randomAmount;
         }