Unverified Commit 1b389175 authored by Hong Minhee (洪 民憙)'s avatar Hong Minhee (洪 民憙) Committed by GitHub
Browse files

Merge pull request #295 from dahlia/fix/294

parents df9a24f0 b12e8cb9
Loading
Loading
Loading
Loading
+7 −0
Original line number Diff line number Diff line
@@ -8,6 +8,13 @@ Version 1.6.6

To be released.

 -  Fixed `TypeError: unusable` error that occurred when `doubleKnock()`
    encountered redirects during HTTP signature retry attempts.
    [[#294], [#295]]

[#294]: https://github.com/fedify-dev/fedify/issues/294
[#295]: https://github.com/fedify-dev/fedify/pull/295


Version 1.6.5
-------------
+58 −0
Original line number Diff line number Diff line
@@ -1877,3 +1877,61 @@ test("verifyRequest() [rfc9421] error handling for invalid signature base creati
    "Verification should fail gracefully for malformed signature inputs",
  );
});

test("doubleKnock() regression test for TypeError: unusable bug #294", async () => {
  // This test reproduces the bug where request.clone().body in the second redirect
  // handling path causes "TypeError: unusable" when the body is consumed before
  // subsequent clone() calls in signRequest functions.

  fetchMock.spyGlobal();

  let requestCount = 0;

  // Mock server that:
  // 1. Returns 401 for first spec (triggers retry with different spec)
  // 2. Returns 302 redirect for second spec (triggers redirect handling)
  // 3. Returns 200 for final destination
  fetchMock.post("https://example.com/inbox-retry-redirect", (_cl) => {
    requestCount++;

    if (requestCount === 1) {
      // First request: reject to trigger retry with different spec
      return new Response("Unauthorized", { status: 401 });
    } else if (requestCount === 2) {
      // Second request: redirect to trigger the problematic redirect handling
      return Response.redirect("https://example.com/final-destination", 302);
    }

    return new Response("Should not reach here", { status: 500 });
  });

  // Mock final destination
  fetchMock.post("https://example.com/final-destination", () => {
    return new Response("Success", { status: 200 });
  });

  const request = new Request("https://example.com/inbox-retry-redirect", {
    method: "POST",
    body: "Test activity content",
    headers: {
      "Content-Type": "application/activity+json",
    },
  });

  // This should trigger the bug: 401 -> retry -> 302 -> TypeError: unusable
  // because the second redirect path uses request.clone().body instead of
  // await request.clone().arrayBuffer()
  const response = await doubleKnock(
    request,
    {
      keyId: rsaPublicKey2.id!,
      privateKey: rsaPrivateKey2,
    },
  );

  // The test should pass after the fix
  assertEquals(response.status, 200);
  assertEquals(requestCount, 2, "Should make 2 requests before redirect");

  fetchMock.hardReset();
});
+36 −16
Original line number Diff line number Diff line
@@ -1221,6 +1221,34 @@ export interface DoubleKnockOptions {
  tracerProvider?: TracerProvider;
}

/**
 * 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.
 * @returns A new Request object for the redirect.
 */
function createRedirectRequest(
  request: Request,
  location: string,
  body: ArrayBuffer | undefined,
): Request {
  return new Request(location, {
    method: request.method,
    headers: request.headers,
    body,
    redirect: "manual",
    signal: request.signal,
    mode: request.mode,
    credentials: request.credentials,
    referrer: request.referrer,
    referrerPolicy: request.referrerPolicy,
    integrity: request.integrity,
    keepalive: request.keepalive,
    cache: request.cache,
  });
}

/**
 * Performs a double-knock request to the given URL.  For the details of
 * double-knocking, see
@@ -1264,19 +1292,7 @@ export async function doubleKnock(
      ? await request.clone().arrayBuffer()
      : undefined;
    return doubleKnock(
      new Request(location, {
        method: request.method,
        headers: request.headers,
        body,
        redirect: "manual",
        signal: request.signal,
        mode: request.mode,
        credentials: request.credentials,
        referrer: request.referrer,
        referrerPolicy: request.referrerPolicy,
        integrity: request.integrity,
        keepalive: request.keepalive,
      }),
      createRedirectRequest(request, location, body),
      identity,
      options,
    );
@@ -1322,11 +1338,15 @@ 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"
        ? request.clone().body
        : null;
        ? await request.clone().arrayBuffer()
        : undefined;
      return doubleKnock(
        new Request(location, { ...request, body }),
        createRedirectRequest(request, location, body),
        identity,
        options,
      );