nodejs / undici

An HTTP/1.1 client, written from scratch for Node.js
https://nodejs.github.io/undici
MIT License
6.09k stars 531 forks source link

Failing autobahn tests #3252

Closed KhafraDev closed 4 months ago

KhafraDev commented 4 months ago

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 commented 4 months ago

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.

Uzlopak commented 4 months ago

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/.

> undici@6.16.1 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 commented 4 months ago

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/.

> undici@6.16.1 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 }
KhafraDev commented 4 months ago

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

Uzlopak commented 4 months ago

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?

KhafraDev commented 4 months ago

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