Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dividing by zero crashes of texture modifiers #14224

Merged
merged 1 commit into from Jan 7, 2024

Conversation

cx384
Copy link
Contributor

@cx384 cx384 commented Jan 7, 2024

  • This fixes client crash with invalid verticalframe texture modifier #14223
  • I also found a similar problem with ^[sheet which crashes when the width or height is zero, so this also gets fixed.
  • Moreover, I removed some tailoring whitespaces.
  • Imo the whole texture modifiers code needs to get refactored, but to be easier mergeable this PR only removes bugs that cause crashes. To name some further things that should be changed, unnecessary code duplication in the error code, many casts from int to u32 when using stoi (this causes unexpected integer overflows when using negative numbers) or some checks if the values are valid, e.g. whether frame_count > frame_index.

To do

This PR is Ready for Review.

How to test

Test if it does not crash anymore for verticalframe or sheet.

minetest.register_chatcommand("crashme", {
    func = function(name)
        minetest.show_formspec(name, "vframe:crashme", [[
            size[1,1]
            bgcolor[#FFFFFF]
            image[0,0;1,1;crack_anylength.png^\[verticalframe:0:0]
            image[0,0;1,1;crack_anylength.png^\[sheet:0x0:1,1]
        ]])
    end,
})

@cx384 cx384 force-pushed the texture_modifier_crash branch 2 times, most recently from 5a46863 to 657814c Compare January 7, 2024 07:07
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works on my PC.

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sfan5 sfan5 merged commit 2766c70 into minetest:master Jan 7, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

client crash with invalid verticalframe texture modifier
4 participants