Skip to content

Commit

Permalink
feat: add check for closed requests (#162)
Browse files Browse the repository at this point in the history
## Checklist

- [x] I have ensured my pull request is not behind the main or master
branch of the original repository.
- [x] I have rebased all commits where necessary so that reviewing this
pull request can be done without having to merge it first.
- [x] I have written a commit message that passes commitlint linting.
- [x] I have ensured that my code changes pass linting tests.
- [x] I have ensured that my code changes pass unit tests.
- [x] I have described my pull request and the reasons for code changes
along with context if necessary.

## Changes

Added a check for `req.ctx.closed` to ensure that, when a request is
closed (canceled) before reaching the bodyparser middleware, the body of
the closed request is not attempted to be parsed (which would result in
an error). Instead, if the request is closed, then the middleware
returns a `499 Client Closed Request` status.

## Reason

Errors can occur when parsing bodies of closed requests - `stream not
readable`. This indicates the request was cancelled. This can happen
when body parser is not on the "synchronous path" of a request - e.g. if
Auth is verified asynchronously from headers before bothering to parse
the bodies. This allows time for users to occasionally close the
request, resulting in a `500 'stream not readable'` error.
  • Loading branch information
zbrydon authored Jun 14, 2024
1 parent e6fe74d commit 031348a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/body-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ export function bodyParserWrapper(opts: BodyParserOptions = {}) {
return next();
}

if (ctx.req.closed) {
ctx.status = 499;
ctx.body = 'Request already closed';
return;
}

try {
const response = await parseBody(ctx);
// patch node
Expand Down
15 changes: 15 additions & 0 deletions test/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -558,4 +558,19 @@ describe("test/body-parser.test.ts", () => {
.expect({ foo: "bar" });
});
});

describe("request closed", () => {
it("should return 499 on request closed", async () => {
const app = new Koa();

app.use(async (ctx, next) => {
Object.defineProperty(ctx.req, "closed", { value: true });
await next();
});
app.use(bodyParser());
server = app.listen();

await request(server).post("/").send({ foo: "bar" }).expect(499);
});
});
});

0 comments on commit 031348a

Please sign in to comment.