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

[Core] Fix crash when hashing empty CharString #85389

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Nov 26, 2023

Minimal fix, could make hash_djb2 handle nullptr gracefully but limited it to fixing this direct issue here. get_data always returns a non-null pointer.

hash_djb2 is not used directly anywhere in the engine, only HashMapHasherDefault uses it, though you could have issues if you feed it a plain string, but I don't know if that's a major risk here, I think it's fair to assume that these methods should be called with non-null arguments.

If I am to change that to handle this gracefully I'd also change the String::hash methods taking a pointer as they all also assume it to be non-null.

@AThousandShips AThousandShips added bug topic:core crash cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Nov 26, 2023
@AThousandShips AThousandShips added this to the 4.3 milestone Nov 26, 2023
@AThousandShips AThousandShips requested a review from a team as a code owner November 26, 2023 16:38
@akien-mga akien-mga merged commit 8174bce into godotengine:master Dec 4, 2023
15 checks passed
@AThousandShips AThousandShips deleted the hash_fix branch December 4, 2023 22:21
@akien-mga
Copy link
Member

Thanks!

@AThousandShips
Copy link
Member Author

Thank you!

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.2.1.

@YuriSizov YuriSizov removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 5, 2023
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 23, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.4.

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.

Empty string in a HashMap results in a crash
4 participants