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

s3.deleteObjects sends CRC32 checksum even if requestChecksumCalculation is set to WHEN_REQUIRED #6819

Closed
4 tasks done
trivikr opened this issue Jan 17, 2025 · 7 comments
Closed
4 tasks done
Labels
guidance General information and guidance, answers to FAQs, or recommended best practices/resources.

Comments

@trivikr
Copy link
Member

trivikr commented Jan 17, 2025

Checkboxes for prior research

Describe the bug

s3.deleteObjects sends CRC32 checksum even if requestChecksumCalculation is set to WHEN_REQUIRED

Regression Issue

  • Select this option if this issue appears to be a regression.

SDK version number

@aws-sdk/[email protected]

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

All

Reproduction Steps

import { S3 } from "@aws-sdk/client-s3";
import { NodeHttpHandler } from "@smithy/node-http-handler";

class CustomHandler extends NodeHttpHandler {
  constructor() {
    super();
  }

  printChecksumHeaders(prefix, headers) {
    for (const [header, value] of Object.entries(headers)) {
      if (
        header === "content-md5" ||
        header.startsWith("x-amz-checksum-") ||
        header.startsWith("x-amz-sdk-checksum-")
      ) {
        console.log(`${prefix}['${header}']: '${value}'`);
      }
    }
  }

  async handle(request, options) {
    this.printChecksumHeaders("request", request.headers);
    const response = await super.handle(request, options);
    this.printChecksumHeaders("response", response.response.headers);
    return response;
  }
}

const client = new S3({
  requestHandler: new CustomHandler(),
  requestChecksumCalculation: "WHEN_REQUIRED",
});

const Bucket = "test-flexible-checksums-v2"; // Update to your test bucket name.

const prepareClient = new S3();
// Populate an object in the bucket, as deleteObjects requires at least one.
await prepareClient.putObject({ Bucket, Key: "helloworld.txt", Body: "helloworld" });

// Get list of objects to delete.
const response = await prepareClient.listObjectsV2({ Bucket });
const Objects = response.Contents.map(({ Key }) => ({ Key }));

await client.deleteObjects({ Bucket, Delete: { Objects } });

Observed Behavior

The CRC32 checksum is sent even if requestChecksumCalculation is set to WHEN_REQUIRED

$ node test.mjs
request['x-amz-sdk-checksum-algorithm']: 'CRC32'
request['x-amz-checksum-crc32']: 'uDrkBQ=='

Expected Behavior

When requestChecksumCalculation is set to WHEN_REQUIRED, should it send md5 like it was done in <=v3.729.0?

Example output in v3.726.0

$ node test.mjs
request['content-md5']: 'rl/UEqX25ygZIHP8TWK6/g=='

Possible Solution

No response

Additional Information/Context

No response

@trivikr trivikr added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 17, 2025
@github-actions github-actions bot added the potential-regression Marking this issue as a potential regression to be checked by team member label Jan 17, 2025
@trivikr trivikr added guidance General information and guidance, answers to FAQs, or recommended best practices/resources. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. potential-regression Marking this issue as a potential regression to be checked by team member labels Jan 17, 2025
@trivikr
Copy link
Member Author

trivikr commented Jan 17, 2025

This is not a regression, as default algorithm was changed to CRC32 as per discussions with S3 team in #6749

Adding a guidance label.

@trivikr
Copy link
Member Author

trivikr commented Jan 17, 2025

What's the issue that you're facing @OIRNOIR?

I verified through the minimal repro provided above that deleteObjects call is succesful.
Is it unsuccessful in your case? If yes, can you provided a minimal repro?

@trivikr trivikr added the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Jan 17, 2025
@OIRNOIR
Copy link

OIRNOIR commented Jan 17, 2025

What's the issue that you're facing @OIRNOIR?

I verified through the minimal repro provided above that deleteObjects call is succesful. Is it unsuccessful in your case? If yes, can you provided a minimal repro?

I'm using Cloudflare R2, and I can reproduce the issue this way:

import fs from "node:fs";
import path from "node:path";
import {
	S3Client,
	PutObjectCommand,
	ListObjectsV2Command,
	DeleteObjectsCommand
} from "@aws-sdk/client-s3";

const __dirname = path.dirname(decodeURIComponent(new URL(import.meta.url).pathname));

const config = JSON.parse(
	fs.readFileSync(path.join(__dirname, "/config.json")),
);

const S3 = new S3Client({
	region: "auto",
	endpoint: `https://${config.r2.accountId}.r2.cloudflarestorage.com`,
	credentials: {
		accessKeyId: config.r2.keyId,
		secretAccessKey: config.r2.secret,
	},
	requestChecksumCalculation: "WHEN_REQUIRED",
	responseChecksumValidation: "WHEN_REQUIRED"
});

// Put dummy object
await S3.send(
	new PutObjectCommand({
		Bucket: "testing",
		Key: `test-${new Date().toISOString()}`,
		Body: "helloworld"
	})
);

const res = await S3.send(
	new ListObjectsV2Command({
		Bucket: "testing"
	})
);
const Objects = res.Contents.map(({Key}) => ({Key}));

console.log("Sending the DeleteObjectsCommand"); // Crashes after this line
await S3.send(
	new DeleteObjectsCommand({
		Bucket: "testing",
		Delete: {
			Objects
		}
	})
);

@OIRNOIR
Copy link

OIRNOIR commented Jan 17, 2025

It crashes with a 501 error: NotImplemented: Header 'x-amz-sdk-checksum-algorithm' with value 'CRC32' not implemented

@OIRNOIR
Copy link

OIRNOIR commented Jan 17, 2025

This is not a regression, as default algorithm was changed to CRC32 as per discussions with S3 team in #6749

Adding a guidance label.

It is a regression because not all third party S3 apis (like Cloudflare R2) support CRC32 and return 501 whenever CRC32 (or any other checksum algorithm) is attempted. Implementing your CustomHandler, the observed code logs the same two headers, even with requestChecksumCalculation set to WHEN_REQUIRED. However, upon downgrading to @aws-sdk/[email protected], the only header listed is request['content-md5']: 'kbedJZRuTVhyWMNpO7amyA==' and the request succeeds.

My guess as to why you can't reproduce the error is that you are not using Cloudflare R2.

@trivikr
Copy link
Member Author

trivikr commented Jan 17, 2025

Closing as AWS SDK for JavaScript is designed to be used with AWS services.
We've provided a minimal repro where deleteObjects call succeeds with Amazon S3.

If you're using a different cloud service, please refer to their documentation on compatibility.

@trivikr trivikr closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2025
Copy link

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

@trivikr trivikr removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
guidance General information and guidance, answers to FAQs, or recommended best practices/resources.
Projects
None yet
Development

No branches or pull requests

2 participants