Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix uws polling #642

Closed
wants to merge 2 commits into from
Closed

fix uws polling #642

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb11d83
fix uws polling
e3dio Feb 18, 2022
9ccbc34
perf update
e3dio Feb 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 58 additions & 28 deletions lib/transports-uws/polling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,18 @@ export class Polling extends Transport {
return;
}

const contentLengthHeader = Number(req.headers["content-length"]);
Copy link
Member

Choose a reason for hiding this comment

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

If the content is chunked (Transfer-Encoding: chunked), won't the Content-Length header be undefined there?

Copy link
Contributor Author

@e3dio e3dio Feb 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

@darrachequesne darrachequesne Feb 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

OK, thanks for the link. But in that case, what happened in the issue? Does it throw on the first chunk?

Copy link
Contributor Author

@e3dio e3dio Feb 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

if (!contentLengthHeader) {
this.onError("content-length header required");
res.writeStatus("411 Length Required").end();
return;
}
if (contentLengthHeader > 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,55 +143,73 @@ export class Polling extends Transport {
this.dataReq = req;
this.dataRes = res;

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

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

const onClose = () => {
cleanup();
this.onError("data request connection closed prematurely");
};

const headers = {
// text/html is required instead of text/plain to avoid an
// unwanted download dialog on certain user-agents (GH-43)
"Content-Type": "text/html"
};

this.headers(req, headers);
Object.keys(headers).forEach(key => {
for (let key in headers) {
res.writeHeader(key, String(headers[key]));
});

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

if (this.readyState !== "closing") {
res.end("ok");
}
cleanup();
const onEnd = (buffer) => {
this.onData(buffer.toString());
this.onDataRequestCleanup();
res.end("ok");
};

res.onAborted(onClose);
res.onAborted(() => {
this.onDataRequestCleanup();
this.onError("data request connection closed prematurely");
});

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 = contentLength + arrayBuffer.byteLength;
if (totalLength > contentLengthHeader) {
this.onError("content-length mismatch");
res.close(); // calls onAborted
return;
}

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

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

if (isLast) {
onEnd();
if (totalLength != contentLengthHeader) {
this.onError("content-length mismatch");
res.writeStatus("400 content-length mismatch").end();
this.onDataRequestCleanup();
return;
}
onEnd(buffer);
return;
}

contentLength = totalLength;
});
}

/**
* Cleanup onDataRequest.
*
* @api private
*/
onDataRequestCleanup() {
this.dataReq = this.dataRes = null;
}

/**
* Processes the incoming data payload.
*
Expand Down