andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Enforce status code validation in CloseWebSocketFrame (#10846)

Browse Source 
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
This commit is contained in:
Violeta Georgieva 2020-12-07 14:20:08 +02:00 committed by GitHub
parent 3ac9685580
commit 05093de0d6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 136 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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);
}
}
}

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

@ -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 TLS_HANDSHAKE_FAILED(1015, "TLS handshake failed");
public static final WebSocketCloseStatus EMPTY =
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);
}

79
codec-http/src/test/java/io/netty/handler/codec/http/websocketx/CloseWebSocketFrameTest.java Normal file
View File

@ -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);
}
}

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

@ -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());
}
}