brendanhay / amazonka

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

amazonka: fixing sigv4 path encoding so that it happens once for s3 and twice for other services #812

Closed mwu closed 2 years ago

mwu commented 2 years ago

Should fix #529.

I did some manual smoke testing:

mwu commented 2 years ago

All right, so once you get your secondary PR merged I think this is good to go. Would you mind firing off your manual smoke-test requests one last time, too, just to make sure the tweaks are all good?

Ok. These all returned success:

mwu commented 2 years ago

Excellent, thank you for being so thorough. One more nit, and one last question: I think I have misunderstood the purpose of the various metaCanonicalFoo in the metadata record - it seems like they don't get used anywhere because they get built and immediately passed to signRequest. I think you pointed this out earlier? When I asked you to remove it, I didn't realise there was an established practice of keeping this data around. My question is - do you think we should reinstate metaCanonicalPath? I think your tests are fine as they are.

I think it's fine as it is, too. The canonical path gets logged already as part of the canonical request and I can't see any other use for returning it in the metadata. Carrying it around is only useful for testing but as you said, we should really approach that by expanding the test suite to test the whole signature.