Advise on fixing NMSException

[Froxcey]

I found a permanent fix for all versions, kind of

⚠️ This is a guide for plugin developers, not ordinary users or server owners. To see why I don't provide a fix, see the last section

I have a hacky fix for this. There are two major bugs that cause this plugin to be unusable: NBT data manipulation and chunk load event entity handling. I will go through the method to fix them below if you want to fix the plugin yourself.

Yes, the issue is from the poorly maintained Quartzlib dependency, but all fixes below doesn't touch those spaghetti code (cuz Bukkit already provided a better implementation than Quartzlib that we will be implementing).

Get the code to build

I don't think this plugin is possible to be built from source, at least not for me. The POM file need to be updated. Remove Bukkit from the dependencies list, and replace it with latest version of Spigot. Remove quartzlib from dependency list, as the repository is not online as the time of writing. It's not possible to download the dependency to build. Download Quartzlib from the repo, and merge it with the project folder. There are some extra Maven dependencies that need to be installed for the project to build properly, including common-lang, json-simple, and Jetbrain annotation.

NBT

NBT manipulation has different APIs across all versions, so naturally new Minecraft version = broken plugin. In this repo, NBT manipulation is only use for marking a map as part of a poster, which means we can use an alternative method. In the fr.moribus.imageonmap.ui.SplatterMapManager class, search for all parts that involves NBT manipulation. When I was investigating, I tried to fix NBT, which took a lot of effort with no outcome. Replace NBT with Persistent Data API from Bukkit is just much easier. Plus, this means this will have a higher chance of surviving through updates. This fix should work for all 1.14+ servers, but I only tested it on 1.19. Now build the jar, load the plugin, and you should be able to create a poster and place it normally on item frames.

Chunk load entity handling

Now that NBT is fixed, try shutting down the server and starting it again. Yeah... Posters don't survive a server restart. You can take the poster down and place them again to manually reload the poster, but that's just lame. Thankfully, I also got a fix for this.

This time around, the error is in the fr.moribus.imageonmap.image.MapInitEvent class. The onChunkLoad method is an event handler that loads all posters when a chunk loads, which includes server restarts. However, entities aren't loaded when the chunk load event is fired. This means the plugin is trying to initialise a chunk without any entities in it, including item frames (posters). The fix I implemented myself is to add a 20 ticks of delay to wait for entities to load, then initialise all the maps. The fix I implement is BukkitRunnable, which is really easy to use. Now just build the jar and load the plugin.

There are still some minor bugs, but basic usage should be normal after this fix.

Criticising the maintainer

What caused the crash? Long story short, poorly maintained spaghetti code. It has been a lot of pain for me to look into this plugin. Old hacky methods are not replaced with newer and cleaner APIs. Dependency list isn't up to date. There are a lot of deprecated API used in this code base. I don't know if the dependency library should be zlib or quartzlib. The plugin itself imports zlib but the actual artifact name is quartzlib, which confuses the compiler. The maven dependency repository for quartzlib is down, so I had to cloned it from the GitHub repo. It has been a lot of work just to get the code to build. There are pull requests from last year that are still waiting for review. Duper trooper made a video on the barrier exploit for half a year now (for more details please see #250 ), and it still isn't patched. Many duplicated issues are not dealt with. Maintainer promises fixes, but actually responded with silence. Overall, just too much spaghetti code and lack of maintenance. This plugin has a lot of downloads, and it deserves more care from the creator. If you can't maintain this project, archive it and leave it to someone else.

My fix isn't perfect

I'm not willing to make a PR because this fix is, as I mentioned above, a bit hacky. I don't want to be responsible for a PR that is vulnerable, especially when I'm dealing with spaghetti code. I can make a PR if people can confirm that my fix isn't vulnerable, which I believe is. I provide the method to fix to save time for other devs who want to look into this too. If you don't implement the fix yourself, you don't understand what is wrong with this fix, and I don't think it's a good idea to let you run the plugin. If you really want to fix it, follow the guide, and you will have a plugin that works while having an understanding of what potential vulnerability exists in this plugin after the patch. Or maybe I'm just too lazy to deal with PRs and maintaining forks lol.

[Vlammar]

Hello, current (and only) maintainer right here. First, thanks for the fix ideas. I know that I don't go as fast as I should on this side project. I was rewritting and removing most of the dependancies to QL (old name is zlib) mainly because it became a pain to update and maintain espacially on the latest minecraft versions. The first thing I'm doing is to trim all of the unused/spaghetti code, that take some time and is not an short task to do. My main issue with this is that I have lot of things going on in my life right now and minecraft plugin development is not on my high priority list.
On your fix points, the nms is not usefull and will be removed, on my dev branch I use the map id instead of nms, that's more stable and better. For the load entity part I will try your fix, and investigate it when the rewritting of the plugin is done.

Ps i love your gif at the end

[Froxcey]

Hey, thank you for your attention on this issue. Regarding the entity loading fix, it may lead to vulnerabilities. If maps are loaded after the player joins, I'm not sure if that will lead to any unexpected behavior. Also, I'm investigating the barrier exploit. Since it is a vulnerability, I will submit a pull request if I make a fix.