Error with item serialization in fireworks
takatalvi opened this issue · 11 comments
Information
- Server version:
[11:55:26 INFO]: Checking version, please wait...
[11:55:27 INFO]: This server is running Paper version git-Paper-81 (MC: 1.19) (Implementing API version 1.19-R0.1-SNAPSHOT) (Git: 86f87ba)
You are running the latest version
Previous version: git-Paper-70 (MC: 1.19)
-
Full output of
/mv version -p: https://paste.gg/2d170c6b881f4eea9671772060fea610 -
Server log: https://gist.github.com/takatalvi/7b2d025cfedd8f711fa2d76b9d6cccc4
Details
I was able to reproduce my issue on a freshly setup and up-to-date server with the latest version of Multiverse plugins with no other plugins and with no kinds of other server or client mods.
Description
Fireworks with Flight:-2 in the inventory or ender chests are stored by MV-Inventories data as "power":254, which causes an exception to show up periodically, each time the player's MV-Inventories data file is accessed:
[10:46:21 ERROR]: [org.bukkit.configuration.serialization.ConfigurationSerialization] Could not call method 'public static org.bukkit.inventory.meta.ItemMeta org.bukkit.craftbukkit.v1_19_R1.inventory.CraftMetaItem$SerializableMeta.deserialize(java.util.Map) throws java.lang.Throwable' of class org.bukkit.craftbukkit.v1_19_R1.inventory.CraftMetaItem$SerializableMeta for deserialization
java.lang.IllegalArgumentException: Power cannot be more than 127: 254
at org.apache.commons.lang.Validate.isTrue(Validate.java:93) ~[commons-lang-2.6.jar:2.6]
at org.bukkit.craftbukkit.v1_19_R1.inventory.CraftMetaFirework.setPower(CraftMetaFirework.java:404) ~[paper-1.19.jar:git-Paper-81]
at org.bukkit.craftbukkit.v1_19_R1.inventory.CraftMetaFirework.<init>(CraftMetaFirework.java:185) ~[paper-1.19.jar:git-Paper-81]
at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:?]
at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77) ~[?:?]
at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[?:?]
at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499) ~[?:?]
at java.lang.reflect.Constructor.newInstance(Constructor.java:480) ~[?:?]
at org.bukkit.craftbukkit.v1_19_R1.inventory.CraftMetaItem$SerializableMeta.deserialize(CraftMetaItem.java:201) ~[paper-1.19.jar:git-Paper-81]
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:?]
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) ~[?:?]
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]
at java.lang.reflect.Method.invoke(Method.java:568) ~[?:?]
at org.bukkit.configuration.serialization.ConfigurationSerialization.deserializeViaMethod(ConfigurationSerialization.java:85) ~[paper-api-1.19-R0.1-SNAPSHOT.jar:?]
at org.bukkit.configuration.serialization.ConfigurationSerialization.deserialize(ConfigurationSerialization.java:127) ~[paper-api-1.19-R0.1-SNAPSHOT.jar:?]
at org.bukkit.configuration.serialization.ConfigurationSerialization.deserializeObject(ConfigurationSerialization.java:207) ~[paper-api-1.19-R0.1-SNAPSHOT.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.util.SerializationHelper.deserialize(SerializationHelper.java:125) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.util.SerializationHelper.deserialize(SerializationHelper.java:116) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.util.SerializationHelper.deserialize(SerializationHelper.java:116) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.util.SerializationHelper.deserialize(SerializationHelper.java:116) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.util.SerializationHelper.deserialize(SerializationHelper.java:116) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.json.JsonConfiguration.convertMapsToSections(JsonConfiguration.java:75) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.json.JsonConfiguration.loadFromString(JsonConfiguration.java:68) ~[Multiverse-Inventories-4.2.3.jar:?]
at org.bukkit.configuration.file.FileConfiguration.load(FileConfiguration.java:160) ~[paper-api-1.19-R0.1-SNAPSHOT.jar:?]
at org.bukkit.configuration.file.FileConfiguration.load(FileConfiguration.java:128) ~[paper-api-1.19-R0.1-SNAPSHOT.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.json.JsonConfiguration.loadConfiguration(JsonConfiguration.java:110) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.utils.configuration.json.JsonConfiguration.loadConfiguration(JsonConfiguration.java:131) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.FlatFileProfileDataSource.getConfigHandleNow(FlatFileProfileDataSource.java:96) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.FlatFileProfileDataSource.access$100(FlatFileProfileDataSource.java:37) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.FlatFileProfileDataSource$ConfigLoader.call(FlatFileProfileDataSource.java:108) ~[Multiverse-Inventories-4.2.3.jar:?]
at com.onarandombox.multiverseinventories.FlatFileProfileDataSource$ConfigLoader.call(FlatFileProfileDataSource.java:99) ~[Multiverse-Inventories-4.2.3.jar:?]
at java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) ~[?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ~[?:?]
at java.lang.Thread.run(Thread.java:833) ~[?:?]
Steps to reproduce
Place a firework rocket with its NBT Flight set to a negative number (-2 fires the rocket immediately upon using) in your inventory or ender chest.
Expected behavior
When MV-Inventories is set up with different world groups and inventory/enderchest data is written to individual player files, this negative number is stored as a positive >127 number, which causes an exception as the plugin is not expecting values above 127.
Hi, I just wanted to add that this is still happening on the latest Paper 1.19.3.
This looks like a Spigot bug. MV Inventories doesn't handle serialization/deserialization of items, that's left to the server. You should probably report this to Spigot, and if that fails, to Paper.
Thank you for reviewing this, I'm preparing things to report it to Spigot, but I'm unable to reproduce this without MV-Inventories. I think that's because it only happens when MV has to save a separate inventory into a player's file. Even with the plugin installed, if you have a single world group, this doesn't happen, because the inventory and ender chest are saved in vanilla userdata. Are you sure this will not get rejected by Spigot because it's an exception from a plugin?
I set up a Spigot test server with only MV and MV-Inventories, split worlds into two groups and managed to reproduce the issue this way. I'm ready to report it, but I want to make sure everything needed is included. If you have something specific that I could quote, from the backend-serialization-technical point of view, it would be very helpful. Thanks!
On second thought, I'll just open a PR and will let you know how that goes.
edit: it seems it's more work than I thought it was (not really a lot, just more than I thought). Passing this back to you to report it to Spigot.
I am positive this is an issue in CraftBukkit, as I took a look at their code and it's clear that there is some issue with serializing/deserializing present, I just didn't know what approach they would have preferred, and I didn't really want to go back and forth on it.
Unfortunately, I can't say that it won't be rejected, as Spigot has rejected valid bugs in the past.
If you'd like something to quote: "The issue is that the NBT allows for any value between -128 and 127, which Spigot serializes as a value between 0 and 255. This is fine, but when deserializing, Spigot only accepts values from 0 to 127, which is what causes the error.
I'd probably recommend changing the serialization to keep the sign bit, but I'm not sure if that's possible. Either way, the deserialization will need to be changed to accommodate the bigger range".
Thank you so much, I've opened a bug for Spigot: https://hub.spigotmc.org/jira/browse/SPIGOT-7212
Thanks!
No worries. I took a quick look and your report looks good, nice and detailed! Hopefully it gets resolved.
I got a notification (3 days ago, spam filter failed) that my Spigot bug has been resolved, there has been a commit for this issue: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/commits/0a1c89e4b1ce8902cfc9b21ae1844653d1f59781
I won't be able to test it today, but it looks like md_5 took care of it personally :)
I'll test as soon as I can to confirm, but I'm sure you will be able to tell by the changes in the code if it's done. Thanks for your help so far!
Looks as though it should work. The fix is in Paper build 362, so give it a try when you get the chance, hopefully it works!
I just updated to the new build, tested with the same setup as before, and the error didn't come up. Thanks for your help and guidance, I'm glad we managed to sort it out!