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

AbstractHttp2StreamFrame and DefaultHttp2PingFrame hashCode seems incorrect #13659

Open
Marcono1234 opened this issue Oct 14, 2023 · 4 comments · May be fixed by #13903
Open

AbstractHttp2StreamFrame and DefaultHttp2PingFrame hashCode seems incorrect #13659

Marcono1234 opened this issue Oct 14, 2023 · 4 comments · May be fixed by #13903

Comments

@Marcono1234
Copy link

Expected behavior

The hashCode methods should return the same result for objects which are considered equal, according to equals.

Actual behavior

The hashCode methods return different results.
The reason is that they call super.hashCode, which is actually Object.hashCode, and therefore differs between instances.

For DefaultHttp2PingFrame a possible solution (which would also include the content field) could look like this:

public int hashCode() {
    return Boolean.hashCode(ack) * 31 + Long.hashCode(content);
}

Steps to reproduce

see reproducer code below

Minimal yet complete reproducer code (or URL to code)

  • AbstractHttp2StreamFrame:
    // For simplicity create a dummy subclass here, but can also be seen with the other subclasses
    class C extends AbstractHttp2StreamFrame {
        // Doesn't matter
        @Override public String name() {
            return null;
        }
    }
    
    Object o1 = new C();
    Object o2 = new C();
    
    // This works
    System.out.println("Equals: " + o1.equals(o2));
    // This doesn't work; prints different hashCode values
    System.out.println("hashCode:\n  o1: " + o1.hashCode() + "\n  o2: " + o2.hashCode());
  • DefaultHttp2PingFrame:
    Object o1 = new DefaultHttp2PingFrame(1, true);
    Object o2 = new DefaultHttp2PingFrame(1, true);
    
    // This works
    System.out.println("Equals: " + o1.equals(o2));
    // This doesn't work; prints different hashCode values
    System.out.println("hashCode:\n  o1: " + o1.hashCode() + "\n  o2: " + o2.hashCode());

Netty version

4.1.100.Final

JVM version (e.g. java -version)

Java 17

OS version (e.g. uname -a)

Windows 10

@normanmaurer
Copy link
Member

@Marcono1234 sounds like a bug... Can you please open A PR ?

@Marcono1234
Copy link
Author

@normanmaurer, will this require me to sign the Contributor License Agreement? In that case, sorry I won't create a pull request.
Otherwise I can try it, but I am not familiar with the development setup of this project at all. I just found this issue while analyzing the code and thought it might be useful if I reported it.

@normanmaurer
Copy link
Member

@Marcono1234 yes it will require signing a CLA

@Marcono1234
Copy link
Author

Ok, thanks for the confirmation. As mentioned above, in that case I will not submit a PR, sorry.

normanmaurer added a commit that referenced this issue Mar 12, 2024
Motivation:

Our implementations of hashCode() and equals(...) were not correct for some frames.

Modifications:

Correctly implement both methods

Result:

Fixes #13659
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 a pull request may close this issue.

2 participants