Repository: netty/netty
Issue: 4.0 HttpHeader.set(HttpHeader) will cause itself to be emptied when passed self

public HttpHeaders set(HttpHeaders headers) {
        if (headers == null) {
            throw new NullPointerException("headers");
        }
        clear();
        for (Map.Entry<String, String> e: headers) {
            add(e.getKey(), e.getValue());
        }
        return this;
}

this is in conflict with DefaultHttpHeaders which will throw an exception when doing the same thing so there is inconsistency in the interface contract


Issue comments:
@louiscryan - I think we can close this now?


Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
index 3b3d9eca61b..7e5b986f54a 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java
@@ -77,15 +77,14 @@ public HttpHeaders add(HttpHeaders headers) {
     @Override
     public HttpHeaders set(HttpHeaders headers) {
         if (headers instanceof DefaultHttpHeaders) {
-            if (headers == this) {
-                throw new IllegalArgumentException("can't add to itself.");
-            }
-            clear();
-            DefaultHttpHeaders defaultHttpHeaders = (DefaultHttpHeaders) headers;
-            HeaderEntry e = defaultHttpHeaders.head.after;
-            while (e != defaultHttpHeaders.head) {
-                add(e.key, e.value);
-                e = e.after;
+            if (headers != this) {
+                clear();
+                DefaultHttpHeaders defaultHttpHeaders = (DefaultHttpHeaders) headers;
+                HeaderEntry e = defaultHttpHeaders.head.after;
+                while (e != defaultHttpHeaders.head) {
+                    add(e.key, e.value);
+                    e = e.after;
+                }
             }
             return this;
         } else {
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java
index dd7a198bf7e..ab18acad1e5 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java
@@ -1617,9 +1617,11 @@ public HttpHeaders set(HttpHeaders headers) {
         if (headers == null) {
             throw new NullPointerException("headers");
         }
-        clear();
-        for (Map.Entry<String, String> e: headers) {
-            add(e.getKey(), e.getValue());
+        if (headers != this) {
+            clear();
+            for (Map.Entry<String, String> e : headers) {
+                add(e.getKey(), e.getValue());
+            }
         }
         return this;
     }


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpHeadersTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpHeadersTest.java
index e2eadc33cab..21827c7747f 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpHeadersTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpHeadersTest.java
@@ -74,9 +74,12 @@ public void testAddSelf() {
         headers.add(headers);
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testSetSelf() {
+    @Test
+    public void testSetSelfIsNoOp() {
         HttpHeaders headers = new DefaultHttpHeaders(false);
+        headers.add("some", "thing");
         headers.set(headers);
+        Assert.assertEquals(1, headers.entries().size());
+        Assert.assertEquals("thing", headers.get("some"));
     }
 }