diff --git a/.github/workflows/autobahn.yml b/.github/workflows/autobahn.yml new file mode 100644 index 00000000000..27e0c67f25f --- /dev/null +++ b/.github/workflows/autobahn.yml @@ -0,0 +1,71 @@ +name: Autobahn +on: + workflow_dispatch: + + pull_request: + paths: + - '.github/workflows/autobahn.yml' + - 'lib/web/websocket/**' + - 'test/autobahn/**' + +permissions: + contents: read + pull-requests: write + +jobs: + autobahn: + name: Autobahn Test Suite + runs-on: ubuntu-latest + container: node:22 + services: + fuzzingserver: + image: crossbario/autobahn-testsuite:latest + ports: + - '9001:9001' + options: --name fuzzingserver + volumes: + - ${{ github.workspace }}/test/autobahn/config:/config + - ${{ github.workspace }}/test/autobahn/reports:/reports + steps: + - name: Checkout Code + uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2 + with: + persist-credentials: false + clean: false + + - name: Restart Autobahn Server + # Restart service after volumes have been checked out + uses: docker://docker + with: + args: docker restart --time 0 --signal=SIGKILL fuzzingserver + + - name: Setup Node + uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2 + with: + node-version: 22 + + - name: Run Autobahn Test Suite + run: npm run test:websocket:autobahn + env: + FUZZING_SERVER_URL: ws://fuzzingserver:9001 + + - name: Report into CI + id: report-ci + run: npm run test:websocket:autobahn:report + + - name: Generate Report for PR Comment + if: github.event_name == 'pull_request' + id: report-markdown + run: | + echo "comment<> $GITHUB_OUTPUT + node test/autobahn/report.js >> $GITHUB_OUTPUT + echo "nEOFn" >> $GITHUB_OUTPUT + env: + REPORTER: markdown + + - name: Comment PR + if: github.event_name == 'pull_request' + uses: thollander/actions-comment-pull-request@v2 + with: + message: ${{ steps.report-markdown.outputs.comment }} + comment_tag: autobahn diff --git a/lib/web/websocket/connection.js b/lib/web/websocket/connection.js index 5058fc58095..85b471e6e0b 100644 --- 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 index 087b962a3ce..66f7a3e8f64 100644 --- 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 index 35d67d17eb5..ea5b29d3549 100644 --- 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 index 32ec97e68db..7b62dde43c6 100644 --- 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 = { diff --git a/package.json b/package.json index bf594948491..95cf7b38b94 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,8 @@ "test:typescript": "tsd && tsc --skipLibCheck test/imports/undici-import.ts", "test:webidl": "borp -p \"test/webidl/*.js\"", "test:websocket": "borp -p \"test/websocket/*.js\"", + "test:websocket:autobahn": "node test/autobahn/client.js", + "test:websocket:autobahn:report": "node test/autobahn/report.js", "test:wpt": "node test/wpt/start-fetch.mjs && node test/wpt/start-FileAPI.mjs && node test/wpt/start-mimesniff.mjs && node test/wpt/start-xhr.mjs && node test/wpt/start-websockets.mjs && node test/wpt/start-cacheStorage.mjs && node test/wpt/start-eventsource.mjs", "test:wpt:withoutintl": "node test/wpt/start-fetch.mjs && node test/wpt/start-mimesniff.mjs && node test/wpt/start-xhr.mjs && node test/wpt/start-cacheStorage.mjs && node test/wpt/start-eventsource.mjs", "coverage": "npm run coverage:clean && cross-env NODE_V8_COVERAGE=./coverage/tmp npm run test:javascript && npm run coverage:report", diff --git a/test/autobahn/.gitignore b/test/autobahn/.gitignore new file mode 100644 index 00000000000..f073a2d3495 --- /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 index 00000000000..41bf1d61063 --- /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 index 00000000000..e21b420ffa5 --- /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 index 00000000000..105eb4f1496 --- /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 + +| 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')} + +
+`) +} + +process.exit(runFailed ? 1 : 0) diff --git a/test/autobahn/run.sh b/test/autobahn/run.sh new file mode 100755 index 00000000000..907ff71778b --- /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