undici icon indicating copy to clipboard operation
undici copied to clipboard

Failing autobahn tests

Open KhafraDev opened this issue 9 months ago • 6 comments

  • [x] Case 2.6
  • [x] Case 2.11
  • [x] Case 5.5
  • [x] Case 5.8
  • [x] Case 9.3.1
  • [x] Case 9.4.1
  • [ ] Case 9.3.2 -> Case 9.3.8
  • [ ] Case 9.4.2 -> Case 9.4.8
  • [ ] Case 10.1.1

Total tests failing: 15

All of these tests relate to fragmentation but I can't reproduce locally using ws. Seems to be something about receiving chunked data (could it be sending?).

KhafraDev avatar May 13 '24 05:05 KhafraDev

Interestingly, at a certain threshold the fragmentation tests start to pass, such as 9.3.9 & 9.4.9. Sending out smaller chunks = tests fail for some reason.

KhafraDev avatar May 13 '24 05:05 KhafraDev

What I am currently looking is 2.6. it seems that a frame containing 125 times 0xfe is not properly processed. I assume that we should actually fast forward the 125 times 0xfe but we dont, we do a consume(2) and not consume(payloadLength).

Undici:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/undici$ npm run test:websocket:autobahn
npm warn cli npm v10.7.0 does not support Node.js v23.0.0-nightly20240507be8d64ec14. This version of npm supports the following node versions: `^18.17.0 || >=20.5.0`. You can find the latest version at https://nodejs.org/.

> [email protected] test:websocket:autobahn
> node test/autobahn/client.js

(node:95584) [UNDICI-WS] Warning: WebSockets are experimental, expect them to change at any time.
(Use `node --trace-warnings ...` to show where the warning was created)
<Buffer 81 01>
<Buffer 88 00>
Running test case 1/1
<Buffer 89 7d>
<Buffer fe fe>
Error: Invalid opcode received
    at failWebsocketConnection (/home/aras/workspace/undici/lib/web/websocket/util.js:207:14)
    at ByteParser.run (/home/aras/workspace/undici/lib/web/websocket/receiver.js:73:11)
    at ByteParser._write (/home/aras/workspace/undici/lib/web/websocket/receiver.js:49:10)
    at writeOrBuffer (node:internal/streams/writable:564:12)
    at _write (node:internal/streams/writable:493:10)
    at Writable.write (node:internal/streams/writable:502:10)
    at Socket.onSocketData (/home/aras/workspace/undici/lib/web/websocket/connection.js:283:29)
    at Socket.emit (node:events:520:28)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
<Buffer 88 00>

ws:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/ws$ node test/autobahn
<Buffer 81 01>
<Buffer 88 00>
Running test case 1/1
<Buffer 89 7d>
<Buffer 88 02>
<Buffer 88 00>

Uzlopak avatar May 13 '24 06:05 Uzlopak

The state at the beginning of the loop:

undici:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/undici$ npm run test:websocket:autobahn
npm warn cli npm v10.7.0 does not support Node.js v23.0.0-nightly20240507be8d64ec14. This version of npm supports the following node versions: `^18.17.0 || >=20.5.0`. You can find the latest version at https://nodejs.org/.

> [email protected] test:websocket:autobahn
> node test/autobahn/client.js

(node:96897) [UNDICI-WS] Warning: WebSockets are experimental, expect them to change at any time.
(Use `node --trace-warnings ...` to show where the warning was created)
{ state: 0 }
<Buffer 81 01>
{ state: 4 }
{ state: 0 }
<Buffer 88 00>
Running test case 1/1
{ state: 0 }
<Buffer 89 7d>
{ state: 0 }
<Buffer fe fe>
Error: Invalid opcode received
    at failWebsocketConnection (/home/aras/workspace/undici/lib/web/websocket/util.js:207:14)
    at ByteParser.run (/home/aras/workspace/undici/lib/web/websocket/receiver.js:74:11)
    at ByteParser._write (/home/aras/workspace/undici/lib/web/websocket/receiver.js:49:10)
    at writeOrBuffer (node:internal/streams/writable:564:12)
    at _write (node:internal/streams/writable:493:10)
    at Writable.write (node:internal/streams/writable:502:10)
    at Socket.onSocketData (/home/aras/workspace/undici/lib/web/websocket/connection.js:283:29)
    at Socket.emit (node:events:520:28)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
{ state: 0 }
<Buffer 88 00>

ws:

aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/ws$ node test/autobahn
{ state: 0 }
<Buffer 81 01>
{ state: 4 }
{ state: 0 }
<Buffer 88 00>
{ state: 4 }
Running test case 1/1
{ state: 0 }
<Buffer 89 7d>
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 4 }
{ state: 0 }
{ state: 0 }
<Buffer 88 02>
{ state: 4 }
{ state: 0 }
<Buffer 88 00>
{ state: 4 }

Uzlopak avatar May 13 '24 06:05 Uzlopak

It's literally a single bug and the rest of the tests will start passing. That's super annoying.

KhafraDev avatar May 13 '24 18:05 KhafraDev

Its super annoying that the autobahn workflow did not post atleast once a comment on the PR. I would like to see it. Anyhow, we should set then the env variable FAIL_ON_ERROR to true in the workflow to fail if somebody adds a PR which breaks the autobahn.

Also i wonder, there are some tests reporting being not strict. We should review them. Maybe we beed to make them strict?

Uzlopak avatar May 13 '24 19:05 Uzlopak

non-strict is passing, no reason to touch them until there's nothing else to do

KhafraDev avatar May 13 '24 19:05 KhafraDev