Conversation
|
I don't use Socket.io and am not familiar with building and testing it so this is not tested at all, should work but should be reviewed and tested |
e3dio
force-pushed
the
fix-uws-polling
branch
from
69c4583 to
7d033f1
Compare
| return; | ||
| } | ||
|
|
||
| const contentLengthHeader = Number(req.headers["content-length"]); |
There was a problem hiding this comment.
If the content is chunked (Transfer-Encoding: chunked), won't the Content-Length header be undefined there?
There was a problem hiding this comment.
transfer-encoding: chunked POST's from the client are not supported by uWS, see uNetworking/uWebSockets.js#669 it is only supported for responses. The Socket.io client needs to not use it for this case
There was a problem hiding this comment.
OK, thanks for the link. But in that case, what happened in the issue? Does it throw on the first chunk?
There was a problem hiding this comment.
I see there is specific http code 411 for Length Required, I will update that https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/411 then the client knows to use length
There was a problem hiding this comment.
Ok I tested this with new Node v17.5 fetch using { body: createReadStream() } to send transfer-encoding: chunked request, uWS.js successfully responds with status 411 Length Required when request is missing content-length header
e3dio
force-pushed
the
fix-uws-polling
branch
from
e5c469b to
7487d7a
Compare
|
I changed existing |
|
I saw this comment from @darrachequesne on recent commit:
A normal POST does not use |
properly handle multiple onData events
replace Object.keys(headers).forEach remove nested inner functions
e3dio
force-pushed
the
fix-uws-polling
branch
from
f4bcf7d to
9ccbc34
Compare
With the engine based on µWebSockets.js (introduced in version 6.1.0), a huge request body split in multiple chunks would throw the following error: > node:buffer:254 > TypedArrayPrototypeSet(target, source, targetStart); > ^ > > TypeError: Cannot perform %TypedArray%.prototype.set on a detached ArrayBuffer > at Buffer.set (<anonymous>) > at _copyActual (node:buffer:254:3) > node:buffer:254 > TypedArrayPrototypeSet(target, source, targetStart); > ^ > > TypeError: Cannot perform %TypedArray%.prototype.set on a detached ArrayBuffer > at Buffer.set (<anonymous>) > at _copyActual (node:buffer:254:3) > at Function.concat (node:buffer:562:12) > at onEnd (.../node_modules/engine.io/build/transports-uws/polling.js:126:32) > at .../node_modules/engine.io/build/transports-uws/polling.js:143:17 Note: µWebSockets.js does not currently support chunked transfer encoding.
|
Merged as 3367440. Thanks. |
2 participants
The kind of change this PR does introduce
Current behaviour
Polling error, see socketio/socket.io#4281
New behaviour
Prevent error, also improves performance: reduces copies from 2 to 0 or 1, replaces Object.keys(headers).forEach, removes nested inner functions