From dcc175b4bc2a32759e684398e9e433577c35eb49 Mon Sep 17 00:00:00 2001
From: Santiago Pericasgeertsen <santiago.pericasgeertsen@oracle.com>
Date: Thu, 22 Feb 2024 17:03:26 -0500
Subject: [PATCH] Adds support for sending CLOSE frames without a payload as
 required by spec. Updates tests to verify use case. See issue 8407.

---
 .../websocket/ClientWsConnection.java         | 18 +++++++++-------
 .../tests/websocket/WebSocketClientTest.java  | 21 ++++++++++++++++++-
 .../webserver/websocket/WsConnection.java     | 11 +++++-----
 .../io/helidon/websocket/AbstractWsFrame.java |  5 +++--
 4 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/webclient/websocket/src/main/java/io/helidon/webclient/websocket/ClientWsConnection.java b/webclient/websocket/src/main/java/io/helidon/webclient/websocket/ClientWsConnection.java
index 6b7c57827a8..e7bd3a0cad5 100644
--- a/webclient/websocket/src/main/java/io/helidon/webclient/websocket/ClientWsConnection.java
+++ b/webclient/websocket/src/main/java/io/helidon/webclient/websocket/ClientWsConnection.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2023, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -134,12 +134,16 @@ public WsSession pong(BufferData bufferData) {
     public WsSession close(int code, String reason) {
         closeSent = true;
 
-        byte[] reasonBytes = reason.getBytes(StandardCharsets.UTF_8);
-        BufferData bufferData = BufferData.create(2 + reasonBytes.length);
-        bufferData.writeInt16(code);
-        bufferData.write(reasonBytes);
-        send(ClientWsFrame.control(WsOpCode.CLOSE, bufferData));
-
+        // send empty close (no code or reason) if code is negative
+        if (code < 0) {
+            send(ClientWsFrame.control(WsOpCode.CLOSE, BufferData.empty()));
+        } else {
+            byte[] reasonBytes = reason.getBytes(StandardCharsets.UTF_8);
+            BufferData bufferData = BufferData.create(2 + reasonBytes.length);
+            bufferData.writeInt16(code);
+            bufferData.write(reasonBytes);
+            send(ClientWsFrame.control(WsOpCode.CLOSE, bufferData));
+        }
         return this;
     }
 
diff --git a/webserver/tests/websocket/src/test/java/io/helidon/webserver/tests/websocket/WebSocketClientTest.java b/webserver/tests/websocket/src/test/java/io/helidon/webserver/tests/websocket/WebSocketClientTest.java
index 2c387d25f36..c2101267475 100644
--- a/webserver/tests/websocket/src/test/java/io/helidon/webserver/tests/websocket/WebSocketClientTest.java
+++ b/webserver/tests/websocket/src/test/java/io/helidon/webserver/tests/websocket/WebSocketClientTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2023, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -98,4 +98,23 @@ public void onOpen(WsSession session) {
         assertThat(await, is(true));
         assertThat(messages, hasItems(text));
     }
+
+    /**
+     * Tests sending a close frame with no code or reason.
+     */
+    @Test
+    void testEmptyClose() throws InterruptedException {
+        CountDownLatch messageLatch = new CountDownLatch(1);
+
+        wsClient.connect("/echo", new WsListener() {
+            @Override
+            public void onOpen(WsSession session) {
+                session.close(-1, "");      // empty close
+                messageLatch.countDown();
+            }
+        });
+
+        boolean await = messageLatch.await(10, TimeUnit.SECONDS);
+        assertThat(await, is(true));
+    }
 }
diff --git a/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WsConnection.java b/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WsConnection.java
index 55faf658075..98550b90009 100644
--- a/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WsConnection.java
+++ b/webserver/websocket/src/main/java/io/helidon/webserver/websocket/WsConnection.java
@@ -249,12 +249,13 @@ private boolean processFrame(ClientWsFrame frame) {
             listener.onMessage(this, payload, frame.fin());
         }
         case CLOSE -> {
-            int status = payload.readInt16();
-            String reason;
+            int status = WsCloseCodes.NORMAL_CLOSE;
+            String reason = "normal";
             if (payload.available() > 0) {
-                reason = payload.readString(payload.available(), StandardCharsets.UTF_8);
-            } else {
-                reason = "normal";
+                status = payload.readInt16();
+                if (payload.available() > 0) {
+                    reason = payload.readString(payload.available(), StandardCharsets.UTF_8);
+                }
             }
             listener.onClose(this, status, reason);
             if (!closeSent) {
diff --git a/websocket/src/main/java/io/helidon/websocket/AbstractWsFrame.java b/websocket/src/main/java/io/helidon/websocket/AbstractWsFrame.java
index 35671337f65..c223e8efbe2 100644
--- a/websocket/src/main/java/io/helidon/websocket/AbstractWsFrame.java
+++ b/websocket/src/main/java/io/helidon/websocket/AbstractWsFrame.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2023, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -113,7 +113,8 @@ protected static FrameHeader readFrameHeader(DataReader reader, int maxFrameLeng
     }
 
     protected static BufferData readPayload(DataReader reader, FrameHeader header) {
-        return reader.readBuffer(header.length());
+        int length = header.length();
+        return length == 0 ? BufferData.empty() : reader.readBuffer(length);
     }
 
     protected static boolean isPayload(FrameHeader header) {