Non-GET requests don't work when passed as a single Request argument

[sbking]

When passing a non-GET Request object as a single argument, the method is automatically changed to "GET".

[zirkelc]

Hi @sbking thanks for reporting this. I'm going to look into this.

Do you have an example how you call it with a Request object?

[sbking]

Hi @zirkelc, I'm trying to use this with a REST API client generated by @hey-api/openapi-ts. Here is where they construct a new Request() and pass it to the provided fetch function:

https://github.com/hey-api/openapi-ts/blob/main/packages/client-fetch/src/index.ts#L62-L69

And a little example on MDN:

https://developer.mozilla.org/en-US/docs/Web/API/Window/fetch#examples

Something like this is a minimal example:

const request = new Request("https://example.com", { method: "POST" });
fetch(request);

[zirkelc]

Hi @sbking, I have fixed this issue and released a new version v4.0.1.

Could you please check if it works for you?

[sbking]

@zirkelc Unfortunately now I'm getting this when sending a POST request:

TypeError: RequestInit: duplex option is required when sending a body.

[zirkelc]

@sbking I had the same issue, however, it looks like this is specific to Node.js implementation of fetch(). I can reproduce the same error on Node.js v20 and plain fetch(). Taking this example from Google Chrome:

(async () => {
  const stream = new ReadableStream({
    async start(controller) {
      controller.enqueue("This ");
      controller.enqueue("is ");
      controller.enqueue("a ");
      controller.enqueue("slow ");
      controller.enqueue("request.");
      controller.close();
    },
  }).pipeThrough(new TextEncoderStream());

  const request = new Request("https://example.com", {
    method: "POST",
    body: stream,
    // duplex: "half",
  });
  const response = await fetch(request);
  console.log(response);
})();

2024-08-21_08-55-46.mp4

If your body is a stream, you have to set the duplex option to half. It is not required for string body though. Looking at you library, it applies some kind of serialization to the body:

https://github.com/hey-api/openapi-ts/blob/7e84960713160fc942c6e5a109e7d9687fb9dafc/packages/client-fetch/src/index.ts#L37-L39

So maybe it is streaming the body even though you supply only a string.

Can you set the duplex: "half" option as part of the RequestInit and see if it works? I know TypeScript is complaining that the option doesn't exist, but it surely does.

[sbking]

@zirkelc I don't really have control over RequestInit - the library gives me a Request object that already works with the vanilla Node 20 fetch, but not with this library's fetch

[zirkelc]

@sbking could you set up a small repository with some code so I can debug this issue on my side? No sensitive data or secret keys, just the code you are using and a few comments where I can insert my own AWS credentials to investigate it further

[sbking]

Hey @zirkelc I can try to set up a small reproduction repo when I have some time. For now, I'm using this instead which seems to be working perfectly. I believe the trick is the call to request.clone() which seems to respect whatever the original request's duplex option is:

export async function sigv4Fetch(request: Request) {
  const newRequest = request.clone();

  const url = new URL(request.url);
  const body = await request.text();
  const headers = {
    ...Object.fromEntries(request.headers.entries()),
    // host is required by AWS Signature V4: https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
    host: url.host,
  };

  const smithyRequest = new HttpRequest({
    hostname: url.hostname,
    path: url.pathname,
    protocol: url.protocol,
    port: url.port ? Number(url.port) : undefined,
    username: url.username,
    password: url.password,
    method: request.method.toUpperCase(),
    body,
    query: Object.fromEntries(url.searchParams.entries()),
    fragment: url.hash,
    headers,
  });

  const signer = new SignatureV4({
    credentials: fromNodeProviderChain(),
    service: "lambda",
    region: process.env.AWS_REGION ?? "us-east-1",
    sha256: Sha256,
  });

  const signedRequest = await signer.sign(smithyRequest);

  for (const [key, value] of Object.entries(signedRequest.headers)) {
    newRequest.headers.set(key, value);
  }

  return fetch(newRequest);
}

[zirkelc]

Hi @sbking good that you found a workaround. I will investigate this further. I assume the signer.sign() somehow changes the body and you avoid that by cloning the request and simply copying the signed headers back.

[zirkelc]

Hey @sbking I'm planning to give this issue another go. I want to try your solution and copy the signed headers instead of using the signedRequest.headers like I do now.

As I can't really reproduce the issue on my side, would you be able to try it? I will publish a release candidate package which you can temporarily install.

[zirkelc]

@sbking I've released v4.1.0 which only copies the headers to the request. Could you try if that works for you?

[florianbepunkt]

@zirkelc This does not solve the issue I'm afraid.

See nodejs/node#46221

And the spec change introduced here: whatwg/fetch#1457

Not sure why they did not use a default value as there is only one option, but you need to add duplex: half to the request.

You can repro this rather easily: Run fetch post call to an api gateway (or basically any server/endpoint) on Node@22 with a body and omit the duplex option.

[zirkelc]

@florianbepunkt thank you very much for these links, they have finally put me on the right track.

I think there are two different issues here:

1. Fetching with a Request({ duplex: "half" }) should work

I was able to identify the cause of the @sbking issue: I parse the Request and RequestInit to extract the URL, body and headers to sign them with SignV4:

https://github.com/zirkelc/aws-sigv4-fetch/blob/44d19e270783e8566c2f39eebc1eb83e1b8af3ad/src/create-signed-fetcher.ts#L34

However, the final signed fetch() at the end does not pass the original Request object:
https://github.com/zirkelc/aws-sigv4-fetch/blob/44d19e270783e8566c2f39eebc1eb83e1b8af3ad/src/create-signed-fetcher.ts#L62-L68

This means that any duplex option on Request object was lost. I fixed this by refactoring the parseRequest() function.

2. Node.js expects the duplex option to be set when a body is sent through the request object

I can confirm that adding duplex: "half" will fix this issue. However, I am not sure if this library should apply this value without the sender knowing about it. The problem I see is that there could be other values for duplex in the future, or different runtime behavior of bun or deno, so blindly setting this value could have unexpected side effects that are hard to debug.

What do you think?

Btw, I created PR #25 if you want to take a look.

[florianbepunkt]

Thanks. Spent half of my day on this. Currently I got it working with this:

import { SignatureV4 } from "@smithy/signature-v4";
import { Sha256 } from "@aws-crypto/sha256-js";
import { HttpRequest } from "@smithy/protocol-http";

async function signRequest2(request: Request) {
  const url = new URL(request.url);
  const headers = new Headers(request.headers);
  const body = request.method !== "GET" && request.method !== "HEAD" ? await request.text() : undefined;

  const credentials = {
    // redacted
  }

  // Construct the HttpRequest for SigV4
  const awsRequest = new HttpRequest({
    protocol: url.protocol.slice(0, -1), // "https:"
    hostname: url.hostname,
    port: url.port ? parseInt(url.port) : undefined,
    method: request.method,
    path: url.pathname,
    query: Object.fromEntries(url.searchParams),
    headers: Object.fromEntries(headers),
    body, // Only include the body for non-GET/HEAD requests
  });

  // Create the SigV4 signer
  const signer = new SignatureV4({
    credentials: credentials,
    sha256: Sha256,
    region: "eu-central-1",
    service: "execute-api",
  });

  // Sign the request
  const signedRequest = await signer.sign(awsRequest);

  // Convert the signed request back to a fetch-compatible format
  const signedHeaders = new Headers(signedRequest.headers);

  return new Request(request.url, {
    method: signedRequest.method,
    headers: signedHeaders,
    body: signedRequest.body, // Include the body if it exists
  });
}

Interestingly enough, this works without specifying duplex: "half" in the request. Not sure why.

Will test your PR tomorrow.

[zirkelc]

Interestingly enough, this works without specifying duplex: "half" in the request. Not sure why.

In which environment / Node.js version did you try it? Maybe Request or fetch was somehow patched by another dependency.

Will test your PR tomorrow.

Thank you!

[florianbepunkt]

@zirkelc Sorry, took me a bit longer. Tried your PR, but still got a 403. I went with my solution above for the time being.

[zirkelc]

@florianbepunkt it's not working for me either, and I'm trying to figure out the issue. I'll let you know when I know more.

[zirkelc]

Hey @florianbepunkt

I think I finally managed to fix it. I now always create a Request object based on the original input and init objects, that means whatever options is present on either of them will be reflected in the final signed request. Only the headers are replaced with signed ones. If you pass in signedFetch(new Request(url, { duplex: 'half' })) it will be carried over to the signed request.

If you get a chance to test it, you can install it from the PR: #25 (comment)

Also in response to your previous comment #19 (comment):

Interestingly enough, this works without specifying duplex: "half" in the request. Not sure why.

I think this works without setting duplex because you consume the body as string and create a new Request:

const body = request.method !== "GET" && request.method !== "HEAD" ? await request.text() : undefined;

Not sure if this works with all body types the same way.

[zirkelc]

Hey @sbking and @florianbepunkt

I released v4.2.0 which should close finally this issue 🙏

Thanks to @florianbepunkt I identified the root cause: when provided with a Request object like await signedFetch(new Request('https:/...', { body: 'foo' })) I didn't extract all properties correctly, in particular the duplex property. The duplex property is required in Node.js fetch, but is not available on the TypeScript types.
The new implementation will always create a Request object, therefore copying all available properties to the signed request.

Thank you for your help!