ISSUE #240: Fix signing issues for endpoints with a path component

[rafd123]

In order to properly preserve the relationship between IRequest.Endpoint and IRequest.ResourcePath, have, AWS3Signer and AWS4Signer canonicalize the URL returned by AmazonServiceClient.ComposeUrl()

[stevejroberts]

Thanks for the PR. We're running it through our integration test suite and will follow up later.

[rafd123]

Thanks, @steveataws

I have to admit, going into this, I didn't fully understand the intent of IRequest.ResourcePath as it relates to IRequest.Endpoint.

The closer I look at the code, I'm wondering if the signers should be using AmazonServiceClient.ComposeUrl() as a means to resolve the URL that ultimately gets canonicalized.

Do you agree? If so, the PR should be revised.

[stevejroberts]

Hi,

When I ran the integration tests we had a couple of fails in a test which validate some S3 keys we've had issues with in the past. You can find the test class in sdk\test\IntegrationTests\Tests\S3\KeyNameTests.cs called TestKeyNameWithAwkwardChars_AWS4Signing if you want to take a look.

The two keys that triggered fails are:

ObjectWith!and?+forgood:measureThis=And&InKeynÄme

and

ObjectWith\InKeyname

In the first case, the use of Uri.AbsolutePath returns 'ObjectWith!and', terminating on the ? character. In the second case the Uri class is encoding the \ before we get a chance to work with the url in canonicalization and as a result we end up working with a double-encoding (this has been a common problem for use with use of the Uri class, which is why we tend to prefer string manipulation for urls for as long as possible).

Do you want to take another look at the fix in light of these two keys? The rest of the integration test suite ran without any issues.

[rafd123]

Will do!

[rafd123]

I addressed the problematic keys by grabbing the unescaped path from the composed request URL and feeding it to the original logic of "CanonicalizeResourcePath."

I ran the failing integration test using my account and verified that it now passes.

Let me know! :)

[stevejroberts]

OK, will do. Thanks!

[stevejroberts]

Hi,

I've completed running all of the integration tests against the revised update and all seems good. We'll get started on incorporating the changes.

Thanks for the contribution!

[stevejroberts]

PR is merged into core version 3.1.1.1.