Ignore empty string world names in Bukkit WorldCalculator (#2119)

api/src/main/java/net/luckperms/api/context/Context.java

@@ -26,12 +26,62 @@
 package net.luckperms.api.context;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 /**
  * Represents an individual context pair.
+ *
+ * <p>Context keys and values may not be null or empty. A key/value will be
+ * deemed empty if it's length is zero, or if it consists of only space
+ * characters.</p>
  */
 public interface Context {
 
+    /**
+     * Tests whether {@code key} is valid.
+     *
+     * <p>Context keys and values may not be null or empty. A key/value will be
+     * deemed empty if it's length is zero, or if it consists of only space
+     * characters.</p>
+     *
+     * <p>An exception is thrown when an invalid key is added to a {@link ContextSet}.</p>
+     *
+     * @param key the key to test
+     * @return true if valid, false otherwise.
+     * @since 5.1
+     */
+    static boolean isValidKey(@Nullable String key) {
+        if (key == null || key.isEmpty()) {
+            return false;
+        }
+
+        // look for a non-whitespace character
+        for (int i = 0, n = key.length(); i < n; i++) {
+            if (key.charAt(i) != ' ') {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Tests whether {@code value} is valid.
+     *
+     * <p>Context keys and values may not be null or empty. A key/value will be
+     * deemed empty if it's length is zero, or if it consists of only space
+     * characters.</p>
+     *
+     * <p>An exception is thrown when an invalid value is added to a {@link ContextSet}.</p>
+     *
+     * @param value the value to test
+     * @return true if valid, false otherwise.
+     * @since 5.1
+     */
+    static boolean isValidValue(@Nullable String value) {
+        return isValidKey(value); // the same for now...
+    }
+
     /**
      * Gets the context key.
      *

bukkit/src/main/java/me/lucko/luckperms/bukkit/context/WorldCalculator.java

@@ -29,6 +29,7 @@ import me.lucko.luckperms.bukkit.LPBukkitPlugin;
 import me.lucko.luckperms.common.config.ConfigKeys;
 import me.lucko.luckperms.common.context.contextset.ImmutableContextSetImpl;
 
+import net.luckperms.api.context.Context;
 import net.luckperms.api.context.ContextCalculator;
 import net.luckperms.api.context.ContextConsumer;
 import net.luckperms.api.context.ContextSet;
@@ -54,7 +55,9 @@ public class WorldCalculator implements ContextCalculator<Player> {
     public void calculate(@NonNull Player subject, @NonNull ContextConsumer consumer) {
         Set<String> seen = new HashSet<>();
         String world = subject.getWorld().getName().toLowerCase();
-        while (seen.add(world)) {
+        // seems like world names can sometimes be the empty string
+        // see: https://github.com/lucko/LuckPerms/issues/2119
+        while (Context.isValidValue(world) && seen.add(world)) {
             consumer.accept(DefaultContextKeys.WORLD_KEY, world);
             world = this.plugin.getConfiguration().get(ConfigKeys.WORLD_REWRITES).getOrDefault(world, world).toLowerCase();
         }
@@ -66,7 +69,7 @@ public class WorldCalculator implements ContextCalculator<Player> {
         ImmutableContextSet.Builder builder = new ImmutableContextSetImpl.BuilderImpl();
         for (World world : worlds) {
             String name = world.getName().toLowerCase();
-            if (!name.trim().isEmpty()) {
+            if (Context.isValidValue(name)) {
                 builder.add(DefaultContextKeys.WORLD_KEY, name);
             }
         }

common/src/main/java/me/lucko/luckperms/common/command/utils/ArgumentParser.java

@@ -39,6 +39,7 @@ import me.lucko.luckperms.common.sender.Sender;
 import me.lucko.luckperms.common.storage.misc.DataConstraints;
 import me.lucko.luckperms.common.util.DurationParser;
 
+import net.luckperms.api.context.Context;
 import net.luckperms.api.context.DefaultContextKeys;
 import net.luckperms.api.context.ImmutableContextSet;
 import net.luckperms.api.context.MutableContextSet;
@@ -192,10 +193,8 @@ public class ArgumentParser {
                 value = entry;
             }
 
-            if (AbstractContextSet.stringIsEmpty(key) ||
-                    AbstractContextSet.stringIsEmpty(value) ||
             // TODO reconsider a better place to insert / avoid this special case
-                    AbstractContextSet.shouldIgnoreEntry(key, value)) {
+            if (!Context.isValidKey(key) || !Context.isValidValue(value) || AbstractContextSet.isGlobalServerWorldEntry(key, value)) {
                 continue;
             }
 

common/src/main/java/me/lucko/luckperms/common/context/contextset/AbstractContextSet.java

@@ -28,6 +28,7 @@ package me.lucko.luckperms.common.context.contextset;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.SetMultimap;
 
+import net.luckperms.api.context.Context;
 import net.luckperms.api.context.ContextSet;
 import net.luckperms.api.context.DefaultContextKeys;
 
@@ -76,7 +77,7 @@ public abstract class AbstractContextSet implements ContextSet {
 
     static String sanitizeKey(String key) {
         Objects.requireNonNull(key, "key is null");
-        if (stringIsEmpty(key)) {
+        if (!Context.isValidKey(key)) {
             throw new IllegalArgumentException("key is (effectively) empty");
         }
         return key.toLowerCase();
@@ -84,25 +85,13 @@ public abstract class AbstractContextSet implements ContextSet {
 
     static String sanitizeValue(String value) {
         Objects.requireNonNull(value, "value is null");
-        if (stringIsEmpty(value)) {
+        if (!Context.isValidValue(value)) {
             throw new IllegalArgumentException("value is (effectively) empty");
         }
         return value.toLowerCase();
     }
 
-    public static boolean stringIsEmpty(String s) {
+    public static boolean isGlobalServerWorldEntry(String key, String value) {
-        if (s.isEmpty()) {
-            return true;
-        }
-        for (char c : s.toCharArray()) {
-            if (c != ' ') {
-                return false;
-            }
-        }
-        return true;
-    }
-
-    public static boolean shouldIgnoreEntry(String key, String value) {
         return (key.equalsIgnoreCase(DefaultContextKeys.SERVER_KEY) || key.equalsIgnoreCase(DefaultContextKeys.WORLD_KEY)) && value.equalsIgnoreCase("global");
     }
 

common/src/main/java/me/lucko/luckperms/common/node/AbstractNodeBuilder.java

@@ -119,7 +119,7 @@ public abstract class AbstractNodeBuilder<N extends ScopedNode<N, B>, B extends
     @Override
     public @NonNull B withContext(@NonNull String key, @NonNull String value) {
         // TODO reconsider a better place to insert / avoid this special case
-        if (AbstractContextSet.shouldIgnoreEntry(key, value)) {
+        if (AbstractContextSet.isGlobalServerWorldEntry(key, value)) {
             return (B) this;
         }
         this.context.add(key, value);