Also use AWS S3 subdomain URL when directory name contains a period. #809

Merged
merged 1 commit into from Sep 24, 2012

4 participants

@DouweM

See #285 for info about the difference between AWS subdomain URLs (https://#{bucket_name}.s3.amazonaws.com) and path URLs (https://s3.amazonaws.com/#{bucket_name}).

Currently, the subdomain URL is not being used for buckets with a name containing a period like assets.example.com, while Amazon is perfectly fine with this and while this is actually required for buckets outside of the US Standard zone.

This pull request modifies the regular expression to allow this.

@travisbot

This pull request fails (merged 0b4f79e0 into 2b32dbd).

@thiagofm
CarrierWave member

Hello @DouweM

Can you create the test case for it?

Thanks.

@DouweM

Test case added!

@thiagofm
CarrierWave member

Awesome, can you please squash your commits?

@DouweM

Done.

@thiagofm
CarrierWave member

@DouweM just giving you a update, it looks good. But for some reason travis-ci isn't running new pull requests submissions from our repository, once it does it and we're still green I will merge.

Thanks a lot for your contribution.

@thiagofm thiagofm merged commit 56592f3 into carrierwaveuploader:master Sep 24, 2012
@rymai

Actually, I think this was intentional since it's causing a SSL warning when a subdomain contains period(s).

Hmm. I've been using this for the last 8 months now and haven't had any problems. Note that the old way wasn't working at all, because the subdomain style is required for buckets outside of the US Standard zone. See the PR that introduced this commit: #809.

I see... For now the better trade-off is then to not have bucket names with period I guess.

Unfortunately there's no way around bucket names with periods if you want to have your bucket accessible through a whitelabel URL such as assets.domain.com. In this case, your bucket has to be named "assets.domain.com" and CarrierWave has to connect to assets.domain.com.s3.amazonaws.com, even though this causes issues with SSL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment