From c6fb60e0a0c5792d31f7d7258b50f83245f02c41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 16 Dec 2013 11:44:59 +0100
Subject: [PATCH] FIX: S3 upload when using dots in bucket name

---
 app/models/site_setting.rb       |  4 ++++
 lib/cooked_post_processor.rb     |  2 +-
 lib/file_store/local_store.rb    |  2 +-
 lib/file_store/s3_store.rb       | 21 ++++++++++++++++++++-
 spec/models/site_setting_spec.rb | 14 ++++++++++++++
 5 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb
index 80b330f359b..e670d5da725 100644
--- a/app/models/site_setting.rb
+++ b/app/models/site_setting.rb
@@ -84,6 +84,10 @@ class SiteSetting < ActiveRecord::Base
     authorized_images.count > 0 && file.original_filename =~ /\.(#{authorized_images.join("|")})$/i
   end
 
+  def self.scheme
+    use_ssl? ? "https" : "http"
+  end
+
 end
 
 # == Schema Information
diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index 77239b3fec7..a511cc46104 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -98,7 +98,7 @@ class CookedPostProcessor
     absolute_url = url
     absolute_url = Discourse.base_url_no_prefix + absolute_url if absolute_url =~ /^\/[^\/]/
     # FastImage fails when there's no scheme
-    absolute_url = (SiteSetting.use_ssl? ? "https:" : "http:") + absolute_url if absolute_url.start_with?("//")
+    absolute_url = SiteSetting.scheme + ":" + absolute_url if absolute_url.start_with?("//")
     return unless is_valid_image_url?(absolute_url)
     # we can *always* crawl our own images
     return unless SiteSetting.crawl_images? || Discourse.store.has_been_uploaded?(url)
diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb
index 22c4542253f..de18cdacaee 100644
--- a/lib/file_store/local_store.rb
+++ b/lib/file_store/local_store.rb
@@ -128,7 +128,7 @@ module FileStore
     end
 
     def is_local?(url)
-      absolute_url = url.start_with?("//") ? (SiteSetting.use_ssl? ? "https:" : "http:") + url : url
+      absolute_url = url.start_with?("//") ? SiteSetting.scheme + ":" + url : url
       absolute_url.start_with?(absolute_base_url) || absolute_url.start_with?(absolute_base_cdn_url)
     end
 
diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb
index 9f5d714fa2e..8b16137b032 100644
--- a/lib/file_store/s3_store.rb
+++ b/lib/file_store/s3_store.rb
@@ -49,7 +49,7 @@ module FileStore
 
       extension = File.extname(upload.original_filename)
       temp_file = Tempfile.new(["discourse-s3", extension])
-      url = (SiteSetting.use_ssl? ? "https:" : "http:") + upload.url
+      url = SiteSetting.scheme + ":" + upload.url
 
       File.open(temp_file.path, "wb") do |f|
         f.write(open(url, "rb", read_timeout: 5).read)
@@ -108,6 +108,9 @@ module FileStore
         provider: 'AWS',
         aws_access_key_id: SiteSetting.s3_access_key_id,
         aws_secret_access_key: SiteSetting.s3_secret_access_key,
+        scheme: SiteSetting.scheme,
+        # cf. https://github.com/fog/fog/issues/2381
+        path_style: dns_compatible?(s3_bucket, SiteSetting.use_ssl?),
       }
       options[:region] = SiteSetting.s3_region unless SiteSetting.s3_region.empty?
       options
@@ -164,6 +167,22 @@ module FileStore
       "tombstone/"
     end
 
+    # cf. https://github.com/aws/aws-sdk-core-ruby/blob/master/lib/aws/plugins/s3_bucket_dns.rb#L56-L78
+    def dns_compatible?(bucket_name, ssl)
+      if valid_subdomain?(bucket_name)
+        bucket_name.match(/\./) && ssl ? false : true
+      else
+        false
+      end
+    end
+
+    def valid_subdomain?(bucket_name)
+      bucket_name.size < 64 &&
+      bucket_name =~ /^[a-z0-9][a-z0-9.-]+[a-z0-9]$/ &&
+      bucket_name !~ /(\d+\.){3}\d+/ &&
+      bucket_name !~ /[.-]{2}/
+    end
+
   end
 
 end
diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb
index a170b715aa2..d5bd6efd5d2 100644
--- a/spec/models/site_setting_spec.rb
+++ b/spec/models/site_setting_spec.rb
@@ -110,4 +110,18 @@ describe SiteSetting do
 
   end
 
+  describe "scheme" do
+
+    it "returns http when ssl is disabled" do
+      SiteSetting.expects(:use_ssl).returns(false)
+      SiteSetting.scheme.should == "http"
+    end
+
+    it "returns https when using ssl" do
+      SiteSetting.expects(:use_ssl).returns(true)
+      SiteSetting.scheme.should == "https"
+    end
+
+  end
+
 end