From 4f18b150b5cf72a76fd7a59302a57e06ce0af0cf Mon Sep 17 00:00:00 2001
From: uzlopak <aras.abbasi@googlemail.com>
Date: Sat, 2 Mar 2024 00:51:08 +0100
Subject: [PATCH 1/6] websocket: use isClosed and isClosing in
 websocket.close()

---
 lib/web/websocket/util.js      | 3 +++
 lib/web/websocket/websocket.js | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/web/websocket/util.js b/lib/web/websocket/util.js
index 8abe73c83e3..ba3408fc532 100644
--- a/lib/web/websocket/util.js
+++ b/lib/web/websocket/util.js
@@ -8,6 +8,7 @@ const { MessageEvent, ErrorEvent } = require('./events')
 
 /**
  * @param {import('./websocket').WebSocket} ws
+ * @returns {boolean}
  */
 function isEstablished (ws) {
   // If the server's response is validated as provided for above, it is
@@ -18,6 +19,7 @@ function isEstablished (ws) {
 
 /**
  * @param {import('./websocket').WebSocket} ws
+ * @returns {boolean}
  */
 function isClosing (ws) {
   // Upon either sending or receiving a Close control frame, it is said
@@ -28,6 +30,7 @@ function isClosing (ws) {
 
 /**
  * @param {import('./websocket').WebSocket} ws
+ * @returns {boolean}
  */
 function isClosed (ws) {
   return ws[kReadyState] === states.CLOSED
diff --git a/lib/web/websocket/websocket.js b/lib/web/websocket/websocket.js
index 0072da48193..9ddef5b0ff7 100644
--- a/lib/web/websocket/websocket.js
+++ b/lib/web/websocket/websocket.js
@@ -13,7 +13,7 @@ const {
   kSentClose,
   kByteParser
 } = require('./symbols')
-const { isEstablished, isClosing, isValidSubprotocol, failWebsocketConnection, fireEvent } = require('./util')
+const { isEstablished, isClosing, isValidSubprotocol, failWebsocketConnection, fireEvent, isClosed } = require('./util')
 const { establishWebSocketConnection } = require('./connection')
 const { WebsocketFrameSend } = require('./frame')
 const { ByteParser } = require('./receiver')
@@ -184,7 +184,7 @@ class WebSocket extends EventTarget {
     }
 
     // 3. Run the first matching steps from the following list:
-    if (this[kReadyState] === WebSocket.CLOSING || this[kReadyState] === WebSocket.CLOSED) {
+    if (isClosing(this) || isClosed(this)) {
       // If this's ready state is CLOSING (2) or CLOSED (3)
       // Do nothing.
     } else if (!isEstablished(this)) {

From 87e7feaa8eab3071b9e0ab8746a778de25b06c86 Mon Sep 17 00:00:00 2001
From: uzlopak <aras.abbasi@googlemail.com>
Date: Sat, 2 Mar 2024 01:48:00 +0100
Subject: [PATCH 2/6] add isConnecting

---
 lib/web/websocket/util.js      | 11 +++++++++++
 lib/web/websocket/websocket.js | 12 ++++++++++--
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/lib/web/websocket/util.js b/lib/web/websocket/util.js
index ba3408fc532..4f0e4dcf5dd 100644
--- a/lib/web/websocket/util.js
+++ b/lib/web/websocket/util.js
@@ -6,6 +6,16 @@ const { MessageEvent, ErrorEvent } = require('./events')
 
 /* globals Blob */
 
+/**
+ * @param {import('./websocket').WebSocket} ws
+ * @returns {boolean}
+ */
+function isConnecting (ws) {
+  // If the WebSocket connection is not yet established, and the connection
+  // is not yet closed, then the WebSocket connection is in the CONNECTING state.
+  return ws[kReadyState] === states.CONNECTING
+}
+
 /**
  * @param {import('./websocket').WebSocket} ws
  * @returns {boolean}
@@ -193,6 +203,7 @@ function failWebsocketConnection (ws, reason) {
 }
 
 module.exports = {
+  isConnecting,
   isEstablished,
   isClosing,
   isClosed,
diff --git a/lib/web/websocket/websocket.js b/lib/web/websocket/websocket.js
index 9ddef5b0ff7..c0e9ff1a3cb 100644
--- a/lib/web/websocket/websocket.js
+++ b/lib/web/websocket/websocket.js
@@ -13,7 +13,15 @@ const {
   kSentClose,
   kByteParser
 } = require('./symbols')
-const { isEstablished, isClosing, isValidSubprotocol, failWebsocketConnection, fireEvent, isClosed } = require('./util')
+const {
+  isConnecting,
+  isEstablished,
+  isClosed,
+  isClosing,
+  isValidSubprotocol,
+  failWebsocketConnection,
+  fireEvent
+} = require('./util')
 const { establishWebSocketConnection } = require('./connection')
 const { WebsocketFrameSend } = require('./frame')
 const { ByteParser } = require('./receiver')
@@ -187,7 +195,7 @@ class WebSocket extends EventTarget {
     if (isClosing(this) || isClosed(this)) {
       // If this's ready state is CLOSING (2) or CLOSED (3)
       // Do nothing.
-    } else if (!isEstablished(this)) {
+    } else if (isConnecting(this)) {
       // If the WebSocket connection is not yet established
       // Fail the WebSocket connection and set this's ready state
       // to CLOSING (2).

From cc52e2378a2cde0a20452f83a92666427e854e56 Mon Sep 17 00:00:00 2001
From: uzlopak <aras.abbasi@googlemail.com>
Date: Sat, 2 Mar 2024 03:27:48 +0100
Subject: [PATCH 3/6] sentCloseFrameState

---
 lib/web/websocket/connection.js |  6 +++---
 lib/web/websocket/constants.js  |  7 +++++++
 lib/web/websocket/receiver.js   |  6 +++---
 lib/web/websocket/websocket.js  | 10 +++++++---
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/web/websocket/connection.js b/lib/web/websocket/connection.js
index 45f68e1de93..d7e3efdc830 100644
--- a/lib/web/websocket/connection.js
+++ b/lib/web/websocket/connection.js
@@ -1,6 +1,6 @@
 'use strict'
 
-const { uid, states } = require('./constants')
+const { uid, states, sentCloseFrameState } = require('./constants')
 const {
   kReadyState,
   kSentClose,
@@ -229,7 +229,7 @@ function onSocketClose () {
   // If the TCP connection was closed after the
   // WebSocket closing handshake was completed, the WebSocket connection
   // is said to have been closed _cleanly_.
-  const wasClean = ws[kSentClose] && ws[kReceivedClose]
+  const wasClean = ws[kSentClose] === sentCloseFrameState.SENT && ws[kReceivedClose]
 
   let code = 1005
   let reason = ''
@@ -239,7 +239,7 @@ function onSocketClose () {
   if (result) {
     code = result.code ?? 1005
     reason = result.reason
-  } else if (!ws[kSentClose]) {
+  } else if (ws[kSentClose] !== sentCloseFrameState.SENT) {
     // If _The WebSocket
     // Connection is Closed_ and no Close control frame was received by the
     // endpoint (such as could occur if the underlying transport connection
diff --git a/lib/web/websocket/constants.js b/lib/web/websocket/constants.js
index 406b8e3e2f0..d5de91460f5 100644
--- a/lib/web/websocket/constants.js
+++ b/lib/web/websocket/constants.js
@@ -20,6 +20,12 @@ const states = {
   CLOSED: 3
 }
 
+const sentCloseFrameState = {
+  NOT_SENT: 0,
+  PROCESSING: 1,
+  SENT: 2
+}
+
 const opcodes = {
   CONTINUATION: 0x0,
   TEXT: 0x1,
@@ -42,6 +48,7 @@ const emptyBuffer = Buffer.allocUnsafe(0)
 
 module.exports = {
   uid,
+  sentCloseFrameState,
   staticPropertyDescriptors,
   states,
   opcodes,
diff --git a/lib/web/websocket/receiver.js b/lib/web/websocket/receiver.js
index 18dc474f16e..e590b468b6d 100644
--- a/lib/web/websocket/receiver.js
+++ b/lib/web/websocket/receiver.js
@@ -1,7 +1,7 @@
 'use strict'
 
 const { Writable } = require('node:stream')
-const { parserStates, opcodes, states, emptyBuffer } = require('./constants')
+const { parserStates, opcodes, states, emptyBuffer, sentCloseFrameState } = require('./constants')
 const { kReadyState, kSentClose, kResponse, kReceivedClose } = require('./symbols')
 const { channels } = require('../../core/diagnostics')
 const { isValidStatusCode, failWebsocketConnection, websocketMessageReceived } = require('./util')
@@ -104,7 +104,7 @@ class ByteParser extends Writable {
 
           this.#info.closeInfo = this.parseCloseBody(false, body)
 
-          if (!this.ws[kSentClose]) {
+          if (this.ws[kSentClose] !== sentCloseFrameState.SENT) {
             // If an endpoint receives a Close frame and did not previously send a
             // Close frame, the endpoint MUST send a Close frame in response.  (When
             // sending a Close frame in response, the endpoint typically echos the
@@ -120,7 +120,7 @@ class ByteParser extends Writable {
               closeFrame.createFrame(opcodes.CLOSE),
               (err) => {
                 if (!err) {
-                  this.ws[kSentClose] = true
+                  this.ws[kSentClose] = sentCloseFrameState.SENT
                 }
               }
             )
diff --git a/lib/web/websocket/websocket.js b/lib/web/websocket/websocket.js
index c0e9ff1a3cb..0da21b36fea 100644
--- a/lib/web/websocket/websocket.js
+++ b/lib/web/websocket/websocket.js
@@ -3,7 +3,7 @@
 const { webidl } = require('../fetch/webidl')
 const { URLSerializer } = require('../fetch/dataURL')
 const { getGlobalOrigin } = require('../fetch/global')
-const { staticPropertyDescriptors, states, opcodes, emptyBuffer } = require('./constants')
+const { staticPropertyDescriptors, states, sentCloseFrameState, opcodes, emptyBuffer } = require('./constants')
 const {
   kWebSocketURL,
   kReadyState,
@@ -140,6 +140,8 @@ class WebSocket extends EventTarget {
     // be CONNECTING (0).
     this[kReadyState] = WebSocket.CONNECTING
 
+    this[kSentClose] = sentCloseFrameState.NOT_SENT
+
     // The extensions attribute must initially return the empty string.
 
     // The protocol attribute must initially return the empty string.
@@ -201,7 +203,7 @@ class WebSocket extends EventTarget {
       // to CLOSING (2).
       failWebsocketConnection(this, 'Connection was closed before it was established.')
       this[kReadyState] = WebSocket.CLOSING
-    } else if (!isClosing(this)) {
+    } else if (this[kSentClose] === sentCloseFrameState.NOT_SENT) {
       // If the WebSocket closing handshake has not yet been started
       // Start the WebSocket closing handshake and set this's ready
       // state to CLOSING (2).
@@ -212,6 +214,8 @@ class WebSocket extends EventTarget {
       // - If reason is also present, then reasonBytes must be
       //   provided in the Close message after the status code.
 
+      this[kSentClose] = sentCloseFrameState.PROCESSING
+
       const frame = new WebsocketFrameSend()
 
       // If neither code nor reason is present, the WebSocket Close
@@ -238,7 +242,7 @@ class WebSocket extends EventTarget {
 
       socket.write(frame.createFrame(opcodes.CLOSE), (err) => {
         if (!err) {
-          this[kSentClose] = true
+          this[kSentClose] = sentCloseFrameState.SENT
         }
       })
 

From a793b3c60035920873c04a7d862cae6bc8e7ce40 Mon Sep 17 00:00:00 2001
From: uzlopak <aras.abbasi@googlemail.com>
Date: Sat, 2 Mar 2024 04:15:42 +0100
Subject: [PATCH 4/6] use again !isEstablished

---
 lib/web/websocket/websocket.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/web/websocket/websocket.js b/lib/web/websocket/websocket.js
index 0da21b36fea..dc56b419f2b 100644
--- a/lib/web/websocket/websocket.js
+++ b/lib/web/websocket/websocket.js
@@ -197,7 +197,7 @@ class WebSocket extends EventTarget {
     if (isClosing(this) || isClosed(this)) {
       // If this's ready state is CLOSING (2) or CLOSED (3)
       // Do nothing.
-    } else if (isConnecting(this)) {
+    } else if (!isEstablished(this)) {
       // If the WebSocket connection is not yet established
       // Fail the WebSocket connection and set this's ready state
       // to CLOSING (2).
@@ -270,7 +270,7 @@ class WebSocket extends EventTarget {
 
     // 1. If this's ready state is CONNECTING, then throw an
     //    "InvalidStateError" DOMException.
-    if (this[kReadyState] === WebSocket.CONNECTING) {
+    if (isConnecting(this)) {
       throw new DOMException('Sent before connected.', 'InvalidStateError')
     }
 

From 827cffc2859b0525968aa9b808075cd234ff637c Mon Sep 17 00:00:00 2001
From: uzlopak <aras.abbasi@googlemail.com>
Date: Sat, 2 Mar 2024 05:47:47 +0100
Subject: [PATCH 5/6] add tests to increase coverage

---
 test/websocket/close.js | 53 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/test/websocket/close.js b/test/websocket/close.js
index cb8825645ba..15d2e6ad625 100644
--- a/test/websocket/close.js
+++ b/test/websocket/close.js
@@ -1,9 +1,11 @@
 'use strict'
 
-const { describe, test } = require('node:test')
+const { tspl } = require('@matteo.collina/tspl')
+const { describe, test, after } = require('node:test')
 const assert = require('node:assert')
 const { WebSocketServer } = require('ws')
 const { WebSocket } = require('../..')
+const { kReadyState } = require('../../lib/web/websocket/symbols')
 
 describe('Close', () => {
   test('Close with code', () => {
@@ -128,4 +130,53 @@ describe('Close', () => {
       ws.addEventListener('open', () => ws.close(3000))
     })
   })
+
+  test('calling close twice will only trigger the close event once', async (t) => {
+    t = tspl(t, { plan: 1 })
+
+    const server = new WebSocketServer({ port: 0 })
+
+    after(() => server.close())
+
+    server.on('connection', (ws) => {
+      ws.on('close', (code) => {
+        t.strictEqual(code, 1000)
+      })
+    })
+
+    const ws = new WebSocket(`ws://localhost:${server.address().port}`)
+    ws.addEventListener('open', () => {
+      ws.close(1000)
+      ws.close(1000)
+    })
+
+    await t.completed
+  })
+
+  test('calling close when connection is open but close handshake has started does not result in sending the handshake again', async (t) => {
+    t = tspl(t, { plan: 1 })
+
+    const server = new WebSocketServer({ port: 0 })
+
+    after(() => server.close())
+
+    server.on('connection', (ws) => {
+      ws.on('close', (code) => {
+        t.strictEqual(code, 1000)
+      })
+    })
+
+    const ws = new WebSocket(`ws://localhost:${server.address().port}`)
+    ws.addEventListener('open', () => {
+      // .close() will start the close handshake and set the readyState to
+      // CLOSING
+      ws.close(1000)
+      // set the readyState to OPEN to simulate the close handshake being
+      // started but not yet completed
+      ws[kReadyState] = ws.OPEN
+      ws.close(1000)
+    })
+
+    await t.completed
+  })
 })

From d096575735f3d2fc1ce96c4e939c9220f43ca5b5 Mon Sep 17 00:00:00 2001
From: uzlopak <aras.abbasi@googlemail.com>
Date: Sat, 2 Mar 2024 07:27:48 +0100
Subject: [PATCH 6/6] remove test

---
 test/websocket/close.js | 28 ----------------------------
 1 file changed, 28 deletions(-)

diff --git a/test/websocket/close.js b/test/websocket/close.js
index 15d2e6ad625..c4305d60948 100644
--- a/test/websocket/close.js
+++ b/test/websocket/close.js
@@ -5,7 +5,6 @@ const { describe, test, after } = require('node:test')
 const assert = require('node:assert')
 const { WebSocketServer } = require('ws')
 const { WebSocket } = require('../..')
-const { kReadyState } = require('../../lib/web/websocket/symbols')
 
 describe('Close', () => {
   test('Close with code', () => {
@@ -152,31 +151,4 @@ describe('Close', () => {
 
     await t.completed
   })
-
-  test('calling close when connection is open but close handshake has started does not result in sending the handshake again', async (t) => {
-    t = tspl(t, { plan: 1 })
-
-    const server = new WebSocketServer({ port: 0 })
-
-    after(() => server.close())
-
-    server.on('connection', (ws) => {
-      ws.on('close', (code) => {
-        t.strictEqual(code, 1000)
-      })
-    })
-
-    const ws = new WebSocket(`ws://localhost:${server.address().port}`)
-    ws.addEventListener('open', () => {
-      // .close() will start the close handshake and set the readyState to
-      // CLOSING
-      ws.close(1000)
-      // set the readyState to OPEN to simulate the close handshake being
-      // started but not yet completed
-      ws[kReadyState] = ws.OPEN
-      ws.close(1000)
-    })
-
-    await t.completed
-  })
 })
