[Crash][Potential byte overflow in dungeon-sync packet]

[judgekreep]

General Information To check a checkbox: Insert a X between the square brackets, you need to remove the space though.

• I play...
  • [ ] With a large modpack
  • [X] Only with CQR and it's dependencies
• The issue occurs in...
  • [X] Singleplayer
  • [ ] Multiplayer
• [X] I have searched for this or a similar issue before reporting and it was either (1) not previously reported, or (2) previously fixed and I'm having the same problem.
• [X] I am using the latest version of the mod (all versions can be found on github under releases)
• [X] I read through the FAQ and I could not find something helpful (FAQ)
• [X] I reproduced the bug without any other mod's except forge, cqr and it's dependencies
• [X] The game crashes because of this bug

Describe the bug Having 136 or more .property files will not allow you to join a world - it results in: Shutting down internal server... > Connection lost: A fatal error has occurred, this connection is terminated > Multiplayer screen

• I did some testing and I believe it's 136, but not 100% sure. I did not crash at 135, but did at 136. Here's a snippet from the log that I think is what's causing it: Caused by: java.lang.IllegalArgumentException: Illegal Capacity: -104 at java.util.ArrayList.(ArrayList.java:157) ~[?:1.8.0_332] It also references team.cqr.cqrepoured.network.server.packet.SPacketDungeonSync.fromBytes (Full log file provided below).

To Reproduce Steps to reproduce the behavior:

1. Have 136 or more .property files in the dungeons folder
2. Attempt to load into a world.
3. This happens: Shutting down internal server... > Connection lost: A fatal error has occurred, this connection is terminated > Multiplayer screen

Versions (Note that 'latest' is NOT a valid version! Chocolate Quest Repoured: 2.6.9B Forge: 14.23.5.2860 Minecraft: 1.12.2 Other mods (necessary to reproduce the bug): Only dependencies (Reachfix and Geckolib)

Log File There is no crash file, but there are errors in the log: https://pastebin.com/MEKtkF6r Starts at line 125 and says: FMLIndexedMessageCodec exception caught io.netty.handler.codec.DecoderException: java.lang.IllegalArgumentException: Illegal Capacity: -104

P.S. I have over 136 legit structures/properties and it started crashing once I hit 136. This error still happens if you just copy and paste the existing CQR property files, so long as you have 136 or more.

[DerToaster98]

Looks like the sync packet gets too big, is there really the need for 136 dungeon types? I honestly doubt that. This issue is fixable but it would be quite the effort

[judgekreep]

For majority of people, 135 is probably more than enough.

I like to diversify where structures spawn (biomes, dimension) as well as how high and if underwater or underground, and of course, the 'chance' variable allows for further balancing of structures that are in the same grid - - there's so many different combinations of those factors that it's quite rare for more than 2 of my structures to use the same property files of another.

What exactly does the sync packet do in this scenario? I ask because I'm playing with a private mod pack and all players are strictly using the same mods and config settings, would this particular packet still be required in our case? (I hope that this packet is the network packet that I'm thinking of).

[DerToaster98]

That packet is required to sync dungeons between server and client, internally it has to do with the dungeon placers. I have a fix in mind but i need to test it first once i get home. Even if the packet would not be required there is not really a way to "disable" it.

Also i'm not really trusting you from now on since you used the term modpack which indicates that other mods may be present too in this scenario (from experience most don't test in an isolated environment but just directly lie to us about that). So just to confirm: Could you please upload a zipped copy of your CQR folder for testing?

[DerToaster98]

I still wonder about the value 136, cause that doesn't make sense, except if netty's ByteBuf does some weird things internally.

If it would be an overflow it would've occured at 129 dungeon configs. Could you please check the exact value again?

[DerToaster98]

Wait, it could still make sense, depending on how that library acts, though if it acts like i think it would mess things up beginning once 129 entries are synced

[judgekreep]

The log file provided at the bottom of my initial report should list all of the mods that I used when I tested, but I completely understand your skepticism.

My configs and everything were freshly installed. All I did was copy and paste duplicate property files in the dungeon folder - - the issue still occurred in that instance. CQR.zip Again, the folder above is what I used after the fresh install, not the one that I used with my mod pack. Those test files will not crash since there's only 135 but you'll encounter the issue if you add just one more file.

I can't run tests at the moment, but I ran at least 5 different tests earlier to confirm the crash - the results were the same. Three tests were in the mod pack environment and two tests were with only CQR and its dependencies - - it took more than 135 files to crash, but I never crashed at 135 files or less.

If you'd still like me to test again, please let me know and I'll try later tonight.

[DerToaster98]

will look at it when i get home, if we're lucky the patch i made (which literally just increases the potential number for files to be synched (changed byte to short)) fixes it

[DerToaster98]

Testing now

[DerToaster98]

fixed it

Will be patched in the next update (will release it either tomorrow or next tuesday)

[DerToaster98]

FYI It should support about 32.768 config files, i guess that is more than enough, even for your methods ;)

(I actually appreciate people making max use of the systems we created :) )

[DerToaster98]

Will be fixed in the next update

[DerToaster98]

Fixed in update 6.10B