Merge pull request #300 from allouis/request-clone-unusable-error

Author: dahlia

CHANGES.md

@@ -52,6 +52,15 @@ To be released.
  -  Added `LookupWebFingerOptions.maxRedirection` option.
     [[#248], [#281] by Lee ByeongJun]
 
+ -  Optimized `doubleKnock()` function to avoid multiple request body clones
+    during redirects.  The request body is now read once and reused throughout
+    the entire operation, preventing potential `TypeError: unusable` errors
+    and improving performance.  [[#300] by Fabien O'Carroll]
+
+     -  Added `SignRequestOptions.body` option.
+     -  Added `DoubleKnockOptions.body` option.
+     -  Updated internal signing functions to accept pre-read body buffers.
+
  -  Added `fedify webfinger` command. This command allows users to look up
     WebFinger information for a given resource.
     [[#260], [#278] by ChanHaeng Lee]
@@ -75,6 +84,7 @@ To be released.
 [#281]: https://github.com/fedify-dev/fedify/pull/281
 [#282]: https://github.com/fedify-dev/fedify/pull/282
 [#285]: https://github.com/fedify-dev/fedify/pull/285
+[#300]: https://github.com/fedify-dev/fedify/pull/300
 
 
 Version 1.7.5

fedify/sig/http.ts

@@ -58,6 +58,12 @@ export interface SignRequestOptions {
    */
   currentTime?: Temporal.Instant;
 
+  /**
+   * The request body as ArrayBuffer. If provided, avoids cloning the request body.
+   * @since 1.8.0
+   */
+  body?: ArrayBuffer | null;
+
   /**
    * The OpenTelemetry tracer provider.  If omitted, the global tracer provider
    * is used.
@@ -102,6 +108,7 @@ export async function signRequest(
             keyId,
             span,
             options.currentTime,
+            options.body,
           );
         } else {
           // Default to draft-cavage
@@ -111,6 +118,7 @@ export async function signRequest(
             keyId,
             span,
             options.currentTime,
+            options.body,
           );
         }
 
@@ -142,15 +150,17 @@ async function signRequestDraft(
   keyId: URL,
   span: Span,
   currentTime?: Temporal.Instant,
+  bodyBuffer?: ArrayBuffer | null,
 ): Promise<Request> {
   if (privateKey.algorithm.name !== "RSASSA-PKCS1-v1_5") {
     throw new TypeError("Unsupported algorithm: " + privateKey.algorithm.name);
   }
   const url = new URL(request.url);
-  const body: ArrayBuffer | null =
-    request.method !== "GET" && request.method !== "HEAD"
-      ? await request.clone().arrayBuffer()
-      : null;
+  const body: ArrayBuffer | null = bodyBuffer !== undefined
+    ? bodyBuffer
+    : request.method !== "GET" && request.method !== "HEAD"
+    ? await request.clone().arrayBuffer()
+    : null;
   const headers = new Headers(request.headers);
   if (!headers.has("Host")) {
     headers.set("Host", url.host);
@@ -382,16 +392,18 @@ async function signRequestRfc9421(
   keyId: URL,
   span: Span,
   currentTime?: Temporal.Instant,
+  bodyBuffer?: ArrayBuffer | null,
 ): Promise<Request> {
   if (privateKey.algorithm.name !== "RSASSA-PKCS1-v1_5") {
     throw new TypeError("Unsupported algorithm: " + privateKey.algorithm.name);
   }
 
   const url = new URL(request.url);
-  const body: ArrayBuffer | null =
-    request.method !== "GET" && request.method !== "HEAD"
-      ? await request.clone().arrayBuffer()
-      : null;
+  const body: ArrayBuffer | null = bodyBuffer !== undefined
+    ? bodyBuffer
+    : request.method !== "GET" && request.method !== "HEAD"
+    ? await request.clone().arrayBuffer()
+    : null;
 
   const headers = new Headers(request.headers);
   if (!headers.has("Host")) {
@@ -1214,6 +1226,12 @@ export interface DoubleKnockOptions {
    */
   log?: (request: Request) => void;
 
+  /**
+   * The request body as ArrayBuffer. If provided, avoids cloning the request body.
+   * @since 1.8.0
+   */
+  body?: ArrayBuffer | null;
+
   /**
    * The OpenTelemetry tracer provider.  If omitted, the global tracer provider
    * is used.
@@ -1225,13 +1243,13 @@ export interface DoubleKnockOptions {
  * Helper function to create a new Request for redirect handling.
  * @param request The original request.
  * @param location The redirect location.
- * @param body The request body as ArrayBuffer or undefined.
+ * @param body The request body as ArrayBuffer or null.
  * @returns A new Request object for the redirect.
  */
 function createRedirectRequest(
   request: Request,
   location: string,
-  body: ArrayBuffer | undefined,
+  body: ArrayBuffer | null,
 ): Request {
   return new Request(location, {
     method: request.method,
@@ -1269,11 +1287,19 @@ export async function doubleKnock(
   const firstTrySpec: HttpMessageSignaturesSpec = specDeterminer == null
     ? "rfc9421"
     : await specDeterminer.determineSpec(origin);
+
+  // Get the request body once at the top level to avoid multiple clones
+  const body = options.body !== undefined
+    ? options.body
+    : request.method !== "GET" && request.method !== "HEAD"
+    ? await request.clone().arrayBuffer()
+    : null;
+
   let signedRequest = await signRequest(
     request,
     identity.privateKey,
     identity.keyId,
-    { spec: firstTrySpec, tracerProvider },
+    { spec: firstTrySpec, tracerProvider, body },
   );
   log?.(signedRequest);
   let response = await fetch(signedRequest, {
@@ -1288,13 +1314,10 @@ export async function doubleKnock(
     response.headers.has("Location")
   ) {
     const location = response.headers.get("Location")!;
-    const body = request.method !== "GET" && request.method !== "HEAD"
-      ? await request.clone().arrayBuffer()
-      : undefined;
     return doubleKnock(
       createRedirectRequest(request, location, body),
       identity,
-      options,
+      { ...options, body },
     );
   } else if (
     // FIXME: Temporary hotfix for Mastodon RFC 9421 implementation bug (as of 2025-06-19).
@@ -1323,7 +1346,7 @@ export async function doubleKnock(
       request,
       identity.privateKey,
       identity.keyId,
-      { spec, tracerProvider },
+      { spec, tracerProvider, body },
     );
     log?.(signedRequest);
     response = await fetch(signedRequest, {
@@ -1338,17 +1361,10 @@ export async function doubleKnock(
       response.headers.has("Location")
     ) {
       const location = response.headers.get("Location")!;
-      // IMPORTANT: Use arrayBuffer() instead of .body to prevent "TypeError: unusable"
-      // When using .body (ReadableStream), subsequent clone() calls in signRequest functions
-      // will fail because the stream has already been consumed. Using arrayBuffer() ensures
-      // the body can be safely cloned for HTTP signature generation.
-      const body = request.method !== "GET" && request.method !== "HEAD"
-        ? await request.clone().arrayBuffer()
-        : undefined;
       return doubleKnock(
         createRedirectRequest(request, location, body),
         identity,
-        options,
+        { ...options, body },
       );
     } else if (response.status !== 400 && response.status !== 401) {
       await specDeterminer?.rememberSpec(origin, spec);