Unverified Commit 8174a586 authored by Hong Minhee's avatar Hong Minhee
Browse files

Merge pull request #335 from allouis/fix-request-clone-issues

parents 1615a640 fc7178a7
Loading
Loading
Loading
Loading
+12 −0
Original line number Diff line number Diff line
@@ -8,6 +8,18 @@ Version 1.7.7

To be released.

 -  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], [#335] by Fabien O'Carroll]

     -  Added `SignRequestOptions.body` option.
     -  Added `DoubleKnockOptions.body` option.
     -  Updated internal signing functions to accept pre-read body buffers.

[#300]: https://github.com/fedify-dev/fedify/pull/300
[#335]: https://github.com/fedify-dev/fedify/pull/335


Version 1.7.6
-------------
+40 −24
Original line number Diff line number Diff line
@@ -58,6 +58,12 @@ export interface SignRequestOptions {
   */
  currentTime?: Temporal.Instant;

  /**
   * The request body as ArrayBuffer. If provided, avoids cloning the request body.
   * @since 1.7.7
   */
  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,13 +150,15 @@ 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"
  const body: ArrayBuffer | null = bodyBuffer !== undefined
    ? bodyBuffer
    : request.method !== "GET" && request.method !== "HEAD"
    ? await request.clone().arrayBuffer()
    : null;
  const headers = new Headers(request.headers);
@@ -382,14 +392,16 @@ 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"
  const body: ArrayBuffer | null = bodyBuffer !== undefined
    ? bodyBuffer
    : request.method !== "GET" && request.method !== "HEAD"
    ? await request.clone().arrayBuffer()
    : null;

@@ -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.7.7
   */
  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 {
  const url = new URL(location, request.url);
  return new Request(url, {
@@ -1270,11 +1288,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, {
@@ -1289,13 +1315,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).
@@ -1324,7 +1347,7 @@ export async function doubleKnock(
      request,
      identity.privateKey,
      identity.keyId,
      { spec, tracerProvider },
      { spec, tracerProvider, body },
    );
    log?.(signedRequest);
    response = await fetch(signedRequest, {
@@ -1339,17 +1362,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);