From fbd06ebd631ce8a1a642380abb499dcbb7ea1329 Mon Sep 17 00:00:00 2001
From: Tommy Brunn <tommy.brunn@gmail.com>
Date: Wed, 15 Jan 2025 20:43:27 +0100
Subject: [PATCH] fix(node-http-handler): Write request body if 100 Continue
 response takes more than 1 second (#1505)

* fix(node-http-handler): Write request body if 100 Continue response times out

* set 100-continue timeout to 6s minimum

---------

Co-authored-by: George Fu <kuhe@users.noreply.github.com>
---
 .changeset/good-buttons-relax.md              |  5 +++
 .../src/write-request-body.spec.ts            | 34 ++++++++++++++++++-
 .../src/write-request-body.ts                 | 14 ++++++--
 3 files changed, 49 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/good-buttons-relax.md

diff --git a/.changeset/good-buttons-relax.md b/.changeset/good-buttons-relax.md
new file mode 100644
index 00000000000..b9cd6569f0d
--- /dev/null
+++ b/.changeset/good-buttons-relax.md
@@ -0,0 +1,5 @@
+---
+"@smithy/node-http-handler": patch
+---
+
+fix sending request when 100 Continue response takes more than 1 second
diff --git a/packages/node-http-handler/src/write-request-body.spec.ts b/packages/node-http-handler/src/write-request-body.spec.ts
index bc0309034c9..6e32edd304e 100644
--- a/packages/node-http-handler/src/write-request-body.spec.ts
+++ b/packages/node-http-handler/src/write-request-body.spec.ts
@@ -1,5 +1,5 @@
 import EventEmitter from "events";
-import { describe, expect, test as it, vi } from "vitest";
+import { afterEach, beforeEach, describe, expect, test as it, vi } from "vitest";
 
 import { writeRequestBody } from "./write-request-body";
 
@@ -59,4 +59,36 @@ describe(writeRequestBody.name, () => {
       await promise;
     }
   );
+
+  describe("with fake timers", () => {
+    beforeEach(() => {
+      vi.useFakeTimers();
+    });
+
+    afterEach(() => {
+      vi.useRealTimers();
+    });
+    it("should send the body if the 100 Continue response is not received before the timeout", 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: "/",
+      };
+
+      const promise = writeRequestBody(httpRequest, request, 12_000);
+      expect(httpRequest.end).not.toHaveBeenCalled();
+      vi.advanceTimersByTime(13000);
+      await promise;
+
+      expect(httpRequest.end).toHaveBeenCalled();
+    });
+  });
 });
diff --git a/packages/node-http-handler/src/write-request-body.ts b/packages/node-http-handler/src/write-request-body.ts
index d1ee0d6b831..d07281130e6 100644
--- a/packages/node-http-handler/src/write-request-body.ts
+++ b/packages/node-http-handler/src/write-request-body.ts
@@ -5,14 +5,14 @@ import { Readable } from "stream";
 
 import { timing } from "./timing";
 
-const MIN_WAIT_TIME = 1000;
+const MIN_WAIT_TIME = 6_000;
 
 /**
  * This resolves when writeBody has been called.
  *
  * @param httpRequest - opened Node.js request.
  * @param request - container with the request body.
- * @param maxContinueTimeoutMs - maximum time to wait for the continue event. Minimum of 1000ms.
+ * @param maxContinueTimeoutMs - time to wait for the continue event.
  */
 export async function writeRequestBody(
   httpRequest: ClientRequest | ClientHttp2Stream,
@@ -28,7 +28,15 @@ export async function writeRequestBody(
   if (expect === "100-continue") {
     sendBody = await Promise.race<boolean>([
       new Promise((resolve) => {
-        timeoutId = Number(timing.setTimeout(resolve, Math.max(MIN_WAIT_TIME, maxContinueTimeoutMs)));
+        // If this resolves first (wins the race), it means that at least MIN_WAIT_TIME ms
+        // elapsed and no continue, response, or error has happened.
+        // The high default timeout is to give the server ample time to respond.
+        // This is an unusual situation, and indicates the server may not be S3 actual
+        // and did not correctly implement 100-continue event handling.
+        // Strictly speaking, we should perhaps keep waiting up to the request timeout
+        // and then throw an error, but we resolve true to allow the server to deal
+        // with the request body.
+        timeoutId = Number(timing.setTimeout(() => resolve(true), Math.max(MIN_WAIT_TIME, maxContinueTimeoutMs)));
       }),
       new Promise((resolve) => {
         httpRequest.on("continue", () => {