Skip to content

Commit

Permalink
fix(uws): properly handle chunked content
Browse files Browse the repository at this point in the history
On Safari, a POST request would result in two chunks (the first being
empty), and threw 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)
>     at Function.concat (node:buffer:562:12)
>     at onEnd (xxx/node_modules/engine.io/build/transports-uws/polling.js:126:32)
>     at xxx/node_modules/engine.io/build/transports-uws/polling.js:143:17

Which is a bit weird, because it seems µWebSockets.js does not support
chunked content: uNetworking/uWebSockets.js#669
  • Loading branch information
e3dio authored and darrachequesne committed Feb 22, 2022
1 parent a463d26 commit d970d66
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 13 deletions.
58 changes: 45 additions & 13 deletions lib/transports-uws/polling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,20 @@ export class Polling extends Transport {
return;
}

const expectedContentLength = Number(req.headers["content-length"]);

if (!expectedContentLength) {
this.onError("content-length header required");
res.writeStatus("411 Length Required").end();
return;
}

if (expectedContentLength > this.maxHttpBufferSize) {
this.onError("payload too large");
res.writeStatus("413 Payload Too Large").end();
return;
}

const isBinary = "application/octet-stream" === req.headers["content-type"];

if (isBinary && this.protocol === 4) {
Expand All @@ -131,11 +145,11 @@ export class Polling extends Transport {
this.dataReq = req;
this.dataRes = res;

let chunks = [];
let contentLength = 0;
let buffer;
let offset = 0;

const cleanup = () => {
this.dataReq = this.dataRes = chunks = null;
this.dataReq = this.dataRes = null;
};

const onClose = () => {
Expand All @@ -154,8 +168,8 @@ export class Polling extends Transport {
res.writeHeader(key, String(headers[key]));
});

const onEnd = () => {
this.onData(Buffer.concat(chunks).toString());
const onEnd = buffer => {
this.onData(buffer.toString());

if (this.readyState !== "closing") {
res.end("ok");
Expand All @@ -165,18 +179,36 @@ export class Polling extends Transport {

res.onAborted(onClose);

res.onData((chunk, isLast) => {
chunks.push(Buffer.from(chunk));
contentLength += Buffer.byteLength(chunk);
if (contentLength > this.maxHttpBufferSize) {
this.onError("payload too large");
res.writeStatus("413 Payload Too Large");
res.end();
res.onData((arrayBuffer, isLast) => {
const totalLength = offset + arrayBuffer.byteLength;
if (totalLength > expectedContentLength) {
this.onError("content-length mismatch");
res.close(); // calls onAborted
return;
}

if (!buffer) {
if (isLast) {
onEnd(Buffer.from(arrayBuffer));
return;
}
buffer = Buffer.allocUnsafe(expectedContentLength);
}

Buffer.from(arrayBuffer).copy(buffer, offset);

if (isLast) {
onEnd();
if (totalLength !== expectedContentLength) {
this.onError("content-length mismatch");
res.writeStatus("400 Content-Length Mismatch").end();
cleanup();
return;
}
onEnd(buffer);
return;
}

offset = totalLength;
});
}

Expand Down
35 changes: 35 additions & 0 deletions test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -1955,6 +1955,41 @@ describe("server", () => {
});
});

it.only("should arrive when content is chunked", function(done) {
if (process.env.EIO_WS_ENGINE === "uws") {
// µWebSockets.js does not currently support chunked encoding: https:/uNetworking/uWebSockets.js/issues/669
return this.skip();
}
const engine = listen(port => {
const client = new ClientSocket(`ws://localhost:${port}`, {
transports: ["polling"]
});

engine.on("connection", socket => {
socket.on("message", data => {
expect(data).to.eql("123");

client.close();
done();
});
});

client.on("open", () => {
const req = http.request({
host: "localhost",
port,
path: `/engine.io/?EIO=4&transport=polling&sid=${client.id}`,
method: "POST"
});

req.write(process.env.EIO_CLIENT === "3" ? "4:41" : "41");
req.write("2");
req.write("3");
req.end();
});
});
});

it("should arrive as ArrayBuffer if requested when binary data sent as Buffer (polling)", done => {
const binaryData = Buffer.allocUnsafe(5);
for (let i = 0; i < binaryData.length; i++) {
Expand Down

1 comment on commit d970d66

@e3dio
Copy link
Contributor Author

@e3dio e3dio commented on d970d66 Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your test does not test this issue. This issue is triggered for POST's with data above a certain size, like 200KB will trigger multiple "chunks" of onData callbacks and will cause issue. This has NOTHING to do with transfer-encoding: chunked protocol which just happens to have "chunked" in the name, they are 2 different things. If you are still confused please ask for clarification, because currently you don't understand

Please sign in to comment.