aws.signature inserts us-east-1 breaking some third party object stores

[brokenjacobs]

Please specify whether your issue is about:

• [* ] a possible bug
• [* ] a suggested code or documentation change, improvement to the code, or feature request

aws.signature always inserts us-east-1 if no region is specified. However there is no way to specify 'no region' in url requests.

Amazon assumes us-east-1 is your region if you don't include it in the path based or vhost based access to the bucket. Some third party object stores depend on this functionality to operate properly. There is no need to add us-east-1 to the vhost or path if no region is specified. Doing so breaks functionality in this library.

[jon-mago]

Hi

Thanks for this bug report. Can you be more specific about a 3rd party object store which breaks as a result of this, I'd like to have something to test against...

[brokenjacobs]

We are using EMC elastic cloud storage, I belive that minio would be affected by this as well. The line in question is here: https://github.com/cloudyr/aws.signature/blob/a8fd855a3fac6e6bebdc709903931d1d5b663cb5/R/locate_credentials.R#L377

Amazon lists valid endpoints for aws services here: https://docs.aws.amazon.com/general/latest/gr/rande.html Note that us-east-1 is not required in the path or vhost specification:

s3.amazonaws.com
s3.us-east-1.amazonaws.com
s3.dualstack.us-east-1.amazonaws.com**
account-id.s3-control.us-east-1.amazonaws.com
account-id.s3-control.dualstack.us-east-1.amazonaws.com**

A simple fix for this issue would seem to be to remove the failsafe region settings. I'm not sure if the aws.s3 package would need to be updated as well.

[brokenjacobs]

would you be open to a pull request to resolve this?

[jon-mago]

Yes, though I think the fix might need to be at the aws.s3 level (which I don't maintain). For example aws.sqs does need the region to construct a correct URL, so it can't just be removed from this package without that package also being changed to ask for the region some other way.

[brokenjacobs]

Understood. In that case I would argue that this logic should be completely removed from aws.signature and taken care of by the respective consumers of the library. The signature package shouldn't care about endpoint URL's at all. Of course this presents a larger problem that more libraries have to be updated than just aws.signature.

[jon-mago]

This package has to care about the region, it is needed for the signature (see signature docs) as well as the endpoint url. Also, it's the package which is looking around the config files. Packages should be careful in how they construct URLs based on the information though.

One thing I have noticed is that the region finding doesn't cope with the region being explicitly set as an empty string (which is possibly the minio default, though other parts of the docs in using the standard AWS SDK suggest it might be us-east-1). That is something which should be resolved

[brokenjacobs]

We are in agreement. I'm not saying that the library should not care about region, just that it shouldn't attempt to add one when an empty region is specified.

[raymondben]

Does it make sense that locate_credentials should use the default region if it has not been specified (i.e. it is NULL) but that it should not do this if the user specified locate_credentials(region = "") (just as @brokenjacobs said). If so, that's an easy fix (I think): replace is_blank with is.null at https://github.com/cloudyr/aws.signature/blob/master/R/locate_credentials.R#L346

[brokenjacobs]

That sounds like a reasonable solution to me. I do wonder, will this break other libraries?

[namelessjon]

Realised my PR reversed the logic originally, and allowed NULL regions. This now allows empty regions.

[brokenjacobs]

any chance of getting this merged?