PersistentDataContainer cannot deserialize Strings containing JSON

    •  CraftBukkit version 3053-Spigot-4225eac-69c8e78 (MC: 1.16.5)
    • Yes

      When using an ItemMeta's PersistentDataContainer and adding a PersistentDataType.STRING with a value containing a JSON array, it cannot be loaded from a YamlConfiguration afterwards.

      Small example plugin: https://pastebin.com/KdjUQLPA (See line 32)

      Stacktrace:

      [11:01:07] [Server thread/ERROR]: Could not call method 'public static org.bukkit.inventory.meta.ItemMeta org.bukkit.craftbukkit.v1_16_R3.inventory.CraftMetaItem$SerializableMeta.deserialize(java.util.Map) throws java.lang.Throwable' of class org.bukkit.craftbukkit.v1_16_R3.inventory.CraftMetaItem$SerializableMeta for deserialization
      java.lang.RuntimeException: Could not deserialize found list
              at org.bukkit.craftbukkit.v1_16_R3.util.CraftNBTTagConfigSerializer.deserialize(CraftNBTTagConfigSerializer.java:77) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.craftbukkit.v1_16_R3.util.CraftNBTTagConfigSerializer.deserialize(CraftNBTTagConfigSerializer.java:54) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.craftbukkit.v1_16_R3.inventory.CraftMetaItem.<init>(CraftMetaItem.java:537) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.craftbukkit.v1_16_R3.inventory.CraftMetaBlockState.<init>(CraftMetaBlockState.java:111) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:1.8.0_271]
              at sun.reflect.NativeConstructorAccessorImpl.newInstance(Unknown Source) ~[?:1.8.0_271]
              at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(Unknown Source) ~[?:1.8.0_271]
              at java.lang.reflect.Constructor.newInstance(Unknown Source) ~[?:1.8.0_271]
              at org.bukkit.craftbukkit.v1_16_R3.inventory.CraftMetaItem$SerializableMeta.deserialize(CraftMetaItem.java:191) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_271]
              at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_271]
              at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:1.8.0_271]
              at java.lang.reflect.Method.invoke(Unknown Source) ~[?:1.8.0_271]
              at org.bukkit.configuration.serialization.ConfigurationSerialization.deserializeViaMethod(ConfigurationSerialization.java:85) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.serialization.ConfigurationSerialization.deserialize(ConfigurationSerialization.java:127) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.serialization.ConfigurationSerialization.deserializeObject(ConfigurationSerialization.java:207) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.file.YamlConstructor$ConstructCustomObject.construct(YamlConstructor.java:37) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructObjectNoCheck(BaseConstructor.java:229) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructObject(BaseConstructor.java:219) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructMapping2ndStep(BaseConstructor.java:479) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.SafeConstructor.constructMapping2ndStep(SafeConstructor.java:190) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructMapping(BaseConstructor.java:460) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.SafeConstructor$ConstructYamlMap.construct(SafeConstructor.java:556) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.file.YamlConstructor$ConstructCustomObject.construct(YamlConstructor.java:28) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructObjectNoCheck(BaseConstructor.java:229) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructObject(BaseConstructor.java:219) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructMapping2ndStep(BaseConstructor.java:479) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.SafeConstructor.constructMapping2ndStep(SafeConstructor.java:190) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructMapping(BaseConstructor.java:460) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.SafeConstructor$ConstructYamlMap.construct(SafeConstructor.java:556) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.file.YamlConstructor$ConstructCustomObject.construct(YamlConstructor.java:28) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructObjectNoCheck(BaseConstructor.java:229) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructObject(BaseConstructor.java:219) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.constructDocument(BaseConstructor.java:173) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.constructor.BaseConstructor.getSingleData(BaseConstructor.java:157) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.Yaml.loadFromReader(Yaml.java:490) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.yaml.snakeyaml.Yaml.load(Yaml.java:416) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.file.YamlConfiguration.loadFromString(YamlConfiguration.java:57) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.file.FileConfiguration.load(FileConfiguration.java:160) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.file.FileConfiguration.load(FileConfiguration.java:128) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.configuration.file.YamlConfiguration.loadConfiguration(YamlConfiguration.java:188) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at de.jeff_media.nbtserialization.Main.onEnable(Main.java:48) ~[?:?]
              at org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:263) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.plugin.java.JavaPluginLoader.enablePlugin(JavaPluginLoader.java:351) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.plugin.SimplePluginManager.enablePlugin(SimplePluginManager.java:480) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.craftbukkit.v1_16_R3.CraftServer.enablePlugin(CraftServer.java:493) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at org.bukkit.craftbukkit.v1_16_R3.CraftServer.enablePlugins(CraftServer.java:407) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at net.minecraft.server.v1_16_R3.MinecraftServer.loadWorld(MinecraftServer.java:555) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at net.minecraft.server.v1_16_R3.DedicatedServer.init(DedicatedServer.java:257) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at net.minecraft.server.v1_16_R3.MinecraftServer.w(MinecraftServer.java:928) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at net.minecraft.server.v1_16_R3.MinecraftServer.lambda$0(MinecraftServer.java:273) ~[spigot-1.16.5.jar:3053-Spigot-4225eac-69c8e78]
              at java.lang.Thread.run(Unknown Source) [?:1.8.0_271]
      Caused by: com.mojang.brigadier.exceptions.CommandSyntaxException: Invalid array type '"' at position 1: [<--[HERE]
      

      How to reproduce:

      @Override
      public void onEnable() {
          ItemStack item = new ItemStack(Material.DIRT);
          ItemMeta meta = item.getItemMeta();
          PersistentDataContainer pdc = meta.getPersistentDataContainer();
          pdc.set(new NamespacedKey(this,"key"),PersistentDataType.STRING,"[\"my\",\"list\",\"of\",\"values\"]");
          item.setItemMeta(meta);
          YamlConfiguration yaml = new YamlConfiguration();
          yaml.set("item",item);
          try {
              yaml.save(new File(getDataFolder(), "item.yml"));
          } catch (IOException ioException) {
              ioException.printStackTrace();
          }
      
          // Next line is throwing the stacktrace
          YamlConfiguration loaded = YamlConfiguration.loadConfiguration(new File(getDataFolder(), "item.yml"));
      }
      

      The YAML produced from this looks correct but cannot be read:

      item:
        ==: org.bukkit.inventory.ItemStack
        v: 2586
        type: CHEST
        meta:
          ==: ItemMeta
          meta-type: TILE_ENTITY
          PublicBukkitValues:
            nbtserialization:test: '["This contains","some json"]'
          blockMaterial: CHEST
      

      Online YAML parsers / viewers have no problem with that, they load it fine and display the value as String (like expected), e.g. https://jsonformatter.org/yaml-viewer or http://www.yamllint.com/ 

      The bug also occurs in CraftBukkit version 3053-Bukkit-69c8e78

       

      (It seems like the deserialize method thinks the value is a YAML list although it should be read as normal String)

          [SPIGOT-6439] PersistentDataContainer cannot deserialize Strings containing JSON

          coll1234567 added a comment -

          This was fixed by this commit

          coll1234567 added a comment - This was fixed by this commit

          mfnalex added a comment -

          Seems to be fixed meanwhile (1.20.6)

          mfnalex added a comment - Seems to be fixed meanwhile (1.20.6)

          Phoenix616 added a comment -

          Well it's on a pull request, you need to sign the CLA to have access to those. It was mentioned more as a reference to whomever decides to take a look at this.

          Phoenix616 added a comment - Well it's on a pull request, you need to sign the CLA to have access to those. It was mentioned more as a reference to whomever decides to take a look at this.

          mfnalex added a comment -

          @Pheonix616 I don't have access to view the conversation

          mfnalex added a comment - @Pheonix616 I don't have access to view the conversation

          Phoenix616 added a comment - - edited

          It seems like this is actual intentional behaviour in order to be able to properly store all array types that NBT supports as their string representations. Seems to be due to this conversation. The downside of normal strings being wrongly counted as arrays (or even integers/doubles) seems to have not come up there which is unfortunate, ideally there would've been a custom YAML serialisier registered for those type of data values and fixing this now would require some kind of conversation based on the data version of the item :S (Or ignoring the exception there and treading it as a raw string in such a case but that seems a bit hacky)

          Phoenix616 added a comment - - edited It seems like this is actual intentional behaviour in order to be able to properly store all array types that NBT supports as their string representations. Seems to be due to this conversation . The downside of normal strings being wrongly counted as arrays (or even integers/doubles) seems to have not come up there which is unfortunate, ideally there would've been a custom YAML serialisier registered for those type of data values and fixing this now would require some kind of conversation based on the data version of the item :S (Or ignoring the exception there and treading it as a raw string in such a case but that seems a bit hacky)

            Assignee:
            Unassigned
            Reporter:
            mfnalex
            Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: