Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(fetch-http-handler): check for null fetch response.body #4705

Merged
merged 2 commits into from May 8, 2023

Conversation

kuhe
Copy link
Contributor

@kuhe kuhe commented May 6, 2023

Issue

#4398
#4369

Description

Add null check for fetch API response.body.

Testing

  • add unit test
  • manual testing with Chrome Version 113.0.5672.63 (Official Build) (x86_64)

Workaround

For affected users' clients, you can apply a code workaround until such time that you upgrade to a version of the AWS SDK for JavaScript v3 that contains this fix.

    import { S3 } from "@aws-sdk/client-s3";
    
    const s3 = new S3({
      credentials,
    });

    // this example is on an S3 client. The middlewareStack property is available on all clients.
    // add this middleware to default a fetch response body to an empty Uint8Array, 
    // before making any requests.
    s3.middlewareStack.addRelativeTo(
      (next) => async (args) => {
        const { response } = await next(args);
        if (response.body == null) {
          response.body = new Uint8Array();
        }
        return {
          response,
        };
      },
      {
        name: "nullFetchResponseBodyMiddleware",
        toMiddleware: "deserializerMiddleware",
        relation: "after",
        step: "deserialize",
        override: true,
      }
    );

Additional context

In the API spec for fetch's Response::body https://developer.mozilla.org/en-US/docs/Web/API/Response/body,
the body is supposed to be null for responses that have an empty body.

A note clarifies that this did not happen in browser implementations

Note: Current browsers don't actually conform to the spec requirement to set the body property to null for responses with no body (for example, responses to HEAD requests, or 204 No Content responses).

but with the latest version of Chrome, and possibly other browsers, the spec is now apparently being adhered to. This null check should have been in place originally, but is now necessary.

@kuhe kuhe marked this pull request as ready for review May 6, 2023 04:04
@kuhe kuhe requested review from a team as code owners May 6, 2023 04:04
@cezarderevlean
Copy link

Thank you for providing the workaround.

Co-authored-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
@kuhe kuhe merged commit 4468be6 into aws:main May 8, 2023
2 checks passed
@kuhe kuhe deleted the fix/fetch-http-handler branch May 8, 2023 12:36
@LorhanSohaky
Copy link

When will the fix be made available? @kuhe

@jroru
Copy link

jroru commented May 8, 2023

@LorhanSohaky Looks like it was included in the v3.329.0 release

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants