grorp / ggraffiti

Graffiti mod for Minetest
GNU Affero General Public License v3.0
8 stars 4 forks source link

Crash when bytes exceed 1024 (no idea how to reproduce) #12

Closed KaylebJay closed 5 months ago

KaylebJay commented 7 months ago
2024-04-12 23:06:10: ERROR[Main]: ServerError: AsyncErr: Lua: Runtime error from mod 'ggraffiti' in callback environment_Step(): /home/vpsuser/minetest/bin/../builtin/game/misc_s.lua:76: Incorrect length of 'data', width and height imply 1024 bytes but 1040 were provided
2024-04-12 23:06:10: ERROR[Main]: stack traceback:
2024-04-12 23:06:10: ERROR[Main]:     [C]: in function 'error'
2024-04-12 23:06:10: ERROR[Main]:     /home/vpsuser/minetest/bin/../builtin/game/misc_s.lua:76: in function 'encode_png'
2024-04-12 23:06:10: ERROR[Main]:     /home/vpsuser/minetest/bin/../mods/ggraffiti/src/canvas.lua:103: in function 'update_immediately'
2024-04-12 23:06:10: ERROR[Main]:     /home/vpsuser/minetest/bin/../mods/ggraffiti/src/canvas.lua:183: in function 'update_canvases'
2024-04-12 23:06:10: ERROR[Main]:     ...vpsuser/minetest/bin/../mods/ggraffiti/src/spraycast.lua:323: in function 'after_spraycasts'
2024-04-12 23:06:10: ERROR[Main]:     /home/vpsuser/minetest/bin/../mods/ggraffiti/src/items.lua:218: in function 'func'
2024-04-12 23:06:10: ERROR[Main]:     ...ser/minetest/bin/../builtin/profiler/instrumentation.lua:108: in function <...ser/minetest/bin/../builtin/profiler/instrumentation.lua:101>
2024-04-12 23:06:10: ERROR[Main]:     /home/vpsuser/minetest/bin/../builtin/common/register.lua:26: in function </home/vpsuser/minetest/bin/../builtin/common/register.lua:12>
grorp commented 6 months ago

The quick but wrong fix is truncating the length of "self.bitmap" to match expectations here:

https://github.com/grorp/ggraffiti/blob/c5ff4f1b3d5d3689499b6940d77eb1d439728b7b/src/canvas.lua#L102-L104

... but that would only hide some underlying bug. If you're running a server, you probably want the quick fix. To do a real fix, I need a way to reproduce. Also, do you use a modified version of ggraffiti?

KaylebJay commented 6 months ago

I do use a slightly modified version of ggraffiti, but nothing is changed except a few items are de-registered, and a few other items are registered.

You could test our live deployment of ggraffiti on the server- it's the server Tunnelers' Abyss. We could do the quick fix, but really the stranger thing would be how 1040 bytes could appear there anyways - it's a bug, but I only wish I knew how to reproduce it.

grorp commented 6 months ago

Can I find the source code somewhere? Who knows, maybe your custom canvas nodes are related to #11.

I don't want to spend time farming materials on that server just to find out whether I can reproduce a bug. If I can reproduce the bug, I'd like to be able to debug it.

KaylebJay commented 6 months ago

Sure, what is your username on Gitlab? I'll add you to our repo

mruncreative commented 6 months ago

A user of my mod Blood Splatter (uses this mod as a base) got it too. https://content.minetest.net/threads/8287/ There must be something wrong with the math. I think bitmap data is only manipulated in canvas. Maybe it's indexing too far in drawrect or drawpixel and thus adding to it? In his case it was 1088 bytes which means one row got added somehow (16x17 image) but in this issue it's different so it varies.

grorp commented 6 months ago

Sure, what is your username on Gitlab? I'll add you to our repo

Nice, I am https://gitlab.com/grorp

grorp commented 6 months ago

I did some testing (https://github.com/grorp/ggraffiti/commit/17c004a40efb66fd74a5074484a10eb109608bc6) and the only case where the bitmap should be able to get an incorrect length is with an out-of-bounds CanvasEntity:draw_pixel call. I don't know how this can happen, but I added a bounds check to draw_pixel. I hope that fixes the error. Please test again.

mruncreative commented 6 months ago

ContentDB user ClassicAdam ran Blood Splatter for 6 hours and had no crashes with the bounds check. https://content.minetest.net/threads/8287/

KaylebJay commented 6 months ago

I've now deployed the changes on our server, will let you know how it goes. I also added you to our repo, btw :)

KaylebJay commented 5 months ago

I haven't seen this issue again, so I'm closing this issue for now - if it comes back up I'll reopen. Thank you!