Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public boolean refresh() {
BuiltinCommandSnapshot next = BuiltinCommandDiscovery.discover(
effectiveCommands,
serverAliases,
settings
settings,
feature.getLifecycleManager().getCommandManager()::isLabelOwnedByServerFeatures
);

if (settings.removeFromCommandMap()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.Predicate;

public final class BuiltinCommandDiscovery {

Expand All @@ -31,6 +32,15 @@ public static BuiltinCommandSnapshot discover(
Map<String, Command> knownCommands,
Map<String, String[]> serverAliases,
BuiltinCommandBlockerSettings settings
) {
return discover(knownCommands, serverAliases, settings, ignored -> false);
}

public static BuiltinCommandSnapshot discover(
Map<String, Command> knownCommands,
Map<String, String[]> serverAliases,
BuiltinCommandBlockerSettings settings,
Predicate<String> serverFeaturesOwnedLabel
) {
IdentityHashMap<Command, List<String>> registrations = new IdentityHashMap<>();
knownCommands.forEach((key, command) -> {
Expand All @@ -44,6 +54,14 @@ public static BuiltinCommandSnapshot discover(
LinkedHashMap<String, Integer> detectedSources = emptySourceCounts();

registrations.forEach((command, keys) -> {
// Paper can expose custom Brigadier roots through VanillaCommandWrapper. An unnamespaced registration
// claimed by ServerFeatures is authoritative ownership evidence and must win over wrapper/package
// heuristics. Only plain registrations are considered here so displaced built-in fallbacks such as
// minecraft:restart remain independently discoverable and blockable.
if (hasServerFeaturesOwnedRegistration(keys, serverFeaturesOwnedLabel)) {
return;
}

BuiltinCommandSource source = classify(command, keys);
if (source == null || !settings.blocks(source) || isAllowed(command, keys, source, settings)) {
return;
Expand Down Expand Up @@ -79,14 +97,14 @@ public static BuiltinCommandSnapshot discover(
static BuiltinCommandSource classify(Command command, Collection<String> registrationKeys) {
String className = command.getClass().getName().toLowerCase(Locale.ROOT);

// Modern Paper command registrations are exposed through PluginVanillaCommandWrapper, which lives in
// io.papermc.paper.*. Plugin ownership therefore has to win over implementation-package heuristics.
// Plugin ownership is stronger evidence than Paper implementation packages or fallback namespaces.
// Spark is the one intentional exception because Paper bundles it as a platform command scope.
if (command instanceof PluginIdentifiableCommand identifiable) {
String pluginName = identifiable.getPlugin().getName().toLowerCase(Locale.ROOT);
if (pluginName.equals("spark") || pluginName.equals("spark-paper")) {
return BuiltinCommandSource.SPARK;
}
return sourceFromRegistrationNamespace(registrationKeys);
return null;
}

if (isBundledSparkCommand(command, className)) {
Expand Down Expand Up @@ -130,6 +148,18 @@ static boolean isAliasRegistration(Command command, String registrationKey) {
return true;
}

private static boolean hasServerFeaturesOwnedRegistration(
Collection<String> registrationKeys,
Predicate<String> serverFeaturesOwnedLabel
) {
for (String key : registrationKeys) {
if (!key.isEmpty() && key.indexOf(':') < 0 && serverFeaturesOwnedLabel.test(key)) {
return true;
}
}
return false;
}

private static void addBlockedServerAliases(
Map<String, Command> knownCommands,
Map<String, String[]> serverAliases,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,13 @@ public Set<String> getAllRegisteredCommandNames() {
return Collections.unmodifiableSet(names);
}

/**
* Returns whether a command label is currently claimed by any ServerFeatures feature.
*/
public boolean isLabelOwnedByServerFeatures(String label) {
return ownership.isClaimed(label);
}

public Map<String, FeatureCommand> getRegisteredFeatureCommands() {
return Map.copyOf(registeredCommands);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ synchronized void release(Object owner, Collection<String> labels) {
}
}

synchronized boolean isClaimed(String label) {
return label != null && ownersByLabel.containsKey(normalize(label));
}

private static String normalize(String label) {
return label.toLowerCase(Locale.ROOT);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package nl.hauntedmc.serverfeatures.features.builtincommandblocker.internal;

import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;
import org.bukkit.command.PluginIdentifiableCommand;
import org.bukkit.plugin.Plugin;
import org.jetbrains.annotations.NotNull;
import org.junit.jupiter.api.Test;

import java.util.EnumSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class BuiltinCommandOwnershipTest {

@Test
void serverFeaturesOwnedVanillaWrapperIsNotClassifiedAsMinecraft() {
Command command = new FakeVanillaCommandWrapper("autopickup");
Map<String, Command> commands = registrations(
"autopickup", command,
"minecraft:autopickup", command
);

BuiltinCommandSnapshot snapshot = BuiltinCommandDiscovery.discover(
commands,
Map.of(),
allBlocked(),
Set.of("autopickup")::contains
);

assertTrue(snapshot.blockedCommands().isEmpty());
assertEquals(0, snapshot.detectedSources().get("minecraft"));
}

@Test
void namespacedBuiltinFallbackRemainsBlockableWhenPlainLabelIsOwned() {
Command command = new FakeVanillaCommandWrapper("restart");

BuiltinCommandSnapshot snapshot = BuiltinCommandDiscovery.discover(
registrations("minecraft:restart", command),
Map.of(),
allBlocked(),
Set.of("restart")::contains
);

assertEquals(Set.of("minecraft:restart"), snapshot.blockedCommands());
assertEquals(1, snapshot.detectedSources().get("minecraft"));
}

@Test
void pluginIdentifiableCommandWinsOverBuiltinNamespaceHeuristics() {
Plugin plugin = mock(Plugin.class);
when(plugin.getName()).thenReturn("ExamplePlugin");
Command command = new PluginOwnedCommand("friends", plugin);

BuiltinCommandSnapshot snapshot = BuiltinCommandDiscovery.discover(
registrations(
"friends", command,
"minecraft:friends", command
),
allBlocked()
);

assertTrue(snapshot.blockedCommands().isEmpty());
}

private static BuiltinCommandBlockerSettings allBlocked() {
return new BuiltinCommandBlockerSettings(
EnumSet.allOf(BuiltinCommandSource.class),
true,
false,
Set.of()
);
}

private static Map<String, Command> registrations(Object... entries) {
if (entries.length % 2 != 0) {
throw new IllegalArgumentException("registrations requires key/value pairs");
}

LinkedHashMap<String, Command> result = new LinkedHashMap<>();
for (int index = 0; index < entries.length; index += 2) {
result.put((String) entries[index], (Command) entries[index + 1]);
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
}
return result;
}

private static class FakeVanillaCommandWrapper extends Command {

private FakeVanillaCommandWrapper(String name) {
super(name);
}

@Override
public boolean execute(
@NotNull CommandSender sender,
@NotNull String commandLabel,
@NotNull String @NotNull [] args
) {
return true;
}
}

private static final class PluginOwnedCommand extends Command implements PluginIdentifiableCommand {

private final Plugin plugin;

private PluginOwnedCommand(String name, Plugin plugin) {
super(name);
this.plugin = plugin;
}

@Override
public @NotNull Plugin getPlugin() {
return plugin;
}

@Override
public boolean execute(
@NotNull CommandSender sender,
@NotNull String commandLabel,
@NotNull String @NotNull [] args
) {
return true;
}
}
}