Skip to content

Commit 915a3ed

Browse files
authored
fix(eio): run the polling write callback when the client aborts a compressed response (#5540)
Since [1] the write callback of doWrite() only ran on the response's 'finish' event. When the client aborted mid-stream, 'finish' never fired and the zlib pipeline reported no error, so the callback was lost: the pending request was not cleaned up and the send callbacks queued for that batch were never invoked. The callback now also runs on 'close', guarded so it still fires exactly once. Co-authored-by: chuanghiduoc <chuanghiduoc@users.noreply.github.com> [1]: 91dd763
1 parent ae7fb46 commit 915a3ed

2 files changed

Lines changed: 90 additions & 5 deletions

File tree

packages/engine.io/lib/transports/polling.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,18 +322,27 @@ export class Polling extends Transport {
322322
const stream = compressionMethods[encoding](this.httpCompression);
323323

324324
let isErrored = false;
325+
let isDone = false;
326+
327+
const done = () => {
328+
if (isDone || isErrored) {
329+
return;
330+
}
331+
isDone = true;
332+
callback();
333+
};
325334

326335
stream.on("error", (err) => {
327336
isErrored = true;
328337
this.res.end();
329338
callback(err);
330339
});
331340

332-
this.res.once("finish", () => {
333-
if (!isErrored) {
334-
callback();
335-
}
336-
});
341+
// 'close' also fires after a normal completion, hence the guard: whatever
342+
// happens, the write callback must run exactly once so the transport
343+
// cleans up its request state and emits 'drain'.
344+
this.res.once("finish", done);
345+
this.res.once("close", done);
337346

338347
stream.pipe(this.res);
339348
stream.end(data);
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/* eslint-disable standard/no-callback-literal */
2+
3+
const http = require("http");
4+
const crypto = require("crypto");
5+
const cookieMod = require("cookie");
6+
const { listen } = require("./common");
7+
const expect = require("expect.js");
8+
9+
function getSidFromResponse(res) {
10+
const c = cookieMod.parse(res.headers["set-cookie"][0]);
11+
return c[Object.keys(c)[0]];
12+
}
13+
14+
describe("polling compression", () => {
15+
let engine;
16+
17+
afterEach(() => {
18+
if (engine && engine.httpServer) {
19+
engine.httpServer.close();
20+
}
21+
});
22+
23+
it("should not lose the write callback when the client aborts a compressed response mid-stream", (done) => {
24+
engine = listen(
25+
{
26+
cookie: true,
27+
transports: ["polling"],
28+
httpCompression: { threshold: 0 },
29+
pingInterval: 60000,
30+
pingTimeout: 60000,
31+
},
32+
(port) => {
33+
// incompressible content so real bytes keep flowing to the socket
34+
const chunk = crypto.randomBytes(1024 * 1024).toString("base64");
35+
let sendCallbackCalled = false;
36+
37+
engine.on("connection", (c) => {
38+
const spam = setInterval(() => {
39+
if (c.readyState !== "open") {
40+
clearInterval(spam);
41+
return;
42+
}
43+
c.send(chunk, () => {
44+
sendCallbackCalled = true;
45+
});
46+
}, 5);
47+
setTimeout(() => clearInterval(spam), 2000);
48+
});
49+
50+
http.get({ port, path: "/engine.io/?transport=polling" }, (res) => {
51+
const sid = getSidFromResponse(res);
52+
const pollReq = http.get(
53+
{
54+
port,
55+
path: "/engine.io/?transport=polling&sid=" + sid,
56+
headers: { "Accept-Encoding": "gzip, deflate" },
57+
},
58+
(pollRes) => {
59+
// abort on headers, before any body byte is written
60+
pollReq.destroy();
61+
setTimeout(() => {
62+
try {
63+
expect(sendCallbackCalled).to.be(true);
64+
done();
65+
} catch (e) {
66+
done(e);
67+
}
68+
}, 1500);
69+
},
70+
);
71+
pollReq.on("error", () => {}); // expected: socket hang up
72+
});
73+
},
74+
);
75+
});
76+
});

0 commit comments

Comments
 (0)