From f4e1a45b667bfa0b95f78cddfb027b6c6f01272b Mon Sep 17 00:00:00 2001 From: Juri Gavshin <51777744+bolt-juri-gavshin@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:28:11 +0100 Subject: [PATCH] fix(node-http-handler): skip sending body without waiting for a timeout on response, if "expect" request header with "100-continue" is provided (#1459) * fix(node-http-handler): skip sending body without waiting for a timeout on response, if "expect" request header with "100-continue" is provided * fix(node-http-handler): skip sending body without waiting for a timeout on response, if "expect" request header with "100-continue" is provided * add unit tests --------- Co-authored-by: George Fu --- .changeset/hungry-eels-accept.md | 5 ++ .../src/write-request-body.spec.ts | 62 +++++++++++++++++++ .../src/write-request-body.ts | 18 ++++-- 3 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 .changeset/hungry-eels-accept.md create mode 100644 packages/node-http-handler/src/write-request-body.spec.ts diff --git a/.changeset/hungry-eels-accept.md b/.changeset/hungry-eels-accept.md new file mode 100644 index 00000000000..c4d0d690491 --- /dev/null +++ b/.changeset/hungry-eels-accept.md @@ -0,0 +1,5 @@ +--- +"@smithy/node-http-handler": patch +--- + +skip sending body without waiting for a timeout on response, if "expect" request header with "100-continue" is provided diff --git a/packages/node-http-handler/src/write-request-body.spec.ts b/packages/node-http-handler/src/write-request-body.spec.ts new file mode 100644 index 00000000000..bc0309034c9 --- /dev/null +++ b/packages/node-http-handler/src/write-request-body.spec.ts @@ -0,0 +1,62 @@ +import EventEmitter from "events"; +import { describe, expect, test as it, vi } from "vitest"; + +import { writeRequestBody } from "./write-request-body"; + +describe(writeRequestBody.name, () => { + it("should wait for the continue event if request has expect=100-continue", async () => { + const httpRequest = Object.assign(new EventEmitter(), { + end: vi.fn(), + }) as any; + const request = { + headers: { + expect: "100-continue", + }, + body: Buffer.from("abcd"), + method: "GET", + hostname: "", + protocol: "https:", + path: "/", + }; + let done: (value?: unknown) => void; + const promise = new Promise((r) => (done = r)); + setTimeout(async () => { + httpRequest.emit("continue", {}); + done(); + }, 25); + await writeRequestBody(httpRequest, request); + expect(httpRequest.end).toHaveBeenCalled(); + await promise; + }); + it( + "should not send the body if the request is expect=100-continue" + + "but a response is received before the continue event", + async () => { + const httpRequest = Object.assign(new EventEmitter(), { + end: vi.fn(), + }) as any; + const request = { + headers: { + expect: "100-continue", + }, + body: { + pipe: vi.fn(), + }, + method: "GET", + hostname: "", + protocol: "https:", + path: "/", + }; + let done: (value?: unknown) => void; + const promise = new Promise((r) => (done = r)); + setTimeout(() => { + httpRequest.emit("response", {}); + done(); + }, 25); + await writeRequestBody(httpRequest, request); + expect(request.body.pipe).not.toHaveBeenCalled(); + expect(httpRequest.end).not.toHaveBeenCalled(); + await promise; + } + ); +}); diff --git a/packages/node-http-handler/src/write-request-body.ts b/packages/node-http-handler/src/write-request-body.ts index 7c3e9bf72ab..d1ee0d6b831 100644 --- a/packages/node-http-handler/src/write-request-body.ts +++ b/packages/node-http-handler/src/write-request-body.ts @@ -23,32 +23,38 @@ export async function writeRequestBody( const expect = headers["Expect"] || headers["expect"]; let timeoutId = -1; - let hasError = false; + let sendBody = true; if (expect === "100-continue") { - await Promise.race([ + sendBody = await Promise.race([ new Promise((resolve) => { timeoutId = Number(timing.setTimeout(resolve, Math.max(MIN_WAIT_TIME, maxContinueTimeoutMs))); }), new Promise((resolve) => { httpRequest.on("continue", () => { timing.clearTimeout(timeoutId); - resolve(); + resolve(true); + }); + httpRequest.on("response", () => { + // if this handler is called, then response is + // already received and there is no point in + // sending body or waiting + timing.clearTimeout(timeoutId); + resolve(false); }); httpRequest.on("error", () => { - hasError = true; timing.clearTimeout(timeoutId); // this handler does not reject with the error // because there is already an error listener // on the request in node-http-handler // and node-http2-handler. - resolve(); + resolve(false); }); }), ]); } - if (!hasError) { + if (sendBody) { writeBody(httpRequest, request.body); } }