Commit fc7178a7 authored by Fabien O'Carroll's avatar Fabien O'Carroll Committed by Hong Minhee (洪 民憙)
Browse files

Fix doubleKnock request.clone() errors

This refactors the doubleKnock function and related signing functions to
get the request body once at the top level instead of cloning it
multiple times during iterations and redirects.

Changes:
- Add optional `body` parameter to SignRequestOptions and DoubleKnockOptions
- Update signRequestDraft and signRequestRfc9421 to accept pre-read body
- Modify doubleKnock to read body once and pass through all operations
- Update redirect handling to reuse the same body buffer
parent 1615a640
Loading
Loading
Loading
Loading
+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);