From d272cf41f3b85bf7f51bd952f995516fe0f9608e Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:47:41 +0000 Subject: [PATCH 1/3] fix(commands): complete valid command arguments --- .../coreprotect/command/TabHandler.java | 298 ++++++++++------ .../coreprotect/command/TabHandlerTest.java | 325 ++++++++++++++++++ 2 files changed, 516 insertions(+), 107 deletions(-) create mode 100644 src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java diff --git a/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java b/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java index d1d9f63..338c4b3 100755 --- a/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java +++ b/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java @@ -13,17 +13,22 @@ import org.bukkit.World; import org.bukkit.command.Command; import org.bukkit.command.CommandSender; +import org.bukkit.command.ConsoleCommandSender; import org.bukkit.command.TabCompleter; +import org.bukkit.entity.EntityType; import org.bukkit.entity.Player; import org.bukkit.util.StringUtil; +import net.tfminecraft.coreprotect.command.parser.ActionParser; import net.tfminecraft.coreprotect.command.parser.RollbackStateParser; +import net.tfminecraft.coreprotect.config.Config; import net.tfminecraft.coreprotect.config.ConfigHandler; +import net.tfminecraft.coreprotect.model.action.LookupActions; public class TabHandler implements TabCompleter { // private static String[] COMMANDS = new String[] { "help", "inspect", "rollback", "restore", "lookup", "purge", "reload", "status", "near", "undo" }; // max 10! - private static final String[] HELP = new String[] { "inspect", "rollback", "restore", "lookup", "purge", "teleport", "status", "params", "users", "time", "radius", "action", "include", "exclude", "filter" }; + private static final String[] HELP = new String[] { "inspect", "rollback", "restore", "lookup", "purge", "teleport", "reload", "status", "params", "users", "time", "radius", "action", "include", "exclude", "filter" }; private static final String[] PARAMS = new String[] { "user:", "time:", "radius:", "action:", "include:", "exclude:", "filter:", "#container" }; private static final String[] ACTIONS = new String[] { "block", "+block", "-block", "click", "kill", "spawn", "+container", "-container", "container", "chat", "command", "skill", "+inventory", "-inventory", "inventory", "item", "+item", "-item", "sign", "session", "+session", "-session", "ping", "username" }; private static final String[] NUMBERS = new String[] { "0", "1", "2", "3", "4", "5", "6", "7", "8", "9" }; @@ -32,8 +37,8 @@ public class TabHandler implements TabCompleter { @Override public List onTabComplete(CommandSender sender, Command cmd, String label, String[] args) { - if (!(sender instanceof Player) || args.length == 0) { - return null; + if (args.length == 0) { + return List.of(); } if (args.length == 1) { @@ -41,41 +46,61 @@ public List onTabComplete(CommandSender sender, Command cmd, String labe } String argument0 = args[0].toLowerCase(Locale.ROOT); - String currentArg = args[args.length - 1].toLowerCase(Locale.ROOT).trim(); - String lastArg = args.length > 1 ? args[args.length - 2].toLowerCase(Locale.ROOT).trim() : ""; + String currentArg = args[args.length - 1].trim(); + String currentLower = currentArg.toLowerCase(Locale.ROOT); + String lastArg = pendingParameter(args); + if (currentArg.contains(":")) { + lastArg = ""; + } + + if (argument0.equals("teleport") || argument0.equals("tp")) { + if (args.length == 2 && sender instanceof Player && sender.hasPermission("coreprotect.teleport")) { + return handleTeleportCompletions(currentArg); + } + return List.of(); + } + if (argument0.equals("consumer")) { + if (args.length == 2 && sender instanceof ConsoleCommandSender && sender.hasPermission("coreprotect.consumer")) { + return StringUtil.copyPartialMatches(currentArg, List.of("pause", "resume"), new ArrayList<>()); + } + return List.of(); + } + + boolean lookupCommand = hasLookupCommand(argument0, sender); + boolean purgeCommand = argument0.equals("purge") && sender.hasPermission("coreprotect.purge"); ParamState paramState = getParamState(sender, args); // Handle param-specific completions - if (isActionParam(lastArg, currentArg) && hasLookupPermission(sender)) { - return handleActionParamCompletions(currentArg, lastArg); + if (isActionParam(lastArg, currentLower) && lookupCommand) { + return handleActionParamCompletions(sender, argument0, currentArg); } - else if (isUserParam(lastArg, currentArg) && hasLookupPermission(sender)) { - return handleUserParamCompletions(currentArg, lastArg); + else if (isUserParam(lastArg, currentLower) && lookupCommand) { + return completeValues(currentArg, onlineUsers(), true); } - else if (isTimeParam(lastArg, currentArg) && hasTimePermission(sender)) { + else if (isTimeParam(lastArg, currentLower) && (lookupCommand || purgeCommand)) { return handleTimeParamCompletions(currentArg, lastArg); } - else if (isPageParam(lastArg, currentArg) && hasPagePermission(sender)) { + else if (isPageParam(lastArg, currentLower) && (argument0.equals("lookup") || argument0.equals("l") || argument0.equals("page")) && hasPagePermission(sender)) { return handlePageParamCompletions(currentArg, lastArg); } - else if (isRadiusParam(lastArg, currentArg) && hasRadiusPermission(sender)) { + else if (isRadiusParam(lastArg, currentLower) && (lookupCommand || purgeCommand)) { return handleRadiusParamCompletions(currentArg, lastArg, argument0); } - else if (isMaterialParam(lastArg, currentArg) && hasLookupPermission(sender)) { - return handleMaterialParamCompletions(currentArg, lastArg); + else if (isMaterialParam(lastArg, currentLower) && (lookupCommand || purgeCommand)) { + return handleMaterialParamCompletions(currentArg, lastArg, purgeCommand); + } + else if (purgeCommand) { + return handlePurgeCompletions(currentArg, paramState); } else if (args.length == 2) { return handleSecondArgCompletions(sender, argument0, args[1], paramState); } - else if (args.length == 3 && argument0.equals("purge") && sender.hasPermission("coreprotect.purge")) { - return handlePurgeThirdArgCompletions(args[1], args[2]); - } - else if (hasLookupCommand(argument0, sender) && (!argument0.equals("l") && !argument0.equals("lookup") || !paramState.hasPage)) { - return handleGenericLookupCompletions(argument0, currentArg, paramState); + else if (lookupCommand && (!argument0.equals("l") && !argument0.equals("lookup") || !paramState.hasPage)) { + return handleGenericLookupCompletions(argument0, currentArg, paramState, sender instanceof Player); } - return Arrays.asList(""); + return List.of(); } private List getFirstLevelCompletions(CommandSender sender, String argument) { @@ -92,6 +117,20 @@ private List getFirstLevelCompletions(CommandSender sender, String argum addCompletionIfPermitted(sender, "coreprotect.status", "status", completions); addCompletionIfPermitted(sender, "coreprotect.lookup.near", "near", completions); addCompletionIfPermitted(sender, "coreprotect.restore", "undo", completions); + if (sender instanceof Player) { + addCompletionIfPermitted(sender, "coreprotect.teleport", "teleport", completions); + addCompletionIfPermitted(sender, "coreprotect.give", "give", completions); + if (hasActivePreview(sender) && (sender.hasPermission("coreprotect.rollback") || sender.hasPermission("coreprotect.restore"))) { + completions.add("apply"); + completions.add("cancel"); + } + } + if (sender instanceof ConsoleCommandSender) { + addCompletionIfPermitted(sender, "coreprotect.consumer", "consumer", completions); + } + if (Config.getGlobal().NETWORK_DEBUG) { + addCompletionIfPermitted(sender, "coreprotect.networking", "network-debug", completions); + } return StringUtil.copyPartialMatches(arg, completions, new ArrayList<>(completions.size())); } @@ -102,16 +141,9 @@ private void addCompletionIfPermitted(CommandSender sender, String permission, S } } - private boolean hasLookupPermission(CommandSender sender) { - return sender.hasPermission("coreprotect.lookup") || sender.hasPermission("coreprotect.rollback") || sender.hasPermission("coreprotect.restore"); - } - - private boolean hasTimePermission(CommandSender sender) { - return hasLookupPermission(sender) || sender.hasPermission("coreprotect.purge"); - } - - private boolean hasRadiusPermission(CommandSender sender) { - return hasTimePermission(sender); + private boolean hasActivePreview(CommandSender sender) { + List rollback = ConfigHandler.lastRollback.get(sender.getName()); + return rollback != null && rollback.size() > 2 && rollback.get(2) instanceof String[] arguments && Arrays.asList(arguments).contains("#preview"); } private boolean hasPagePermission(CommandSender sender) { @@ -146,6 +178,56 @@ private boolean isMaterialParam(String lastArg, String currentArg) { return lastArg.equals("i:") || lastArg.equals("include:") || lastArg.equals("item:") || lastArg.equals("items:") || lastArg.equals("b:") || lastArg.equals("block:") || lastArg.equals("blocks:") || currentArg.startsWith("i:") || currentArg.startsWith("include:") || currentArg.startsWith("item:") || currentArg.startsWith("items:") || currentArg.startsWith("b:") || currentArg.startsWith("block:") || currentArg.startsWith("blocks:") || lastArg.equals("e:") || lastArg.equals("exclude:") || currentArg.startsWith("e:") || currentArg.startsWith("exclude:"); } + // Only user/material lists continue across spaces after a comma. Action lists do not. + private String pendingParameter(String[] args) { + String pending = ""; + for (int index = 1; index < args.length - 1; index++) { + String token = args[index].toLowerCase(Locale.ROOT).trim(); + int colon = token.indexOf(':'); + String marker = colon >= 0 ? token.substring(0, colon + 1) : pending; + String value = colon >= 0 ? token.substring(colon + 1) : token; + boolean list = isUserParam("", marker) || isMaterialParam("", marker); + boolean parameter = list || isActionParam("", marker) || isTimeParam("", marker) || isRadiusParam("", marker) || isPageParam("", marker); + pending = parameter && ((colon >= 0 && value.isEmpty()) || (list && value.endsWith(","))) ? marker : ""; + } + return pending; + } + + private List onlineUsers() { + return Bukkit.getOnlinePlayers().stream().map(Player::getName).collect(Collectors.toList()); + } + + // Bukkit replaces the current space-delimited token, including its marker/list prefix. + private List completeValues(String currentArg, List values, boolean allowList) { + int colon = currentArg.indexOf(':'); + int start = colon + 1; + if (allowList) { + start = Math.max(start, currentArg.lastIndexOf(',') + 1); + } + else if (currentArg.indexOf(',') >= 0) { + return List.of(); + } + String prefix = currentArg.substring(0, start); + String partial = currentArg.substring(start); + List matches = StringUtil.copyPartialMatches(partial, values, new ArrayList<>()); + return matches.stream().distinct().map(value -> prefix + value).collect(Collectors.toList()); + } + + private List handleTeleportCompletions(String currentArg) { + List worlds = new ArrayList<>(); + for (World world : Bukkit.getWorlds()) { + boolean singleToken = world.getName().chars().noneMatch(Character::isWhitespace); + if (singleToken) { + worlds.add(world.getName()); + } + Integer id = ConfigHandler.worlds.get(world.getName()); + if (id != null && (!singleToken || currentArg.startsWith("wid:"))) { + worlds.add("wid:" + id); + } + } + return StringUtil.copyPartialMatches(currentArg, worlds, new ArrayList<>()); + } + private static class ParamState { boolean hasUser; boolean hasAction; @@ -159,6 +241,9 @@ private static class ParamState { boolean hasSummary; boolean hasRollbackState; boolean hasPreview; + boolean hasVerbose; + boolean hasSilent; + boolean hasOptimize; boolean hasPage; boolean validContainer; boolean pageLookup; @@ -172,7 +257,7 @@ private ParamState getParamState(CommandSender sender, String[] args) { state.pageLookup = true; } - for (int i = 1; i < args.length; i++) { + for (int i = 1; i < args.length - 1; i++) { String arg = args[i].toLowerCase(Locale.ROOT); if (arg.equals("#container")) { state.hasContainer = true; @@ -189,6 +274,15 @@ else if (RollbackStateParser.isModifier(arg)) { else if (arg.equals("#preview")) { state.hasPreview = true; } + else if (arg.equals("#verbose") || arg.equals("#v") || arg.equals("verbose") || arg.equals("v") || arg.equals("noisy") || arg.equals("n")) { + state.hasVerbose = true; + } + else if (arg.equals("#silent")) { + state.hasSilent = true; + } + else if (arg.equals("#optimize")) { + state.hasOptimize = true; + } else if (arg.startsWith("f:") || arg.startsWith("filter:")) { state.hasFilter = true; } @@ -233,44 +327,31 @@ else if (lookupType == 5) { return state; } - private List handleActionParamCompletions(String currentArg, String lastArg) { - String arg = ""; - String filter = lastArg; - - if (currentArg.contains(":")) { - String[] split = currentArg.split(":", 2); - filter = split[0] + ":"; - if (split.length > 1) { - arg = split[1]; + private List handleActionParamCompletions(CommandSender sender, String command, String currentArg) { + boolean lookup = command.equals("lookup") || command.equals("l"); + List actions = new ArrayList<>(); + for (String action : ACTIONS) { + String category = action.replaceFirst("^[+-]", ""); + if (lookup) { + String permission = category.equals("skill") ? "command" : category.equals("ping") ? "session" : category; + if (!sender.hasPermission("coreprotect.lookup." + permission)) { + continue; + } } + else if (!List.of("block", "kill", "spawn", "container", "inventory").contains(category)) { + continue; + } + actions.add(action); } - - List completions = new ArrayList<>(Arrays.asList(ACTIONS)); - for (int index = 0; index < completions.size(); index++) { - completions.set(index, filter + completions.get(index)); + List completions = completeValues(currentArg, actions, lookup); + if (lookup) { + completions.removeIf(value -> { + String argument = value.contains(":") ? value : "a:" + value; + ActionParser.ParseResult parsed = ActionParser.parseActions(new String[] { "lookup", argument }, true); + return parsed.isSkillLookup() && parsed.getActions().stream().anyMatch(action -> action != LookupActions.COMMAND); + }); } - return StringUtil.copyPartialMatches(filter + arg, completions, new ArrayList<>(completions.size())); - } - - private List handleUserParamCompletions(String currentArg, String lastArg) { - if (lastArg.equals("u:") || lastArg.equals("user:") || lastArg.equals("users:") || lastArg.equals("p:")) { - return null; - } - - String arg = ""; - String[] split = currentArg.split(":", 2); - String filter = split[0] + ":"; - if (split.length > 1) { - arg = split[1]; - } - - List completions = Bukkit.getOnlinePlayers().stream().map(Player::getName).collect(Collectors.toList()); - - for (int index = 0; index < completions.size(); index++) { - completions.set(index, filter + completions.get(index)); - } - - return StringUtil.copyPartialMatches(filter + arg, completions, new ArrayList<>(completions.size())); + return completions; } private List handleTimeParamCompletions(String currentArg, String lastArg) { @@ -339,7 +420,7 @@ private List handlePageParamCompletions(String currentArg, String lastAr return StringUtil.copyPartialMatches(filter + arg, completions, new ArrayList<>(completions.size())); } - return null; + return List.of(); } private List handleRadiusParamCompletions(String currentArg, String lastArg, String baseCommand) { @@ -383,31 +464,30 @@ else if (baseCommand.equals("purge") || arg.startsWith("#")) { return StringUtil.copyPartialMatches(filter + arg, completions, new ArrayList<>(completions.size())); } - return null; + return List.of(); } - private List handleMaterialParamCompletions(String currentArg, String lastArg) { - String filter = lastArg; - String arg = ""; - if (currentArg.contains(":")) { - String[] split = currentArg.split(":", 2); - filter = split[0] + ":"; - if (split.length > 1) { - arg = split[1]; - } + private List handleMaterialParamCompletions(String currentArg, String lastArg, boolean purge) { + String marker = currentArg.contains(":") ? currentArg.substring(0, currentArg.indexOf(':') + 1).toLowerCase(Locale.ROOT) : lastArg; + boolean exclude = marker.equals("e:") || marker.equals("exclude:"); + if (purge && exclude) { + return List.of(); } - else { - filter = ""; - arg = currentArg; - } - initializeMaterialsIfNeeded(); - List completions = new ArrayList<>(materials); - for (int index = 0; index < completions.size(); index++) { - completions.set(index, filter + completions.get(index)); + if (!purge) { + // The parser only accepts entity types already recorded in its identifier cache. + for (EntityType type : EntityType.values()) { + String name = type.name().toLowerCase(Locale.ROOT); + if (type != EntityType.UNKNOWN && ConfigHandler.entities.containsKey(name)) { + completions.add(name); + } + } + if (exclude) { + completions.addAll(onlineUsers()); + } } - return StringUtil.copyPartialMatches(filter + arg, completions, new ArrayList<>(completions.size())); + return completeValues(currentArg, completions, true); } private void initializeMaterialsIfNeeded() { @@ -446,41 +526,39 @@ private List handleSecondArgCompletions(CommandSender sender, String cmd List completions = new ArrayList<>(Arrays.asList(HELP)); return StringUtil.copyPartialMatches(argument1, completions, new ArrayList<>(completions.size())); } - else if (argument0.equals("purge") && sender.hasPermission("coreprotect.purge")) { - List completions = new ArrayList<>(Arrays.asList("t:", "r:", "i:")); - return StringUtil.copyPartialMatches(argument1, completions, new ArrayList<>(completions.size())); - } else if (hasLookupCommand(argument0, sender)) { - List completions = new ArrayList<>(filterParams(true, argument0, argument1, paramState)); + List completions = new ArrayList<>(filterParams(true, argument0, argument1, paramState, sender instanceof Player)); completions.addAll(Bukkit.getOnlinePlayers().stream().filter(player -> player.getName().toLowerCase(Locale.ROOT).startsWith(argument1)).map(Player::getName).collect(Collectors.toList())); return StringUtil.copyPartialMatches(argument1, completions, new ArrayList<>(completions.size())); } - return null; + return List.of(); } - private List handlePurgeThirdArgCompletions(String arg1, String arg2) { - String argument1 = arg1.toLowerCase(Locale.ROOT); - String argument2 = arg2.toLowerCase(Locale.ROOT); - - if (argument1.startsWith("t:")) { - List completions = new ArrayList<>(Arrays.asList("r:", "i:")); - return StringUtil.copyPartialMatches(argument2, completions, new ArrayList<>(completions.size())); + private List handlePurgeCompletions(String currentArg, ParamState state) { + List params = new ArrayList<>(); + if (!state.hasTime) { + params.addAll(List.of("t:", "time:")); + } + if (!state.hasRadius) { + params.addAll(List.of("r:", "radius:")); + } + if (!state.hasInclude) { + params.addAll(List.of("i:", "include:")); } - else if (argument1.startsWith("r:") || argument1.startsWith("i:")) { - List completions = new ArrayList<>(Arrays.asList("t:")); - return StringUtil.copyPartialMatches(argument2, completions, new ArrayList<>(completions.size())); + if (!state.hasOptimize) { + params.add("#optimize"); } - return Arrays.asList(""); + return StringUtil.copyPartialMatches(currentArg, params, new ArrayList<>()); } - private List handleGenericLookupCompletions(String cmd, String currentArg, ParamState paramState) { - ArrayList params = filterParams(false, cmd, currentArg, paramState); + private List handleGenericLookupCompletions(String cmd, String currentArg, ParamState paramState, boolean player) { + ArrayList params = filterParams(false, cmd, currentArg, paramState, player); List completions = new ArrayList<>(params); return StringUtil.copyPartialMatches(currentArg, completions, new ArrayList<>(completions.size())); } - private ArrayList filterParams(boolean firstParam, String lastArgument, String argument, ParamState state) { + private ArrayList filterParams(boolean firstParam, String lastArgument, String argument, ParamState state, boolean player) { ArrayList params = new ArrayList<>(); for (String param : PARAMS) { if (param.equals("user:") && !state.hasUser) { @@ -519,9 +597,15 @@ else if (param.equals("#container") && !state.hasContainer && !state.hasRadius & params.add("#rolledback"); params.add("#restored"); } - if (!state.hasPreview) { + if (!state.hasPreview && player && !lastArgument.equals("lookup") && !lastArgument.equals("l")) { params.add("#preview"); } + if (!state.hasVerbose) { + params.add("#verbose"); + } + if (!state.hasSilent) { + params.add("#silent"); + } } return params; diff --git a/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java b/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java new file mode 100644 index 0000000..82ae573 --- /dev/null +++ b/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java @@ -0,0 +1,325 @@ +package net.tfminecraft.coreprotect.command; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.bukkit.Bukkit; +import org.bukkit.Material; +import org.bukkit.Server; +import org.bukkit.World; +import org.bukkit.command.Command; +import org.bukkit.command.CommandSender; +import org.bukkit.command.ConsoleCommandSender; +import org.bukkit.entity.Player; +import org.bukkit.plugin.PluginManager; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedStatic; + +import net.tfminecraft.coreprotect.command.parser.ActionParser; +import net.tfminecraft.coreprotect.command.parser.UserParser; +import net.tfminecraft.coreprotect.config.Config; +import net.tfminecraft.coreprotect.config.ConfigHandler; + +class TabHandlerTest { + private final TabHandler handler = new TabHandler(); + private final Set permissions = new HashSet<>(); + private Player player; + private ConsoleCommandSender console; + private MockedStatic bukkit; + private MockedStatic config; + private MockedStatic materialTypes; + private Config settings; + + @BeforeEach + void setUp() { + player = sender(Player.class, "TabTester"); + console = sender(ConsoleCommandSender.class, "Console"); + Player alice = mock(Player.class); + Player bob = mock(Player.class); + when(alice.getName()).thenReturn("Alice"); + when(bob.getName()).thenReturn("Bob"); + World world = mock(World.class); + when(world.getName()).thenReturn("World"); + Server server = mock(Server.class); + when(server.getWorlds()).thenReturn(List.of(world)); + when(server.getPluginManager()).thenReturn(mock(PluginManager.class)); + bukkit = mockStatic(Bukkit.class); + bukkit.when(Bukkit::getServer).thenReturn(server); + bukkit.when(Bukkit::getWorlds).thenReturn(List.of(world)); + bukkit.when(Bukkit::getOnlinePlayers).thenReturn(List.of(alice, bob)); + settings = mock(Config.class); + config = mockStatic(Config.class); + config.when(Config::getGlobal).thenReturn(settings); + + // Material registry access requires a server; use a small block/item fixture. + Material stone = material("STONE", true); + Material diamond = material("DIAMOND", false); + materialTypes = mockStatic(Material.class, CALLS_REAL_METHODS); + materialTypes.when(Material::values).thenReturn(new Material[] { stone, diamond }); + ConfigHandler.entities.put("zombie", 1); + ConfigHandler.entities.put("invalid_entity", 2); + ConfigHandler.worlds.put("World", 7); + ConfigHandler.worlds.put("unloaded", 8); + ConfigHandler.lookupType.remove("TabTester"); + ConfigHandler.lookupPage.remove("TabTester"); + ConfigHandler.lastRollback.remove("TabTester"); + } + + @AfterEach + void tearDown() { + materialTypes.close(); + config.close(); + bukkit.close(); + ConfigHandler.entities.clear(); + ConfigHandler.worlds.clear(); + ConfigHandler.lastRollback.remove("TabTester"); + } + + @ParameterizedTest + @ValueSource(strings = { "co", "core", "coreprotect" }) + void canonicalCommandsRespectPermissionsAndContextForEveryRoot(String root) { + permissions.addAll(List.of("coreprotect.teleport", "coreprotect.give", "coreprotect.rollback", "coreprotect.networking")); + settings.NETWORK_DEBUG = true; + ConfigHandler.lastRollback.put("TabTester", Arrays.asList(0L, 0L, new String[] { "rollback", "#preview" })); + assertEquals(List.of("teleport"), complete(player, root, "TeLe")); + assertEquals(List.of("give"), complete(player, root, "gi")); + assertEquals(List.of("apply"), complete(player, root, "app")); + assertEquals(List.of("cancel"), complete(player, root, "can")); + assertEquals(List.of("network-debug"), complete(player, root, "network-")); + ConfigHandler.lastRollback.remove("TabTester"); + assertEquals(List.of(), complete(player, root, "app")); + settings.NETWORK_DEBUG = false; + assertEquals(List.of(), complete(player, root, "network-")); + permissions.clear(); + assertEquals(List.of(), complete(player, root, "")); + } + + @Test + void consoleGetsSupportedCommandsAndConsumerControlsOnly() { + permissions.addAll(List.of("coreprotect.consumer", "coreprotect.help", "coreprotect.lookup", "coreprotect.purge", "coreprotect.teleport", "coreprotect.give")); + assertEquals(List.of("consumer"), complete(console, "co", "con")); + assertEquals(List.of("pause"), complete(console, "co", "consumer", "PA")); + assertEquals(List.of("resume"), complete(console, "co", "consumer", "re")); + assertTrue(complete(console, "co", "").containsAll(List.of("help", "lookup", "purge"))); + assertFalse(complete(console, "co", "").contains("teleport")); + assertFalse(complete(console, "co", "").contains("give")); + assertEquals(List.of(), complete(player, "co", "con")); + assertEquals(List.of(), complete(player, "co", "consumer", "")); + assertEquals(List.of(), complete(console, "co", "consumer", "pause", "")); + permissions.remove("coreprotect.consumer"); + assertEquals(List.of(), complete(console, "co", "consumer", "pa")); + } + + @ParameterizedTest + @ValueSource(strings = { "teleport", "tp" }) + void teleportCompletesLoadedWorldsAndLeavesCoordinatesEmpty(String verb) { + permissions.add("coreprotect.teleport"); + assertEquals(List.of("World"), complete(player, "co", verb, "wO")); + assertEquals(List.of("wid:7"), complete(player, "co", verb, "wid:")); + assertEquals(List.of(), complete(player, "co", verb, "unloaded")); + assertEquals(List.of(), complete(player, "co", verb, "World", "")); + assertEquals(List.of(), complete(player, "co", verb, "World", "1", "a:ch")); + assertEquals(List.of(), complete(console, "co", verb, "")); + permissions.clear(); + assertEquals(List.of(), complete(player, "co", verb, "w")); + } + + @Test + void actionValuesReplaceOnlyTheCurrentInlineOrWhitespaceToken() { + permissions.addAll(List.of("coreprotect.lookup", "coreprotect.lookup.chat", "coreprotect.rollback")); + assertEquals(List.of("chat"), tab("lookup", "a:", "Ch")); + assertEquals(List.of("a:chat"), tab("lookup", "a:Ch")); + assertEquals(List.of("block"), tab("rollback", "action:", "BL")); + assertTrue(tab("rollback", "action:", "").contains("block")); + assertFalse(tab("rollback", "action:", "").contains("action:block")); + assertEquals(List.of("i:diamond"), tab("rollback", "action:", "i:di")); + } + + @Test + void teleportUsesAnExistingWorldIdWhenTheWorldNameContainsSpaces() { + permissions.add("coreprotect.teleport"); + World world = mock(World.class); + when(world.getName()).thenReturn("World With Spaces"); + bukkit.when(Bukkit::getWorlds).thenReturn(List.of(world)); + ConfigHandler.worlds.put("World With Spaces", 9); + assertEquals(List.of("wid:9"), tab("teleport", "")); + assertEquals(List.of("wid:9"), tab("teleport", "wid:")); + assertEquals(List.of(), tab("teleport", "World", "With")); + } + + @Test + void commaListsPreserveCompletedTextAndUseTheFinalPrefix() { + permissions.addAll(List.of("coreprotect.lookup", "coreprotect.lookup.block", "coreprotect.lookup.chat")); + assertEquals(List.of("u:Alice,Bob"), tab("lookup", "u:Alice,Bo")); + assertEquals(List.of("Alice,Bob"), tab("lookup", "u:", "Alice,bO")); + assertEquals(List.of("Bob"), tab("lookup", "u:Alice,", "Bo")); + assertEquals(List.of("a:block,chat"), tab("lookup", "a:block,CH")); + assertEquals(List.of("block,chat"), tab("lookup", "action:", "block,ch")); + assertEquals(List.of("i:stone,diamond"), tab("lookup", "i:stone,di")); + assertEquals(List.of("e:stone,diamond"), tab("lookup", "e:stone,di")); + assertEquals(List.of("diamond"), tab("lookup", "i:stone,", "di")); + assertEquals(List.of("diamond"), tab("lookup", "include:", "stone,", "di")); + assertEquals(List.of("Bob"), tab("lookup", "user:", "Alice,", "bo")); + assertTrue(ActionParser.parseAction(new String[] { "lookup", "a:block,chat" }, true).size() > 2); + assertEquals(List.of("alice", "bob"), UserParser.parseUsers(new String[] { "lookup", "u:Alice,Bob" })); + permissions.add("coreprotect.rollback"); + assertEquals(List.of(), tab("rollback", "a:block,bl")); + } + + @Test + void materialsIncludeKnownEntitiesAndExclusionsIncludePlayers() { + permissions.addAll(List.of("coreprotect.lookup", "coreprotect.lookup.kill")); + assertTrue(tab("lookup", "a:kill", "i:zomb").contains("i:zombie")); + assertTrue(tab("lookup", "a:kill", "e:zomb").contains("e:zombie")); + assertEquals(List.of("e:Bob"), tab("lookup", "e:Bo")); + assertEquals(List.of("Bob"), tab("lookup", "exclude:", "Bo")); + assertEquals(List.of("e:stone,Bob"), tab("lookup", "e:stone,Bo")); + assertEquals(List.of(), tab("lookup", "i:Bo")); + assertEquals(List.of(), tab("lookup", "i:invalid_entity")); + ConfigHandler.entities.remove("zombie"); + assertFalse(tab("lookup", "i:zomb").contains("i:zombie")); + } + + @Test + void purgeOnlyPermissionsCompleteIncludeValuesAndAllParameterOrders() { + permissions.add("coreprotect.purge"); + assertEquals(List.of("i:diamond"), tab("purge", "i:di")); + assertEquals(List.of("i:diamond"), tab("purge", "t:30d", "i:di")); + assertTrue(tab("purge", "time:30d", "r").containsAll(List.of("r:", "radius:"))); + assertTrue(tab("purge", "t:30d", "r:#World", "i").containsAll(List.of("i:", "include:"))); + assertTrue(tab("purge", "include:stone", "radius:#World", "t").containsAll(List.of("t:", "time:"))); + assertEquals(List.of("#optimize"), tab("purge", "#opt")); + assertEquals(List.of(), tab("purge", "#optimize", "#opt")); + assertEquals(List.of(), tab("purge", "time:30d", "t")); + assertEquals(List.of("diamond"), tab("purge", "include:", "stone,", "di")); + assertEquals(List.of("r:#World"), tab("purge", "r:#wo")); + assertEquals(List.of(), tab("purge", "i:Bo")); + assertEquals(List.of(), tab("purge", "e:Bo")); + assertEquals(List.of(), tab("purge", "a:ch")); + assertEquals(List.of(), tab("purge", "unknown")); + permissions.clear(); + permissions.add("coreprotect.lookup"); + assertEquals(List.of(), tab("purge", "i:di")); + } + + @Test + void actionsUseTheSelectedCommandAndSpecificLookupPermissions() { + permissions.addAll(List.of("coreprotect.lookup", "coreprotect.rollback", "coreprotect.purge")); + assertEquals(List.of(), tab("lookup", "a:")); + permissions.add("coreprotect.lookup.chat"); + assertEquals(List.of("a:chat"), tab("lookup", "a:ch")); + assertEquals(List.of(), tab("rollback", "a:ch")); + assertEquals(List.of(), tab("purge", "a:ch")); + assertEquals(List.of(), tab("help", "a:ch")); + assertEquals(List.of(), tab("restore", "i:di")); + assertEquals(List.of(), tab("rollback", "a:item")); + assertEquals(List.of("a:inventory"), tab("rollback", "a:inv")); + permissions.add("coreprotect.lookup.inventory"); + assertEquals(List.of("a:inventory"), tab("lookup", "a:inv")); + assertEquals(List.of(), tab("lookup", "a:item")); + permissions.add("coreprotect.lookup.command"); + assertEquals(List.of("a:skill"), tab("lookup", "a:sk")); + assertEquals(List.of(), tab("lookup", "a:skill,ch")); + } + + @ParameterizedTest + @ValueSource(strings = { "lookup", "l", "rollback", "rb", "ro", "restore", "rs", "re" }) + void lookupFamilyAliasesRetainInlineWhitespaceAndPermissionBehavior(String verb) { + String permission = List.of("lookup", "l").contains(verb) ? "lookup" : List.of("rollback", "rb", "ro").contains(verb) ? "rollback" : "restore"; + permissions.add("coreprotect." + permission); + permissions.add("coreprotect.lookup.block"); + assertEquals(List.of("a:block"), tab(verb, "a:bl")); + assertEquals(List.of("block"), tab(verb, "action:", "bl")); + assertEquals(List.of("i:stone,diamond"), tab(verb, "i:stone,di")); + assertEquals(List.of("Bob"), tab(verb, "user:", "bo")); + assertEquals(List.of("time:"), tab(verb, "ti")); + permissions.remove("coreprotect." + permission); + assertEquals(List.of(), tab(verb, "a:bl")); + assertEquals(List.of(), tab(verb, "i:di")); + } + + @Test + void eachLookupActionUsesItsOwnPermissionIncludingInventoryAndSessionVariants() { + permissions.add("coreprotect.lookup"); + List> cases = List.of( + List.of("coreprotect.lookup.block", "a:block", "a:+block", "a:-block"), + List.of("coreprotect.lookup.click", "a:click"), + List.of("coreprotect.lookup.kill", "a:kill"), + List.of("coreprotect.lookup.spawn", "a:spawn"), + List.of("coreprotect.lookup.container", "a:+container", "a:-container", "a:container"), + List.of("coreprotect.lookup.chat", "a:chat"), + List.of("coreprotect.lookup.command", "a:command", "a:skill"), + List.of("coreprotect.lookup.inventory", "a:+inventory", "a:-inventory", "a:inventory"), + List.of("coreprotect.lookup.item", "a:item", "a:+item", "a:-item"), + List.of("coreprotect.lookup.sign", "a:sign"), + List.of("coreprotect.lookup.session", "a:session", "a:+session", "a:-session", "a:ping"), + List.of("coreprotect.lookup.username", "a:username")); + for (List permissionCase : cases) { + permissions.add(permissionCase.getFirst()); + assertEquals(permissionCase.subList(1, permissionCase.size()), tab("lookup", "a:")); + permissions.remove(permissionCase.getFirst()); + assertEquals(List.of(), tab("lookup", "a:")); + } + } + + @Test + void verbosityModifiersAndReloadHelpAreFilteredAndDeduplicated() { + permissions.addAll(List.of("coreprotect.help", "coreprotect.lookup", "coreprotect.rollback", "coreprotect.purge")); + assertEquals(List.of("reload"), tab("help", "rEL")); + assertEquals(List.of("#silent"), tab("rollback", "t:1h", "#si")); + assertEquals(List.of("#silent"), tab("rollback", "t:1h", "#silent")); + assertEquals(List.of("#verbose"), tab("lookup", "t:1h", "#ve")); + assertEquals(List.of(), tab("lookup", "#verbose", "#ve")); + assertEquals(List.of(), tab("rollback", "#silent", "#si")); + assertEquals(List.of(), tab("purge", "#ve")); + assertEquals(List.of(), complete(console, "co", "rollback", "#pr")); + } + + @Test + void emptyOrUnhandledArgumentsNeverReturnNullOrAnEmptyStringCandidate() { + permissions.add("coreprotect.lookup"); + assertEquals(List.of(), complete(player, "co")); + assertEquals(List.of(), tab("unknown", "")); + assertEquals(List.of(), tab("give", "")); + assertEquals(List.of(), tab("lookup", "page:x")); + assertEquals(List.of("Bob"), tab("lookup", "u:", "Bo")); + } + + private List tab(String... args) { + return complete(player, "co", args); + } + + private List complete(CommandSender sender, String root, String... args) { + Command command = mock(Command.class); + when(command.getName()).thenReturn(root); + List result = handler.onTabComplete(sender, command, root, args); + assertNotNull(result, "Explicit completion result for " + Arrays.toString(args)); + assertFalse(result.contains(""), "No blank candidate for " + Arrays.toString(args)); + return result; + } + + private T sender(Class type, String name) { + T sender = mock(type); + when(sender.getName()).thenReturn(name); + when(sender.hasPermission(anyString())).thenAnswer(call -> permissions.contains(call.getArgument(0))); + return sender; + } + + private Material material(String name, boolean block) { + Material material = mock(Material.class); + when(material.name()).thenReturn(name); + when(material.isBlock()).thenReturn(block); + when(material.isItem()).thenReturn(true); + return material; + } +} From e9c43a9e4daf8b70aa6b1ce5d8cd85f8562133bc Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:55:44 +0000 Subject: [PATCH 2/3] fix: match world ID completion prefixes without case sensitivity --- .../java/net/tfminecraft/coreprotect/command/TabHandler.java | 2 +- .../net/tfminecraft/coreprotect/command/TabHandlerTest.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java b/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java index 338c4b3..db70e2f 100755 --- a/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java +++ b/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java @@ -221,7 +221,7 @@ private List handleTeleportCompletions(String currentArg) { worlds.add(world.getName()); } Integer id = ConfigHandler.worlds.get(world.getName()); - if (id != null && (!singleToken || currentArg.startsWith("wid:"))) { + if (id != null && (!singleToken || StringUtil.startsWithIgnoreCase(currentArg, "wid:"))) { worlds.add("wid:" + id); } } diff --git a/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java b/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java index 82ae573..ff65094 100644 --- a/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java +++ b/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java @@ -126,6 +126,7 @@ void teleportCompletesLoadedWorldsAndLeavesCoordinatesEmpty(String verb) { permissions.add("coreprotect.teleport"); assertEquals(List.of("World"), complete(player, "co", verb, "wO")); assertEquals(List.of("wid:7"), complete(player, "co", verb, "wid:")); + assertEquals(List.of("wid:7"), complete(player, "co", verb, "WID:")); assertEquals(List.of(), complete(player, "co", verb, "unloaded")); assertEquals(List.of(), complete(player, "co", verb, "World", "")); assertEquals(List.of(), complete(player, "co", verb, "World", "1", "a:ch")); From 30d599a4a900a0afb5ac329c20275f73692dc9a4 Mon Sep 17 00:00:00 2001 From: Ryan Barlow <7389646+ryanbarlow97@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:29:36 +0000 Subject: [PATCH 3/3] fix: restrict near completion to located senders --- .../coreprotect/command/TabHandler.java | 6 +++++- .../coreprotect/command/TabHandlerTest.java | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java b/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java index db70e2f..c09e422 100755 --- a/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java +++ b/src/main/java/net/tfminecraft/coreprotect/command/TabHandler.java @@ -11,6 +11,7 @@ import org.bukkit.Bukkit; import org.bukkit.Material; import org.bukkit.World; +import org.bukkit.command.BlockCommandSender; import org.bukkit.command.Command; import org.bukkit.command.CommandSender; import org.bukkit.command.ConsoleCommandSender; @@ -115,7 +116,10 @@ private List getFirstLevelCompletions(CommandSender sender, String argum addCompletionIfPermitted(sender, "coreprotect.purge", "purge", completions); addCompletionIfPermitted(sender, "coreprotect.reload", "reload", completions); addCompletionIfPermitted(sender, "coreprotect.status", "status", completions); - addCompletionIfPermitted(sender, "coreprotect.lookup.near", "near", completions); + if ((sender instanceof Player || sender instanceof BlockCommandSender) + && (sender.hasPermission("coreprotect.lookup") || sender.hasPermission("coreprotect.lookup.near"))) { + completions.add("near"); + } addCompletionIfPermitted(sender, "coreprotect.restore", "undo", completions); if (sender instanceof Player) { addCompletionIfPermitted(sender, "coreprotect.teleport", "teleport", completions); diff --git a/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java b/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java index ff65094..bcbd8b8 100644 --- a/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java +++ b/src/test/java/net/tfminecraft/coreprotect/command/TabHandlerTest.java @@ -13,6 +13,7 @@ import org.bukkit.Material; import org.bukkit.Server; import org.bukkit.World; +import org.bukkit.command.BlockCommandSender; import org.bukkit.command.Command; import org.bukkit.command.CommandSender; import org.bukkit.command.ConsoleCommandSender; @@ -120,6 +121,21 @@ void consoleGetsSupportedCommandsAndConsumerControlsOnly() { assertEquals(List.of(), complete(console, "co", "consumer", "pa")); } + @Test + void nearRequiresALocatedSenderAndAcceptsEitherExecutionPermission() { + BlockCommandSender block = sender(BlockCommandSender.class, "CommandBlock"); + for (String permission : List.of("coreprotect.lookup", "coreprotect.lookup.near")) { + permissions.clear(); + permissions.add(permission); + assertEquals(List.of("near"), complete(player, "co", "nE")); + assertEquals(List.of("near"), complete(block, "co", "nE")); + assertEquals(List.of(), complete(console, "co", "nE")); + } + permissions.clear(); + assertEquals(List.of(), complete(player, "co", "near")); + assertEquals(List.of(), complete(block, "co", "near")); + } + @ParameterizedTest @ValueSource(strings = { "teleport", "tp" }) void teleportCompletesLoadedWorldsAndLeavesCoordinatesEmpty(String verb) {