pplu / aws-sdk-perl

A community AWS SDK for Perl Programmers
Other
171 stars 94 forks source link

S3 DeleteObjects Method missing 'Content-MD5' Header #149

Open SeptamusNonovant opened 7 years ago

SeptamusNonovant commented 7 years ago

Hello,

The "DeleteObjects" method in the S3 class is returning an XML error when run:

`<?xml version="1.0" encoding="UTF-8"?>

InvalidRequest Missing required header for this request: Content-MD5 08F4BD6B01A2DEB5 kkzJ9SIZASgpca11hIVQuay4LHz1izFJJ9Vc2i/zK0GS9d0wl69ZdLfnPRKAqECWtV/uatpC53g= ` It appears this method requires a Content-MD5 header with a 128-bit base 64 MD5 of the data. From the AWS documentation: > Header: Content-MD5 > > Description: The base64-encoded 128-bit MD5 digest of the data. This header must be used as a message integrity check to verify that the request body was not corrupted in transit. For more information, go to RFC 1864. > > Type: String > > Default: None > Required: Yes So it appears this information will need to be added in order to use the "DeleteObjects" method to delete more than one object from a bucket. Thanks.
pplu commented 7 years ago

Hi,

Thanks for your report. As you can see, S3 support is in the works... Would you mind making a small script that given a bucket, creates some objects, and then deletes them (I'm looking that you exercise the API, like in examples/s3-common-methods.pl so I can fix this.

SeptamusNonovant commented 7 years ago

Hello,

Here is the code snippet you asked for. Hopefully this helps!

s3-delete-objects-example.pl.txt

sven-schubert commented 7 years ago

hopefully gets solved by: https://github.com/pplu/aws-sdk-perl/pull/193

pprocacci commented 6 years ago

193 in its current form doesn't fix this. However the following patch against release .35 works:

--- DeleteObjects.pm.orig   2017-10-19 12:55:41.658754457 -0400
+++ DeleteObjects.pm    2017-10-19 12:56:11.478804747 -0400
@@ -3,6 +3,7 @@
   use Moose;
   has Bucket => (is => 'ro', isa => 'Str', uri_name => 'Bucket', traits => ['ParamInURI'], required => 1);
   has Delete => (is => 'ro', isa => 'Paws::S3::Delete', required => 1);
+  has ContentMD5 => (is => 'ro', isa => 'Str', header_name => 'Content-MD5', auto => 'MD5', traits => ['AutoInHeader']);
   has MFA => (is => 'ro', isa => 'Str', header_name => 'x-amz-mfa', traits => ['ParamInHeader']);
   has RequestPayer => (is => 'ro', isa => 'Str', header_name => 'x-amz-request-payer', traits => ['ParamInHeader']);
sven-schubert commented 6 years ago

Hi, you are right. The problem is Paws code is generated from botocore definitions. As the header is missing in botocore, it is missing in Paws too. I opened an issue in botocore: https://github.com/boto/botocore/issues/1302

castaway commented 6 years ago

The botocore issue responder seems to say that they don't allow the user to specify the header.. which is unhelpful .. it looks like they have a manual list of "methods that require Content-MD5", so we either did to create one too, or annoy whomever maintains the json files to update them. See https://github.com/boto/botocore/blob/develop/botocore/handlers.py#L875-L890

pplu commented 6 years ago

@castaway I maintain a fork of the AWS definitions with some shallow changes to the jsons. I think we can safely add the ContentMD5 parameter to whatever methods we want.

On other projects where I'm reading external JSONs, I find myself in this situation too. In these projects I build a small script (something like fix_definitions) that reads in the JSON(s), and then patches them up programatically (that way I can always import the original definitions and know what changes I'm applying to them (even try taking them out from time to time, since upstream can have them patched). Paws wasn't built this way, but there's always a good time to start :)

castaway commented 6 years ago

Aha, I didn't notice your fork, good idea. I think the fork is a nicer way, as then you can shove PRs onto the main repo using it.

castaway commented 6 years ago

We intend to submit patches to the botocore fork to fix missing Content-MD5 / Content_Length issues ( making the new tests in t/s3/content_headers.t pass)

castaway commented 4 years ago

Looking at this older botocore issue, it sounds like they are not likely to take that patch .. I did add another comment to try and figure out why they'd rather have the list in code: https://github.com/boto/botocore/issues/1302

byterock commented 4 years ago

I raised a similar bug https://github.com/boto/botocore/issues/1888 no response and I raised 6 or seven comments on the original AWS docs that they where in-error on not up todate. Doupt we will hear anyting from either of them.

I just added in that ContentMD5 to all S3 calls as that is what Python and java do