Enforce status code validation in CloseWebSocketFrame (#10846)

Motivation:

According to specification 1006 status code must not be set as a status code in a
Close control frame by the endpoint. However 1006 status code can be
used in applications to indicate that the connection was closed abnormally.

Modifications:

- Enforce status code validation in CloseWebSocketFrame
- Add WebSocketCloseStatus construction with disabled validation
- Add test

Result:

Fixes #10838

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

@@ -40,7 +40,7 @@ public class CloseWebSocketFrame extends WebSocketFrame {
      *            example, <tt>1000</tt> indicates normal closure.
      */
     public CloseWebSocketFrame(WebSocketCloseStatus status) {
-        this(status.code(), status.reasonText());
+        this(requireValidStatusCode(status.code()), status.reasonText());
     }
 
     /**
@@ -53,7 +53,7 @@ public class CloseWebSocketFrame extends WebSocketFrame {
      *            Reason text. Set to null if no text.
      */
     public CloseWebSocketFrame(WebSocketCloseStatus status, String reasonText) {
-        this(status.code(), reasonText);
+        this(requireValidStatusCode(status.code()), reasonText);
     }
 
     /**
@@ -66,7 +66,7 @@ public class CloseWebSocketFrame extends WebSocketFrame {
      *            Reason text. Set to null if no text.
      */
     public CloseWebSocketFrame(int statusCode, String reasonText) {
-        this(true, 0, statusCode, reasonText);
+        this(true, 0, requireValidStatusCode(statusCode), reasonText);
     }
 
     /**
@@ -95,7 +95,7 @@ public class CloseWebSocketFrame extends WebSocketFrame {
      *            Reason text. Set to null if no text.
      */
     public CloseWebSocketFrame(boolean finalFragment, int rsv, int statusCode, String reasonText) {
-        super(finalFragment, rsv, newBinaryData(statusCode, reasonText));
+        super(finalFragment, rsv, newBinaryData(requireValidStatusCode(statusCode), reasonText));
     }
 
     private static ByteBuf newBinaryData(int statusCode, String reasonText) {
@@ -201,4 +201,13 @@ public class CloseWebSocketFrame extends WebSocketFrame {
         super.touch(hint);
         return this;
     }
+
+    static int requireValidStatusCode(int statusCode) {
+        if (WebSocketCloseStatus.isValidStatusCode(statusCode)) {
+            return statusCode;
+        } else {
+            throw new IllegalArgumentException("WebSocket close status code does NOT comply with RFC-6455: " +
+                    statusCode);
+        }
+    }
 }

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

@@ -208,16 +208,26 @@ public final class WebSocketCloseStatus implements Comparable<WebSocketCloseStat
 
     // 1004, 1005, 1006, 1015 are reserved and should never be used by user
     //public static final WebSocketCloseStatus SPECIFIC_MEANING = register(1004, "...");
-    //public static final WebSocketCloseStatus EMPTY = register(1005, "Empty");
+
-    //public static final WebSocketCloseStatus ABNORMAL_CLOSURE = register(1006, "Abnormal closure");
+    public static final WebSocketCloseStatus EMPTY =
-    //public static final WebSocketCloseStatus TLS_HANDSHAKE_FAILED(1015, "TLS handshake failed");
+        new WebSocketCloseStatus(1005, "Empty", false);
+
+    public static final WebSocketCloseStatus ABNORMAL_CLOSURE =
+        new WebSocketCloseStatus(1006, "Abnormal closure", false);
+
+    public static final WebSocketCloseStatus TLS_HANDSHAKE_FAILED =
+        new WebSocketCloseStatus(1015, "TLS handshake failed", false);
 
     private final int statusCode;
     private final String reasonText;
     private String text;
 
     public WebSocketCloseStatus(int statusCode, String reasonText) {
-        if (!isValidStatusCode(statusCode)) {
+        this(statusCode, reasonText, true);
+    }
+
+    public WebSocketCloseStatus(int statusCode, String reasonText, boolean validate) {
+        if (validate && !isValidStatusCode(statusCode)) {
             throw new IllegalArgumentException(
                 "WebSocket close status code does NOT comply with RFC-6455: " + statusCode);
         }
@@ -290,6 +300,10 @@ public final class WebSocketCloseStatus implements Comparable<WebSocketCloseStat
                 return PROTOCOL_ERROR;
             case 1003:
                 return INVALID_MESSAGE_TYPE;
+            case 1005:
+                return EMPTY;
+            case 1006:
+                return ABNORMAL_CLOSURE;
             case 1007:
                 return INVALID_PAYLOAD_DATA;
             case 1008:
@@ -306,6 +320,8 @@ public final class WebSocketCloseStatus implements Comparable<WebSocketCloseStat
                 return TRY_AGAIN_LATER;
             case 1014:
                 return BAD_GATEWAY;
+            case 1015:
+                return TLS_HANDSHAKE_FAILED;
             default:
                 return new WebSocketCloseStatus(code, "Close status #" + code);
         }

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

@@ -0,0 +1,79 @@
+/*
+ * 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:
+ *
+ * https://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 org.assertj.core.api.ThrowableAssert;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+
+class CloseWebSocketFrameTest {
+
+    @Test
+    void testInvalidCode() {
+        doTestInvalidCode(new ThrowableAssert.ThrowingCallable() {
+
+            @Override
+            public void call() throws RuntimeException {
+                new CloseWebSocketFrame(WebSocketCloseStatus.ABNORMAL_CLOSURE);
+            }
+        });
+
+        doTestInvalidCode(new ThrowableAssert.ThrowingCallable() {
+
+            @Override
+            public void call() throws RuntimeException {
+                new CloseWebSocketFrame(WebSocketCloseStatus.ABNORMAL_CLOSURE, "invalid code");
+            }
+        });
+
+        doTestInvalidCode(new ThrowableAssert.ThrowingCallable() {
+
+            @Override
+            public void call() throws RuntimeException {
+                new CloseWebSocketFrame(1006, "invalid code");
+            }
+        });
+
+        doTestInvalidCode(new ThrowableAssert.ThrowingCallable() {
+
+            @Override
+            public void call() throws RuntimeException {
+                new CloseWebSocketFrame(true, 0, 1006, "invalid code");
+            }
+        });
+    }
+
+    @Test
+    void testValidCode() {
+        doTestValidCode(new CloseWebSocketFrame(WebSocketCloseStatus.NORMAL_CLOSURE),
+                WebSocketCloseStatus.NORMAL_CLOSURE.code(), WebSocketCloseStatus.NORMAL_CLOSURE.reasonText());
+
+        doTestValidCode(new CloseWebSocketFrame(WebSocketCloseStatus.NORMAL_CLOSURE, "valid code"),
+                WebSocketCloseStatus.NORMAL_CLOSURE.code(), "valid code");
+
+        doTestValidCode(new CloseWebSocketFrame(1000, "valid code"), 1000, "valid code");
+
+        doTestValidCode(new CloseWebSocketFrame(true, 0, 1000, "valid code"), 1000, "valid code");
+    }
+
+    private static void doTestInvalidCode(ThrowableAssert.ThrowingCallable callable) {
+        assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(callable);
+    }
+
+    private static void doTestValidCode(CloseWebSocketFrame frame, int expectedCode, String expectedReason) {
+        assertThat(frame.statusCode()).isEqualTo(expectedCode);
+        assertThat(frame.reasonText()).isEqualTo(expectedReason);
+    }
+}

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

@@ -18,9 +18,11 @@ import java.util.List;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
+import org.assertj.core.api.ThrowableAssert;
 import org.hamcrest.Matchers;
 import org.junit.Test;
 
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotSame;
@@ -56,6 +58,8 @@ public class WebSocketCloseStatusTest {
         assertSame(ENDPOINT_UNAVAILABLE, valueOf(1001));
         assertSame(PROTOCOL_ERROR, valueOf(1002));
         assertSame(INVALID_MESSAGE_TYPE, valueOf(1003));
+        assertSame(EMPTY, valueOf(1005));
+        assertSame(ABNORMAL_CLOSURE, valueOf(1006));
         assertSame(INVALID_PAYLOAD_DATA, valueOf(1007));
         assertSame(POLICY_VIOLATION, valueOf(1008));
         assertSame(MESSAGE_TOO_BIG, valueOf(1009));
@@ -64,6 +68,7 @@ public class WebSocketCloseStatusTest {
         assertSame(SERVICE_RESTART, valueOf(1012));
         assertSame(TRY_AGAIN_LATER, valueOf(1013));
         assertSame(BAD_GATEWAY, valueOf(1014));
+        assertSame(TLS_HANDSHAKE_FAILED, valueOf(1015));
     }
 
     @Test
@@ -127,4 +132,23 @@ public class WebSocketCloseStatusTest {
         invalidCodes.retainAll(knownCodes);
         assertEquals(invalidCodes, Collections.emptySet());
     }
+
+    @Test
+    public void testValidationEnabled() {
+        assertThatExceptionOfType(IllegalArgumentException.class)
+                .isThrownBy(new ThrowableAssert.ThrowingCallable() {
+
+                    @Override
+                    public void call() throws RuntimeException {
+                        new WebSocketCloseStatus(1006, "validation disabled");
+                    }
+                });
+    }
+
+    @Test
+    public void testValidationDisabled() {
+        WebSocketCloseStatus status = new WebSocketCloseStatus(1006, "validation disabled", false);
+        assertEquals(1006, status.code());
+        assertEquals("validation disabled", status.reasonText());
+    }
 }