Allow empty paths in namespaced keys (#12687)

paper-api/src/main/java/org/bukkit/NamespacedKey.java

@@ -1,8 +1,12 @@
 package org.bukkit;
 
 import com.google.common.base.Preconditions;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 import java.util.Locale;
+import java.util.OptionalInt;
 import java.util.UUID;
+import net.kyori.adventure.key.Key;
 import org.bukkit.plugin.Plugin;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
@@ -11,7 +15,7 @@ import org.jetbrains.annotations.Nullable;
 /**
  * Represents a String based key which consists of two components - a namespace
  * and a key.
- *
+ * <p>
  * Namespaces may only contain lowercase alphanumeric characters, periods,
  * underscores, and hyphens.
  * <p>
@@ -19,7 +23,7 @@ import org.jetbrains.annotations.Nullable;
  * underscores, hyphens, and forward slashes.
  *
  */
-public final class NamespacedKey implements net.kyori.adventure.key.Key, com.destroystokyo.paper.Namespaced { // Paper - implement Key and Namespaced
+public final class NamespacedKey implements Key, com.destroystokyo.paper.Namespaced {
 
     /**
      * The namespace representing all inbuilt keys.
@@ -34,44 +38,6 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
     private final String namespace;
     private final String key;
 
-    private static boolean isValidNamespaceChar(char c) {
-        return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '.' || c == '_' || c == '-';
-    }
-
-    private static boolean isValidKeyChar(char c) {
-        return isValidNamespaceChar(c) || c == '/';
-    }
-
-    private static boolean isValidNamespace(String namespace) {
-        int len = namespace.length();
-        if (len == 0) {
-            return false;
-        }
-
-        for (int i = 0; i < len; i++) {
-            if (!isValidNamespaceChar(namespace.charAt(i))) {
-                return false;
-            }
-        }
-
-        return true;
-    }
-
-    private static boolean isValidKey(String key) {
-        int len = key.length();
-        if (len == 0) {
-            return false;
-        }
-
-        for (int i = 0; i < len; i++) {
-            if (!isValidKeyChar(key.charAt(i))) {
-                return false;
-            }
-        }
-
-        return true;
-    }
-
     /**
      * Create a key in a specific namespace.
      * <p>
@@ -115,29 +81,34 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
 
     private void validate() {
         Preconditions.checkArgument(this.namespace.length() + 1 + this.key.length() <= Short.MAX_VALUE, "NamespacedKey must be less than 32768 characters");
-        Preconditions.checkArgument(isValidNamespace(this.namespace), "Invalid namespace. Must be [a-z0-9._-]: %s", this.namespace);
-        Preconditions.checkArgument(isValidKey(this.key), "Invalid key. Must be [a-z0-9/._-]: %s", this.key);
+        checkError("[a-z0-9_-.]", "namespace", this.namespace, Key.checkNamespace(this.namespace));
+        checkError("[a-z0-9_-./]", "key", this.key, Key.checkValue(this.key));
+    }
+
+    private static void checkError(String pattern, String name, String value, OptionalInt index) {
+        index.ifPresent(indexValue -> {
+            char character = value.charAt(indexValue);
+            throw new IllegalArgumentException(String.format("Non %s character in %s '%s' at index %d ('%s', bytes: %s)", pattern, name, value, indexValue, character, Arrays.toString(String.valueOf(character).getBytes(StandardCharsets.UTF_8))));
+        });
     }
 
     @NotNull
-    @Override // Paper
+    @Override
     public String getNamespace() {
-        return namespace;
+        return this.namespace;
     }
 
     @NotNull
-    @Override // Paper
+    @Override
     public String getKey() {
-        return key;
+        return this.key;
     }
 
     @Override
     public int hashCode() {
-        // Paper start
         int result = this.namespace.hashCode();
         result = (31 * result) + this.key.hashCode();
         return result;
-        // Paper end
     }
 
     @Override
@@ -145,15 +116,14 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
         if (obj == null) {
             return false;
         }
-        // Paper start
-        if (!(obj instanceof net.kyori.adventure.key.Key key)) return false;
+
+        if (!(obj instanceof Key key)) return false;
         return this.namespace.equals(key.namespace()) && this.key.equals(key.value());
-        // Paper end
     }
 
     @Override
     public String toString() {
-        return this.namespace + ":" + this.key;
+        return this.namespace + ':' + this.key;
     }
 
     /**
@@ -206,10 +176,8 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
      */
     @Nullable
     public static NamespacedKey fromString(@NotNull String string, @Nullable Plugin defaultNamespace) {
-        // Paper - Return null for empty string, check length
         Preconditions.checkArgument(string != null, "Input string must not be null");
         if (string.isEmpty() || string.length() > Short.MAX_VALUE) return null;
-        // Paper end - Return null for empty string, check length
 
         String[] components = string.split(":", 3);
         if (components.length > 2) {
@@ -219,12 +187,12 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
         String key = (components.length == 2) ? components[1] : "";
         if (components.length == 1) {
             String value = components[0];
-            if (value.isEmpty() || !isValidKey(value)) {
+            if (value.isEmpty() || !Key.parseableValue(value)) {
                 return null;
             }
 
             return (defaultNamespace != null) ? new NamespacedKey(defaultNamespace, value) : minecraft(value);
-        } else if (components.length == 2 && !isValidKey(key)) {
+        } else if (components.length == 2 && !Key.parseableValue(key)) {
             return null;
         }
 
@@ -233,7 +201,7 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
             return (defaultNamespace != null) ? new NamespacedKey(defaultNamespace, key) : minecraft(key);
         }
 
-        if (!isValidNamespace(namespace)) {
+        if (!Key.parseableNamespace(namespace)) {
             return null;
         }
 
@@ -242,7 +210,7 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
 
     /**
      * Get a NamespacedKey from the supplied string.
-     *
+     * <p>
      * The default namespace will be Minecraft's (i.e.
      * {@link #minecraft(String)}).
      *
@@ -255,7 +223,6 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
         return fromString(key, null);
     }
 
-    // Paper start
     @NotNull
     @Override
     public String namespace() {
@@ -271,7 +238,6 @@ public final class NamespacedKey implements net.kyori.adventure.key.Key, com.des
     @NotNull
     @Override
     public String asString() {
-        return this.namespace + ':' + this.key;
+        return this.toString();
     }
-    // Paper end
 }

paper-api/src/test/java/org/bukkit/NamespacedKeyTest.java

@@ -34,16 +34,6 @@ public class NamespacedKeyTest {
         assertThrows(IllegalArgumentException.class, () -> NamespacedKey.fromString(null));
     }
 
-    @Test
-    public void testEmptyNamespace() {
-        assertThrows(IllegalArgumentException.class, () -> new NamespacedKey("", "foo").toString());
-    }
-
-    @Test
-    public void testEmptyKey() {
-        assertThrows(IllegalArgumentException.class, () -> new NamespacedKey("minecraft", "").toString());
-    }
-
     @Test
     public void testInvalidNamespace() {
         assertThrows(IllegalArgumentException.class, () -> new NamespacedKey("minecraft/test", "foo").toString());

paper-server/src/main/java/org/bukkit/craftbukkit/CraftLootTable.java

@@ -39,7 +39,7 @@ public class CraftLootTable implements org.bukkit.loot.LootTable {
     }
 
     public static org.bukkit.loot.LootTable minecraftToBukkit(ResourceKey<LootTable> minecraft) {
-        return (minecraft == null || minecraft.location().getPath().isEmpty()) ? null : Bukkit.getLootTable(CraftLootTable.minecraftToBukkitKey(minecraft)); // Paper - fix some NamespacedKey parsing
+        return (minecraft == null) ? null : Bukkit.getLootTable(CraftLootTable.minecraftToBukkitKey(minecraft));
     }
 
     public static NamespacedKey minecraftToBukkitKey(ResourceKey<LootTable> minecraft) {

paper-server/src/main/java/org/bukkit/craftbukkit/util/CraftNamespacedKey.java

@@ -3,7 +3,6 @@ package org.bukkit.craftbukkit.util;
 import net.minecraft.resources.ResourceLocation;
 import org.bukkit.NamespacedKey;
 import org.jspecify.annotations.NullMarked;
-import org.jspecify.annotations.Nullable;
 
 @NullMarked
 public final class CraftNamespacedKey {
@@ -11,14 +10,6 @@ public final class CraftNamespacedKey {
     public CraftNamespacedKey() {
     }
 
-    public static @Nullable NamespacedKey fromStringOrNull(@Nullable String string) {
-        if (string == null || string.isEmpty()) {
-            return null;
-        }
-        ResourceLocation minecraft = ResourceLocation.tryParse(string);
-        return (minecraft == null || minecraft.getPath().isEmpty()) ? null : CraftNamespacedKey.fromMinecraft(minecraft); // Paper - Bukkit's parser does not match Vanilla for empty paths
-    }
-
     public static NamespacedKey fromString(String string) {
         return CraftNamespacedKey.fromMinecraft(ResourceLocation.parse(string));
     }