Island YAML corrupted after they ban another player from island

[mattc4t]

Description
The island YAML file of a player becomes corrupted after they ban another player from their island. The YAML file will not parse on server startup
After startup, the player whose island file is corrupted, is given a new island, instead of their current one.

Steps to reproduce the behavior:

1. Ban a player from your island: /island ban username
2. Stop the server
3. Start the server
4. Startup log shows error:
   [21:31:45] [Server thread/ERROR]: [BentoBox] Could not load yml file from database database\Island 09831dd8-b7b4-420e-a643-1cf3b5db4de5.yml could not determine a constructor for the tag tag:yaml.org,2002:java.util.UUID
   in 'string', line 83, column 13:
   player: !!java.util.UUID '51233357-dc2b- ...

Impact:
The player whose island file is corrupt will be forced to start a new island.
The server admin must then find where the player's island was, (using hawkeye or coreprotect), go there, then register the player for the island once again

Expected behavior
Ban should not corrupt data files

Screenshots
[22:02:19 ERROR]: [BentoBox] Could not load yml file from database database\Island 66f769a9-5516-44ef-a092-48d69246aa69.yml could not determine a constructor for the tag tag:yaml.org,2002:java.util.UUID
in 'string', line 83, column 13:
player: !!java.util.UUID '51233357-dc2b- ...
^

Server Information:
CraftBukkit version git-Spigot-f56e2e7-7fc66b2 (MC: 1.13.2) (Implementing API version 1.13.2-R0.1-SNAPSHOT)

[Please complete the following information:]

• Database being used: YAML

• OS: Windows 10

• Java Version: 1.8.0_151

• BentoBox version: 1.1

• Addons installed?
  [21:44:10 INFO]: Bentobox version: 1.1
  [21:44:10 INFO]: Loaded Game Worlds:
  [21:44:10 INFO]: SkyGrid-world (SkyGrid)
  [21:44:10 INFO]: BSkyBlock_world (BSkyBlock)
  [21:44:10 INFO]: Loaded Add-Ons
  [21:44:10 INFO]: BSkyBlock 1.0
  [21:44:10 INFO]: SkyGrid 0.0.1-SNAPSHOT
  [21:44:10 INFO]: Biomes 0.3.0-SNAPSHOT
  [21:44:10 INFO]: Challenges 0.3.1-SNAPSHOT
  [21:44:10 INFO]: Level 0.2.0
  [21:44:10 INFO]: WelcomeWarps 0.1.0-SNAPSHOT

• Other plugins? [Do '/plugins' and copy/paste from the console]
  [21:44:46 INFO]: Plugins (66): PlayerUUID, Admin-Chat-Channel, ColoredSigns, McWiki, ColorCodeTeacher, PlaceholderAPI, PlugMan, LeadTheWay, AntiWorldFly, TicketMaster, BanManager, Hordes, ColoredChat, CheckNameHistory, AdminNotes, TreeGravity, CoreProtect, LuckPerms, WorldEdit, FirstJoinPlus, OreAnnouncer, ChatControl, WorldBorder, ProtocolLib, Stables, FastAsyncWorldEdit, iDisguise, DIYspy, SurvivalFly, Vault, MobHeads, BookManager, PerWorldInventory, HolographicDisplays, LWC, WorldGuard, MaintenanceSpigot, MyCommand, UndineMailer, HawkEye, InfernalMobs, MyWarp, Essentials, BookShelf, Jobs, ExtraContexts, TNTRun, SafeTrade, Citizens, VanishNoPacket, BentoBox, EssentialsChat, CustomOreGen, TouchscreenHolograms, PickupArrows, QuickShop, CommandNPC, Multiverse-Core, Shopkeepers, Multiverse-Portals, GriefPrevention, Holograms, ExtraHardMode, Multiverse-NetherPortals, MonsterApocalypse, Paintball
  66f769a9-5516-44ef-a092-48d69246aa69.zip

[Poslovitch]

Duplicate of #482.

Download 1.2.0 devbuilds to get a fix for this bug.

[mattc4t]

Thanks! Sorry, I searched for a duplicate, could not find one that stood-out to me. I'll try the devbuilds :)

[mattc4t]

Sorry to say that this is still an issue... I downloaded the latest 1.2.0 devbuild, repeated the same steps as before, and the YAML file is corrupted on startup - forcing the player to a newly-generated island.

[mattc4t]

65a594fa-47f5-422b-a2ab-7a40c8195076.zip

[22:59:28 ERROR]: [BentoBox] Could not load yml file from database database\Island 65a594fa-47f5-422b-a2ab-7a40c8195076.yml could not determine a constructor for the tag tag:yaml.org,2002:java.util.UUID
in 'string', line 21, column 13:
player: !!java.util.UUID '51233357-dc2b- ...
^

[Poslovitch]

Hmm, weird.

@tastybento Does the YAML " load object" tries to read non @Expose fields? Or have I just been very naive with ae24643 ?

[BONNe]

Why dont you store UUIDs as strings?
As you definitely are trying to read them as string objects...

[tastybento]

@Poslovitch You should use @Expose, but the actual issue is that as you are defining your own LogEntryListAdapter, the default adapters (like the UUID one) will not be used so you need to serialize the UUID as well (which is easy to do).

[Poslovitch]

Oh, crap!
I'll fix this ASAP.

Will this fix the read issue, or will we need to ask our users to manually remove the "bugged" lines?

[tastybento]

It won't fix it. I'm not sure if there's a programmatic way to catch those bugged lines and ignore them or not.

[Poslovitch]

I'd say I could extend the scope of the "Converter API" to actually allow us to do automated database revisions between versions... But that'd make it harder to implement ^^

[tastybento]

I had a look at the code and the YAML object loading is handled entirely by Bukkit and its YAML processor. It serializes unknown objects by prefixing the class name with !! and using toString() and then hopes that there is a corresponding constructor that'll take the string input and convert it back. In the case of UUID there isn't, so it throws an error. We can't tap into that.

e.g.:
!!java.util.UUID '51233357-dc2b-4ae2-9703-ba8585b21759'

The only "hack" way to avoid this would be to open the file before loading it and scan for lines with !!java.util.UUID in them and remove the whole history section.

[tastybento]

By the way, assuming you store all your history as strings, in Linux/MacOS, running this command in the Island folder of the database will remove all the offending !!java.util.UUID parts and make the files readable:

sed -e "s/!!java.util.UUID //g" -i *

Another approach would be to check if an error regarding UUID is occurring (java.util.UUID) in the exception handler of YamlDatabaseConnector (line 62) and then if it is, run the file through a utility class that will remove the !!java.util.UUID text (see here for how to do that) and then try and load it again. That'll recover any mis-written files.

[Poslovitch]

@tastybento May I have your review on afdfc28 ? Do you think it'll avoid this bug from happening ?

@mattc4t Do you think we should provide an automated "fix" for the islands that got "corrupted" by this bug or do you think it can be easily fixed "by hand"?

[tastybento]

@Poslovitch See db932dd. I added the auto fix too.

[mattc4t]

Great! I have a relatively small server, so I can fix the config by hand relatively easily. Skyblock has been a favorite on our server for a while.

Thank you for the rewrite and providing it for 1.13.2! I also look forward to installing SkyGrid soon...

[mattc4t]

I tried the latest 1.1.1 Devbuild: Mission Accomplished!!!

• On startup, the plugin fixed the corrupt entry
  -After banning a player, the ban entry in the island file looked fine, without a UUID prefix before the UUID.

Thank you for the timely fix 👍