Repository: netty/netty
Issue: MemoryFileUpload & DiskFileUpload NOT equals itself
Labels: defect

test code:

```
@Test
public final void testMemoryFileUploadEquals() {
    final MemoryFileUpload f1 = 
            new MemoryFileUpload("m1", "m1", "application/json", null, null, 100);

    assertEquals(f1, f1);
}

@Test
public final void testDiskFileUploadEquals() {
    final DiskFileUpload f2 = 
            new DiskFileUpload("d1", "d1", "application/json", null, null, 100);

    assertEquals(f2, f2);
}
```

The two testcase above have failed.

please review equals's code （[MemoryFileUpload.java](https://github.com/netty/netty/blob/4.1/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryFileUpload.java#L71)）

```
@Override
public boolean equals(Object o) {
    if (!(o instanceof Attribute)) {
        return false;
    }
    Attribute attribute = (Attribute) o;
    return getName().equalsIgnoreCase(attribute.getName());
}
```


Issue comments:
yes the equals implementation does seem suspect ... @veebs wdyt?

Fixed by https://github.com/netty/netty/pull/5527


Pull request review comments:
nit: kill line

do we have any context for how this was intended to be used? It seems strange to compare the length, but not compare the actual content. If we don't do this consider just making this method `return upload1.getName().compareToIgnoreCase(upload2.getName());`

@Scottmitch no context here.. Let me just remove the TODO and change to what you suggested.


Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskFileUpload.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskFileUpload.java
index 1932004dd6b..8c253b43d23 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskFileUpload.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskFileUpload.java
@@ -69,16 +69,12 @@ public void setFilename(String filename) {
 
     @Override
     public int hashCode() {
-        return getName().hashCode();
+        return FileUploadUtil.hashCode(this);
     }
 
     @Override
     public boolean equals(Object o) {
-        if (!(o instanceof Attribute)) {
-            return false;
-        }
-        Attribute attribute = (Attribute) o;
-        return getName().equalsIgnoreCase(attribute.getName());
+        return o instanceof FileUpload && FileUploadUtil.equals(this, (FileUpload) o);
     }
 
     @Override
@@ -91,13 +87,7 @@ public int compareTo(InterfaceHttpData o) {
     }
 
     public int compareTo(FileUpload o) {
-        int v;
-        v = getName().compareToIgnoreCase(o.getName());
-        if (v != 0) {
-            return v;
-        }
-        // TODO should we compare size ?
-        return v;
+        return FileUploadUtil.compareTo(this, o);
     }
 
     @Override
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/FileUploadUtil.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/FileUploadUtil.java
new file mode 100644
index 00000000000..11b9b85e4a6
--- /dev/null
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/FileUploadUtil.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2016 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.http.multipart;
+
+final class FileUploadUtil {
+
+    private FileUploadUtil() { }
+
+    static int hashCode(FileUpload upload) {
+        return upload.getName().hashCode();
+    }
+
+    static boolean equals(FileUpload upload1, FileUpload upload2) {
+        return upload1.getName().equalsIgnoreCase(upload2.getName());
+    }
+
+    static int compareTo(FileUpload upload1, FileUpload upload2) {
+        return upload1.getName().compareToIgnoreCase(upload2.getName());
+    }
+}
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryFileUpload.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryFileUpload.java
index c644e84db28..141bdd55bbe 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryFileUpload.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryFileUpload.java
@@ -63,16 +63,12 @@ public void setFilename(String filename) {
 
     @Override
     public int hashCode() {
-        return getName().hashCode();
+        return FileUploadUtil.hashCode(this);
     }
 
     @Override
     public boolean equals(Object o) {
-        if (!(o instanceof Attribute)) {
-            return false;
-        }
-        Attribute attribute = (Attribute) o;
-        return getName().equalsIgnoreCase(attribute.getName());
+        return o instanceof FileUpload && FileUploadUtil.equals(this, (FileUpload) o);
     }
 
     @Override
@@ -85,13 +81,7 @@ public int compareTo(InterfaceHttpData o) {
     }
 
     public int compareTo(FileUpload o) {
-        int v;
-        v = getName().compareToIgnoreCase(o.getName());
-        if (v != 0) {
-            return v;
-        }
-        // TODO should we compare size for instance ?
-        return v;
+        return FileUploadUtil.compareTo(this, o);
     }
 
     @Override


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
new file mode 100644
index 00000000000..fa5661e379d
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2016 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.http.multipart;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class DiskFileUploadTest {
+
+    @Test
+    public final void testDiskFileUploadEquals() {
+        DiskFileUpload f2 =
+                new DiskFileUpload("d1", "d1", "application/json", null, null, 100);
+        Assert.assertEquals(f2, f2);
+    }
+}
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/MemoryFileUploadTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/MemoryFileUploadTest.java
new file mode 100644
index 00000000000..4d53f49e55b
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/MemoryFileUploadTest.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright 2016 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.http.multipart;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class MemoryFileUploadTest {
+
+    @Test
+    public final void testMemoryFileUploadEquals() {
+        MemoryFileUpload f1 =
+                new MemoryFileUpload("m1", "m1", "application/json", null, null, 100);
+        Assert.assertEquals(f1, f1);
+    }
+}