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

Hash collisions of item stacks causing wrong items to be stored #501

Open
kurrycat2004 opened this issue Oct 13, 2024 · 8 comments
Open

Comments

@kurrycat2004
Copy link

Describe the bug

AEItemStackRegistry#getRegisteredStack(ItemStack) only compares the hash of the input stack, without comparing the actual item stacks, making it vulnerable to hash collisions. This causes new items put into a system to "transform" into the already existing item if it has the same hash.

To Reproduce

  • make a simple ME system
  • put a Protection 1 enchanted book in
  • put a Fire Protection 2 enchanted book in
  • system now contains 2 Protection 1 books

Expected behavior

ME System correctly stores one Protection 1 book and one Fire Protection 2 book

Additional context

javaw_JnTSTzqIUE.mp4

Environment

Tested with only AE2

  • Minecraft Version: 1.12.2
  • AE2 Version: v0.56.6
  • Forge Version: 14.23.5.2847
@PukPukov
Copy link

Is this an issue in original AE?

@kurrycat2004
Copy link
Author

from what i could see, no

@embeddedt
Copy link

embeddedt commented Nov 2, 2024

This appears to be a regression from 0.56.6; downgrading to 0.56.5 fixes it. Most likely this commit: 724f3ba

@Exaxxion
Copy link

Exaxxion commented Nov 2, 2024

For what it's worth, 0.56.6 has known issues and I was advised months ago not to update to it because of that.

@ghzdude
Copy link

ghzdude commented Nov 2, 2024

this is concerning for CEu, since it uses the same kind of hash strategy

@IntegerLimit
Copy link

If CEu does, wouldn't it use a normal map, and not a weak map? From what I see in 724f3ba, I believe AE2 is using a weak map, which... doesn't compare equality, just hash, right?

At least, that is what I believe is happening, as normal maps check both hash and the equals function.

@ghzdude
Copy link

ghzdude commented Nov 3, 2024

CEu uses this in OpenCustomHashMaps (from fastutil), i'm not sure if those use both hash and identity equals.

@IntegerLimit
Copy link

Yes, that should also check identity equals. AE2 seems to use this: 724f3ba, a 'Weak Values' Map from Google. Not sure if it is what I thought.

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

No branches or pull requests

6 participants