brendanhay / amazonka

A comprehensive Amazon Web Services SDK for Haskell.
https://amazonka.brendanhay.nz
Other
599 stars 227 forks source link

Proper formatting for `if-modified-since` timestamps in S3 #879

Closed divarvel closed 1 year ago

divarvel commented 1 year ago

Currently, amazonka sends the if-modified-since header in GetObject (and HeadObject) as a ISO8601 date (see https://amazonka.brendanhay.nz/docs/libZSservicesZSamazonka-s3ZSamazonka-s3/Amazonka-S3-GetObject.html#t:GetObject), but S3 seems to expect a RFC822-formatted date:

I'm not sure how the time format is selected during codegen, so i don't know how to fix it myself. If that's not too complicated, i can work on a PR

endgame commented 1 year ago

That request field is defined in botocore (AWS' repo of JSON files defining every service) here:

https://github.com/boto/botocore/blob/73d72c93034061a99ac687a8dfe7ee0a0ec1bd3f/botocore/data/s3/2006-03-01/service-2.json#L4755-L4760

It says that the format for that shape is just "timestamp"

https://github.com/boto/botocore/blob/73d72c93034061a99ac687a8dfe7ee0a0ec1bd3f/botocore/data/s3/2006-03-01/service-2.json#L5454

The API Reference for S3 (probably generated from botocore) doesn't specify a format, but the example uses RFC822: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html

A timestamp shape can have a "timestampFormat": "rfc822" field:

https://github.com/boto/botocore/blob/73d72c93034061a99ac687a8dfe7ee0a0ec1bd3f/botocore/data/s3/2006-03-01/service-2.json#L9424-L9427

The JSON files in configs/annexes are overlaid on top of the raw JSON from botocore, so you'll want to add the following object inside configs/annexes/s3.json:

"IfModifiedSince": { "timestampFormat": "rfc822" }

Then commit and regenerate service bindings by entering a nix-shell and running scripts/generate --commit s3. (For single-service changes, I do the regeneration in the PR. For treewide changes, I regenerate in a separate PR.)

Can you please also check if there are other fields that should be tagged RFC822? It would be good to fix them, at least on the request side.

divarvel commented 1 year ago

Thanks for the pointers! I opened #881 to try and solve this. Currently it seems the code generator does not use the overriden timestampFormat value in shape definitions.