Set (and override) reserved websocket handshake response headers after custom to avoid duplication (#10319)

Motivation:
Currently we passing custom websocket handshaker response headers to a `WebSocketServerHandshaker` but they can contain a reserved headers (e.g. Connection, Upgrade, Sec-Websocket-Accept) what lead to duplication because we use response.headers().add(..) instead of response.headers().set(..).

Modification:
In each `WebSocketServerHandshaker00`, ... `WebSocketServerHandshaker13` implementation replace the method add(..) to set(..) for reserved response headers.

Result:

Less error-prone

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java

@@ -147,8 +147,8 @@ public class WebSocketServerHandshaker00 extends WebSocketServerHandshaker {
             res.headers().add(headers);
         }
 
-        res.headers().add(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET);
+        res.headers().set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
-        res.headers().add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE);
+                     .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE);
 
         // Fill in the headers and contents depending on handshake getMethod.
         if (isHixie76) {

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java

@@ -149,9 +149,9 @@ public class WebSocketServerHandshaker07 extends WebSocketServerHandshaker {
             logger.debug("WebSocket version 07 server handshake key: {}, response: {}.", key, accept);
         }
 
-        res.headers().add(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET);
+        res.headers().set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
-        res.headers().add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE);
+                     .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
-        res.headers().add(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, accept);
+                     .set(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, accept);
 
         String subprotocols = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL);
         if (subprotocols != null) {

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java

@@ -155,9 +155,9 @@ public class WebSocketServerHandshaker08 extends WebSocketServerHandshaker {
             logger.debug("WebSocket version 08 server handshake key: {}, response: {}", key, accept);
         }
 
-        res.headers().add(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET);
+        res.headers().set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
-        res.headers().add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE);
+                     .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
-        res.headers().add(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, accept);
+                     .set(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, accept);
 
         String subprotocols = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL);
         if (subprotocols != null) {

codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java

@@ -153,9 +153,9 @@ public class WebSocketServerHandshaker13 extends WebSocketServerHandshaker {
             logger.debug("WebSocket version 13 server handshake key: {}, response: {}", key, accept);
         }
 
-        res.headers().add(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET);
+        res.headers().set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
-        res.headers().add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE);
+                     .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
-        res.headers().add(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, accept);
+                     .set(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, accept);
 
         String subprotocols = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL);
         if (subprotocols != null) {

codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java

@@ -36,7 +36,18 @@ import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
-public class WebSocketServerHandshaker00Test {
+public class WebSocketServerHandshaker00Test extends WebSocketServerHandshakerTest {
+
+    @Override
+    protected WebSocketServerHandshaker newHandshaker(String webSocketURL, String subprotocols,
+            WebSocketDecoderConfig decoderConfig) {
+        return new WebSocketServerHandshaker00(webSocketURL, subprotocols, decoderConfig);
+    }
+
+    @Override
+    protected WebSocketVersion webSocketVersion() {
+        return WebSocketVersion.V00;
+    }
 
     @Test
     public void testPerformOpeningHandshake() {

codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07Test.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2020 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.websocketx;
+
+public class WebSocketServerHandshaker07Test extends WebSocketServerHandshakerTest {
+
+    @Override
+    protected WebSocketServerHandshaker newHandshaker(String webSocketURL, String subprotocols,
+            WebSocketDecoderConfig decoderConfig) {
+        return new WebSocketServerHandshaker07(webSocketURL, subprotocols, decoderConfig);
+    }
+
+    @Override
+    protected WebSocketVersion webSocketVersion() {
+        return WebSocketVersion.V07;
+    }
+}

codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08Test.java

@@ -33,7 +33,18 @@ import org.junit.Test;
 
 import static io.netty.handler.codec.http.HttpVersion.*;
 
-public class WebSocketServerHandshaker08Test {
+public class WebSocketServerHandshaker08Test extends WebSocketServerHandshakerTest {
+
+    @Override
+    protected WebSocketServerHandshaker newHandshaker(String webSocketURL, String subprotocols,
+            WebSocketDecoderConfig decoderConfig) {
+        return new WebSocketServerHandshaker08(webSocketURL, subprotocols, decoderConfig);
+    }
+
+    @Override
+    protected WebSocketVersion webSocketVersion() {
+        return WebSocketVersion.V08;
+    }
 
     @Test
     public void testPerformOpeningHandshake() {

codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13Test.java

@@ -40,7 +40,18 @@ import java.util.Iterator;
 
 import static io.netty.handler.codec.http.HttpVersion.*;
 
-public class WebSocketServerHandshaker13Test {
+public class WebSocketServerHandshaker13Test extends WebSocketServerHandshakerTest {
+
+    @Override
+    protected WebSocketServerHandshaker newHandshaker(String webSocketURL, String subprotocols,
+            WebSocketDecoderConfig decoderConfig) {
+        return new WebSocketServerHandshaker13(webSocketURL, subprotocols, decoderConfig);
+    }
+
+    @Override
+    protected WebSocketVersion webSocketVersion() {
+        return WebSocketVersion.V13;
+    }
 
     @Test
     public void testPerformOpeningHandshake() {

codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshakerTest.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright 2020 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.websocketx;
+
+import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.DefaultHttpHeaders;
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.FullHttpResponse;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpVersion;
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public abstract class WebSocketServerHandshakerTest {
+
+    protected abstract WebSocketServerHandshaker newHandshaker(String webSocketURL, String subprotocols,
+            WebSocketDecoderConfig decoderConfig);
+
+    protected abstract WebSocketVersion webSocketVersion();
+
+    @Test
+    public void testDuplicateHandshakeResponseHeaders() {
+        WebSocketServerHandshaker serverHandshaker = newHandshaker("ws://example.com/chat",
+                                                                   "chat", WebSocketDecoderConfig.DEFAULT);
+        FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/chat");
+        request.headers()
+               .set(HttpHeaderNames.HOST, "example.com")
+               .set(HttpHeaderNames.ORIGIN, "example.com")
+               .set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
+               .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
+               .set(HttpHeaderNames.SEC_WEBSOCKET_KEY, "dGhlIHNhbXBsZSBub25jZQ==")
+               .set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, "http://example.com")
+               .set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, "chat, superchat")
+               .set(HttpHeaderNames.SEC_WEBSOCKET_VERSION, webSocketVersion().toAsciiString());
+        HttpHeaders customResponseHeaders = new DefaultHttpHeaders();
+        // set duplicate required headers and one custom
+        customResponseHeaders
+                .set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
+                .set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
+                .set("custom", "header");
+
+        if (webSocketVersion() != WebSocketVersion.V00) {
+            customResponseHeaders.set(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, "12345");
+        }
+
+        FullHttpResponse response = null;
+        try {
+            response = serverHandshaker.newHandshakeResponse(request, customResponseHeaders);
+            HttpHeaders responseHeaders = response.headers();
+
+            assertEquals(1, responseHeaders.getAll(HttpHeaderNames.CONNECTION).size());
+            assertEquals(1, responseHeaders.getAll(HttpHeaderNames.UPGRADE).size());
+            assertTrue(responseHeaders.containsValue("custom", "header", true));
+            if (webSocketVersion() != WebSocketVersion.V00) {
+                assertFalse(responseHeaders.containsValue(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, "12345", false));
+            }
+        } finally {
+            request.release();
+            if (response != null) {
+                response.release();
+            }
+        }
+    }
+}