Change DefaultByteBufHolder.equals() to treat instances of different classes as not equal (#9855)

# Motivation:
`DefaultByteBufHolder.equals()` considers another object equal if it's an instance of `ByteBufferHolder` and if the contents of two objects are equal. However, the behavior of `equals` method is not a part of the `ByteBufHolder` contract so `DefaultByteBufHolder`'s version may be causing violation of the symmetric property if other classes have different logic.
There are already a few classes that are affected by this: `DefaultHttp2GoAwayFrame`, `DefaultHttp2UnknownFrame`, and `SctpMessage` are all overriding `equals` method breaking the symmetric property.
Another effect of this behavior is that all instances with empty data are considered equal. That may not be desireable in the situations when instances are created for predefined constants, e.g. `FullBulkStringRedisMessage.NULL_INSTANCE` and `FullBulkStringRedisMessage.EMPTY_INSTANCE` in `codec-redis`. 

# Modification:
Make `DefaultByteBufHolder.equals()` implementation only work for the objects of the same class.

# Result:
- The symmetric property of the `equals` method is restored for the classes in question.
- Instances of different classes are not considered equal even if the content of the data they hold are the same.

buffer/src/main/java/io/netty/buffer/DefaultByteBufHolder.java

@@ -135,13 +135,24 @@ public class DefaultByteBufHolder implements ByteBufHolder {
         return StringUtil.simpleClassName(this) + '(' + contentToString() + ')';
     }
 
+    /**
+     * This implementation of the {@code equals} operation is restricted to
+     * work only with instances of the same class. The reason for that is that
+     * Netty library already has a number of classes that extend {@link DefaultByteBufHolder} and
+     * override {@code equals} method with an additional comparison logic and we
+     * need the symmetric property of the {@code equals} operation to be preserved.
+     *
+     * @param   o   the reference object with which to compare.
+     * @return  {@code true} if this object is the same as the obj
+     *          argument; {@code false} otherwise.
+     */
     @Override
     public boolean equals(Object o) {
         if (this == o) {
             return true;
         }
-        if (o instanceof ByteBufHolder) {
+        if (o != null && getClass() == o.getClass()) {
-            return data.equals(((ByteBufHolder) o).content());
+            return data.equals(((DefaultByteBufHolder) o).data);
         }
         return false;
     }

buffer/src/test/java/io/netty/buffer/DefaultByteBufHolderTest.java

@@ -42,4 +42,64 @@ public class DefaultByteBufHolderTest {
             copy.release();
         }
     }
+
+    @SuppressWarnings("SimplifiableJUnitAssertion")
+    @Test
+    public void testDifferentClassesAreNotEqual() {
+        // all objects here have EMPTY_BUFFER data but are instances of different classes
+        // so we want to check that none of them are equal to another.
+        ByteBufHolder dflt = new DefaultByteBufHolder(Unpooled.EMPTY_BUFFER);
+        ByteBufHolder other = new OtherByteBufHolder(Unpooled.EMPTY_BUFFER, 123);
+        ByteBufHolder constant1 = new DefaultByteBufHolder(Unpooled.EMPTY_BUFFER) {
+            // intentionally empty
+        };
+        ByteBufHolder constant2 = new DefaultByteBufHolder(Unpooled.EMPTY_BUFFER) {
+            // intentionally empty
+        };
+        try {
+            // not using 'assertNotEquals' to be explicit about which object we are calling .equals() on
+            assertFalse(dflt.equals(other));
+            assertFalse(dflt.equals(constant1));
+            assertFalse(constant1.equals(dflt));
+            assertFalse(constant1.equals(other));
+            assertFalse(constant1.equals(constant2));
+        } finally {
+            dflt.release();
+            other.release();
+            constant1.release();
+            constant2.release();
+        }
+    }
+
+    private static class OtherByteBufHolder extends DefaultByteBufHolder {
+
+        private final int extraField;
+
+        OtherByteBufHolder(final ByteBuf data, final int extraField) {
+            super(data);
+            this.extraField = extraField;
+        }
+
+        @Override
+        public boolean equals(final Object o) {
+            if (this == o) {
+                return true;
+            }
+            if (o == null || getClass() != o.getClass()) {
+                return false;
+            }
+            if (!super.equals(o)) {
+                return false;
+            }
+            final OtherByteBufHolder that = (OtherByteBufHolder) o;
+            return extraField == that.extraField;
+        }
+
+        @Override
+        public int hashCode() {
+            int result = super.hashCode();
+            result = 31 * result + extraField;
+            return result;
+        }
+    }
 }

codec-http2/src/test/java/io/netty/handler/codec/http2/Http2DefaultFramesTest.java

@@ -0,0 +1,44 @@
+/*
+ * Copyright 2019 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License, version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package io.netty.handler.codec.http2;
+
+import io.netty.buffer.DefaultByteBufHolder;
+import io.netty.buffer.Unpooled;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+
+public class Http2DefaultFramesTest {
+
+    @SuppressWarnings("SimplifiableJUnitAssertion")
+    @Test
+    public void testEqualOperation() {
+        // in this case, 'goAwayFrame' and 'unknownFrame' will also have an EMPTY_BUFFER data
+        // so we want to check that 'dflt' will not consider them equal.
+        DefaultHttp2GoAwayFrame goAwayFrame = new DefaultHttp2GoAwayFrame(1);
+        DefaultHttp2UnknownFrame unknownFrame = new DefaultHttp2UnknownFrame((byte) 1, new Http2Flags((short) 1));
+        DefaultByteBufHolder dflt = new DefaultByteBufHolder(Unpooled.EMPTY_BUFFER);
+        try {
+            // not using 'assertNotEquals' to be explicit about which object we are calling .equals() on
+            assertFalse(dflt.equals(goAwayFrame));
+            assertFalse(dflt.equals(unknownFrame));
+        } finally {
+            goAwayFrame.release();
+            unknownFrame.release();
+            dflt.release();
+        }
+    }
+}

codec-redis/src/test/java/io/netty/handler/codec/redis/RedisDecoderTest.java

@@ -306,4 +306,12 @@ public class RedisDecoderTest {
         ReferenceCountUtil.release(msg);
         ReferenceCountUtil.release(childBuf);
     }
+
+    @Test
+    public void testPredefinedMessagesNotEqual() {
+        // both EMPTY_INSTANCE and NULL_INSTANCE have EMPTY_BUFFER as their 'data',
+        // however we need to check that they are not equal between themselves.
+        assertNotEquals(FullBulkStringRedisMessage.EMPTY_INSTANCE, FullBulkStringRedisMessage.NULL_INSTANCE);
+        assertNotEquals(FullBulkStringRedisMessage.NULL_INSTANCE, FullBulkStringRedisMessage.EMPTY_INSTANCE);
+    }
 }