Skip to content

fix: Incorrect abs of hashes in BadMap #817

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

Merged

Conversation

johanandren
Copy link
Collaborator

Can lead to negative array indexes and throw out of bounds exception on config load

Can lead to negative array indexes and throw out of bounds exception.
@johanandren johanandren force-pushed the wip-fix-badmap-entry-hashing branch from 30a7f78 to 82ce561 Compare June 9, 2025 07:43
@johanandren
Copy link
Collaborator Author

Reported over in one of the Pekko issue trackers: apache/pekko#1886

@johanandren
Copy link
Collaborator Author

Actually reported in this repo tracker as well, I had just missed it: #816

@A248
Copy link

A248 commented Jun 9, 2025

Seems like another case of premature optimization leading to broken behavior.

At this point, why not remove BadMap entirely? Seems like undue maintenance burden, particularly if it hasn't been robustly tested (as this PR demonstrates). The maintainers are already quite absent from this project and it could use a lessened maintenane burden.

@johanandren
Copy link
Collaborator Author

@A248 thanks for the input, we'll limit any fallout by only doing this minimal fix and not introduce a larger change, even if that could be something worth considering.

@johanandren johanandren closed this Jun 9, 2025
@johanandren johanandren reopened this Jun 9, 2025
@johanandren johanandren closed this Jun 9, 2025
@johanandren johanandren reopened this Jun 9, 2025
@johanandren johanandren merged commit 2d9b9ee into lightbend:main Jun 9, 2025
3 of 9 checks passed
@johanandren johanandren deleted the wip-fix-badmap-entry-hashing branch June 9, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants