Repository: nodejs/undici

Problem statement:
running autobahn test suite
We cannot move WebSocket out from experimental until we run the autobahn test suite.

https://github.com/crossbario/autobahn-testsuite


Hints:

Patch:
diff --git a/lib/web/websocket/connection.js b/lib/web/websocket/connection.js
--- a/lib/web/websocket/connection.js
+++ b/lib/web/websocket/connection.js
@@ -261,13 +261,9 @@ function closeWebSocketConnection (ws, code, reason, reasonByteLength) {
     /** @type {import('stream').Duplex} */
     const socket = ws[kResponse].socket
 
-    socket.write(frame.createFrame(opcodes.CLOSE), (err) => {
-      if (!err) {
-        ws[kSentClose] = sentCloseFrameState.SENT
-      }
-    })
+    socket.write(frame.createFrame(opcodes.CLOSE))
 
-    ws[kSentClose] = sentCloseFrameState.PROCESSING
+    ws[kSentClose] = sentCloseFrameState.SENT
 
     // Upon either sending or receiving a Close control frame, it is said
     // that _The WebSocket Closing Handshake is Started_ and that the
diff --git a/lib/web/websocket/receiver.js b/lib/web/websocket/receiver.js
--- a/lib/web/websocket/receiver.js
+++ b/lib/web/websocket/receiver.js
@@ -5,7 +5,16 @@ const assert = require('node:assert')
 const { parserStates, opcodes, states, emptyBuffer, sentCloseFrameState } = require('./constants')
 const { kReadyState, kSentClose, kResponse, kReceivedClose } = require('./symbols')
 const { channels } = require('../../core/diagnostics')
-const { isValidStatusCode, failWebsocketConnection, websocketMessageReceived, utf8Decode, isControlFrame, isContinuationFrame } = require('./util')
+const {
+  isValidStatusCode,
+  isValidOpcode,
+  failWebsocketConnection,
+  websocketMessageReceived,
+  utf8Decode,
+  isControlFrame,
+  isContinuationFrame,
+  isTextBinaryFrame
+} = require('./util')
 const { WebsocketFrameSend } = require('./frame')
 const { CloseEvent } = require('./events')
 
@@ -58,19 +67,45 @@ class ByteParser extends Writable {
         const opcode = buffer[0] & 0x0F
         const masked = (buffer[1] & 0x80) === 0x80
 
+        if (!isValidOpcode(opcode)) {
+          failWebsocketConnection(this.ws, 'Invalid opcode received')
+          return callback()
+        }
+
         if (masked) {
           failWebsocketConnection(this.ws, 'Frame cannot be masked')
           return callback()
         }
 
+        const rsv1 = (buffer[0] & 0x40) !== 0
+        const rsv2 = (buffer[0] & 0x20) !== 0
+        const rsv3 = (buffer[0] & 0x10) !== 0
+
+        // MUST be 0 unless an extension is negotiated that defines meanings
+        // for non-zero values.  If a nonzero value is received and none of
+        // the negotiated extensions defines the meaning of such a nonzero
+        // value, the receiving endpoint MUST _Fail the WebSocket
+        // Connection_.
+        if (rsv1 || rsv2 || rsv3) {
+          failWebsocketConnection(this.ws, 'RSV1, RSV2, RSV3 must be clear')
+          return
+        }
+
         const fragmented = !fin && opcode !== opcodes.CONTINUATION
 
-        if (fragmented && opcode !== opcodes.BINARY && opcode !== opcodes.TEXT) {
+        if (fragmented && !isTextBinaryFrame(opcode)) {
           // Only text and binary frames can be fragmented
           failWebsocketConnection(this.ws, 'Invalid frame type was fragmented.')
           return
         }
 
+        // If we are already parsing a text/binary frame and do not receive either
+        // a continuation frame or close frame, fail the connection.
+        if (isTextBinaryFrame(opcode) && this.#info.opcode !== undefined) {
+          failWebsocketConnection(this.ws, 'Expected continuation frame')
+          return
+        }
+
         const payloadLength = buffer[1] & 0x7F
 
         if (isControlFrame(opcode)) {
@@ -269,7 +304,7 @@ class ByteParser extends Writable {
 
     if (info.payloadLength > 125) {
       // Control frames can have a payload length of 125 bytes MAX
-      callback(new Error('Payload length for control frame exceeded 125 bytes.'))
+      failWebsocketConnection(this.ws, 'Payload length for control frame exceeded 125 bytes.')
       return false
     } else if (this.#byteOffset < info.payloadLength) {
       callback()
@@ -375,7 +410,7 @@ class ByteParser extends Writable {
   parseContinuationFrame (callback, info) {
     // If we received a continuation frame before we started parsing another frame.
     if (this.#info.opcode === undefined) {
-      callback(new Error('Received unexpected continuation frame.'))
+      failWebsocketConnection(this.ws, 'Received unexpected continuation frame.')
       return false
     } else if (this.#byteOffset < info.payloadLength) {
       callback()
diff --git a/lib/web/websocket/util.js b/lib/web/websocket/util.js
--- a/lib/web/websocket/util.js
+++ b/lib/web/websocket/util.js
@@ -226,6 +226,14 @@ function isContinuationFrame (opcode) {
   return opcode === opcodes.CONTINUATION
 }
 
+function isTextBinaryFrame (opcode) {
+  return opcode === opcodes.TEXT || opcode === opcodes.BINARY
+}
+
+function isValidOpcode (opcode) {
+  return isTextBinaryFrame(opcode) || isContinuationFrame(opcode) || isControlFrame(opcode)
+}
+
 // https://nodejs.org/api/intl.html#detecting-internationalization-support
 const hasIntl = typeof process.versions.icu === 'string'
 const fatalDecoder = hasIntl ? new TextDecoder('utf-8', { fatal: true }) : undefined
@@ -255,5 +263,7 @@ module.exports = {
   websocketMessageReceived,
   utf8Decode,
   isControlFrame,
-  isContinuationFrame
+  isContinuationFrame,
+  isTextBinaryFrame,
+  isValidOpcode
 }
diff --git a/lib/web/websocket/websocket.js b/lib/web/websocket/websocket.js
--- a/lib/web/websocket/websocket.js
+++ b/lib/web/websocket/websocket.js
@@ -26,7 +26,7 @@ const { ByteParser } = require('./receiver')
 const { kEnumerableProperty, isBlobLike } = require('../../core/util')
 const { getGlobalDispatcher } = require('../../global')
 const { types } = require('node:util')
-const { ErrorEvent } = require('./events')
+const { ErrorEvent, CloseEvent } = require('./events')
 
 let experimentalWarned = false
 
@@ -594,9 +594,19 @@ function onParserDrain () {
 }
 
 function onParserError (err) {
-  fireEvent('error', this, () => new ErrorEvent('error', { error: err, message: err.reason }))
+  let message
+  let code
+
+  if (err instanceof CloseEvent) {
+    message = err.reason
+    code = err.code
+  } else {
+    message = err.message
+  }
+
+  fireEvent('error', this, () => new ErrorEvent('error', { error: err, message }))
 
-  closeWebSocketConnection(this, err.code)
+  closeWebSocketConnection(this, code)
 }
 
 module.exports = {


Test patch:
diff --git a/test/autobahn/.gitignore b/test/autobahn/.gitignore
new file mode 100644
--- /dev/null
+++ b/test/autobahn/.gitignore
@@ -0,0 +1 @@
+reports/clients
diff --git a/test/autobahn/client.js b/test/autobahn/client.js
new file mode 100644
--- /dev/null
+++ b/test/autobahn/client.js
@@ -0,0 +1,47 @@
+'use strict'
+
+const { WebSocket } = require('../..')
+
+let currentTest = 1
+let testCount
+
+const autobahnFuzzingserverUrl = process.env.FUZZING_SERVER_URL || 'ws://localhost:9001'
+
+function nextTest () {
+  let ws
+
+  if (currentTest > testCount) {
+    ws = new WebSocket(`${autobahnFuzzingserverUrl}/updateReports?agent=undici`)
+    return
+  }
+
+  console.log(`Running test case ${currentTest}/${testCount}`)
+
+  ws = new WebSocket(
+    `${autobahnFuzzingserverUrl}/runCase?case=${currentTest}&agent=undici`
+  )
+  ws.addEventListener('message', (data) => {
+    ws.send(data.data)
+  })
+  ws.addEventListener('close', () => {
+    currentTest++
+    process.nextTick(nextTest)
+  })
+  ws.addEventListener('error', (e) => {
+    console.error(e.error)
+  })
+}
+
+const ws = new WebSocket(`${autobahnFuzzingserverUrl}/getCaseCount`)
+ws.addEventListener('message', (data) => {
+  testCount = parseInt(data.data)
+})
+ws.addEventListener('close', () => {
+  if (testCount > 0) {
+    nextTest()
+  }
+})
+ws.addEventListener('error', (e) => {
+  console.error(e.error)
+  process.exit(1)
+})
diff --git a/test/autobahn/config/fuzzingserver.json b/test/autobahn/config/fuzzingserver.json
new file mode 100644
--- /dev/null
+++ b/test/autobahn/config/fuzzingserver.json
@@ -0,0 +1,7 @@
+{
+  "url": "ws://127.0.0.1:9001",
+  "outdir": "./reports/clients",
+  "cases": ["*"],
+  "exclude-cases": [],
+  "exclude-agent-cases": {}
+}
diff --git a/test/autobahn/report.js b/test/autobahn/report.js
new file mode 100644
--- /dev/null
+++ b/test/autobahn/report.js
@@ -0,0 +1,142 @@
+'use strict'
+
+const result = require('./reports/clients/index.json').undici
+
+const failOnError = process.env.FAIL_ON_ERROR === 'true'
+const reporter = process.env.REPORTER || 'table'
+let runFailed = false
+
+let okTests = 0
+let failedTests = 0
+let nonStrictTests = 0
+let wrongCodeTests = 0
+let uncleanTests = 0
+let failedByClientTests = 0
+let informationalTests = 0
+let unimplementedTests = 0
+
+let totalTests = 0
+
+function testCaseIdToWeight (testCaseId) {
+  const [major, minor, sub] = testCaseId.split('.')
+  return sub
+    ? parseInt(major, 10) * 10000 + parseInt(minor, 10) * 100 + parseInt(sub, 10)
+    : parseInt(major, 10) * 10000 + parseInt(minor, 10) * 100
+}
+
+function isFailedTestCase (testCase) {
+  return (
+    testCase.behavior === 'FAILED' ||
+    testCase.behavior === 'WRONG CODE' ||
+    testCase.behavior === 'UNCLEAN' ||
+    testCase.behavior === 'FAILED BY CLIENT' ||
+    testCase.behaviorClose === 'FAILED' ||
+    testCase.behaviorClose === 'WRONG CODE' ||
+    testCase.behaviorClose === 'UNCLEAN' ||
+    testCase.behaviorClose === 'FAILED BY CLIENT'
+  )
+}
+
+const keys = Object.keys(result).sort((a, b) => {
+  a = testCaseIdToWeight(a)
+  b = testCaseIdToWeight(b)
+  return a - b
+})
+
+const reorderedResult = {}
+for (const key of keys) {
+  reorderedResult[key] = result[key]
+  delete reorderedResult[key].reportfile
+
+  totalTests++
+
+  if (
+    failOnError &&
+    !runFailed &&
+    isFailedTestCase(result[key])
+  ) {
+    runFailed = true
+  }
+
+  switch (result[key].behavior) {
+    case 'OK':
+      okTests++
+      break
+    case 'FAILED':
+      failedTests++
+      break
+    case 'NON-STRICT':
+      nonStrictTests++
+      break
+    case 'WRONG CODE':
+      wrongCodeTests++
+      break
+    case 'UNCLEAN':
+      uncleanTests++
+      break
+    case 'FAILED BY CLIENT':
+      failedByClientTests++
+      break
+    case 'INFORMATIONAL':
+      informationalTests++
+      break
+    case 'UNIMPLEMENTED':
+      unimplementedTests++
+      break
+  }
+}
+
+if (
+  reporter === 'table'
+) {
+  console.log('Autobahn Test Report\n\nSummary:')
+
+  console.table({
+    OK: okTests,
+    Failed: failedTests,
+    'Non-Strict': nonStrictTests,
+    'Wrong Code': wrongCodeTests,
+    Unclean: uncleanTests,
+    'Failed By Client': failedByClientTests,
+    Informational: informationalTests,
+    Unimplemented: unimplementedTests,
+    'Total Tests': totalTests
+  })
+
+  console.log('Details:')
+
+  console.table(reorderedResult)
+}
+
+if (reporter === 'markdown') {
+  console.log(`## Autobahn Test Report
+
+### Summary
+
+| Type | Count |
+|---|---|
+| OK | ${okTests} |
+| Failed | ${failedTests} |
+| Non-Strict | ${nonStrictTests} |
+| Wrong Code | ${wrongCodeTests} |
+| Unclean | ${uncleanTests} |
+| Failed By Client | ${failedByClientTests} |
+| Informational | ${informationalTests} |
+| Unimplemented | ${unimplementedTests} |
+| Total Tests | ${totalTests} |
+
+<details>
+<summary>Details</summary>
+
+| Test Case | Behavior | Close Behavior | Duration | Remote Close Code |
+|---|---|---|---|---|
+${keys.map(key => {
+  const testCase = reorderedResult[key]
+  return `| ${key} | ${testCase.behavior} | ${testCase.behaviorClose} | ${testCase.duration} | ${testCase.remoteCloseCode} |`
+}).join('\n')}
+
+</details>
+`)
+}
+
+process.exit(runFailed ? 1 : 0)
diff --git a/test/autobahn/run.sh b/test/autobahn/run.sh
new file mode 100755
--- /dev/null
+++ b/test/autobahn/run.sh
@@ -0,0 +1,6 @@
+docker run -it --rm \
+    -v "${PWD}/config:/config" \
+    -v "${PWD}/reports:/reports" \
+    -p 9001:9001 \
+    --name fuzzingserver \
+    crossbario/autobahn-testsuite