AWS | storage: big refactor

Logics have been centralised:

* region to hostname
* url generation
* signature
* chaning scheme also changes the port

During the process a couple of inconsistencies have also
been fixed.

Known limitations:

When using the @endpoint with a custom port you need to specify the port
when using get_object_http_url or get_object_https_url.

When using bucket names that contain dots outside of us-east-1 make sure to
access it with the same region in your AWS::Storage.

lib/fog/aws.rb

@@ -3,8 +3,6 @@ require 'fog/aws/credential_fetcher'
 require 'fog/aws/signaturev4'
 module Fog
   module AWS
-    COMPLIANT_BUCKET_NAMES = /^(?:[a-z]|\d(?!\d{0,2}(?:\.\d{1,3}){3}$))(?:[a-z0-9]|\-(?![\.])){1,61}[a-z0-9]$/
-
     extend Fog::Provider
 
     service(:auto_scaling,    'aws/auto_scaling',     'AutoScaling')

lib/fog/aws/models/storage/directory.rb

@@ -80,11 +80,9 @@ module Fog
         def public_url
           requires :key
           if service.get_bucket_acl(key).body['AccessControlList'].detect {|grant| grant['Grantee']['URI'] == 'http://acs.amazonaws.com/groups/global/AllUsers' && grant['Permission'] == 'READ'}
-            if key.to_s =~ Fog::AWS::COMPLIANT_BUCKET_NAMES
+            service.request_url(
-              "https://#{key}.s3.amazonaws.com"
+              :bucket_name => key
-            else
+            )
-              "https://s3.amazonaws.com/#{key}"
-            end
           else
             nil
           end

lib/fog/aws/models/storage/file.rb

@@ -163,11 +163,10 @@ module Fog
         def public_url
           requires :directory, :key
           if service.get_object_acl(directory.key, key).body['AccessControlList'].detect {|grant| grant['Grantee']['URI'] == 'http://acs.amazonaws.com/groups/global/AllUsers' && grant['Permission'] == 'READ'}
-            if directory.key.to_s =~ Fog::AWS::COMPLIANT_BUCKET_NAMES
+            service.request_url(
-              "https://#{directory.key}.s3.amazonaws.com/#{Fog::AWS.escape(key)}".gsub('%2F','/')
+              :bucket_name => directory.key,
-            else
+              :object_name => key
-              "https://s3.amazonaws.com/#{directory.key}/#{Fog::AWS.escape(key)}".gsub('%2F','/')
+            )
-            end
           else
             nil
           end

lib/fog/aws/requests/storage/abort_multipart_upload.rb

@@ -15,9 +15,9 @@ module Fog
           request({
             :expects    => 204,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :method     => 'DELETE',
-            :path       => CGI.escape(object_name),
             :query      => {'uploadId' => upload_id}
           })
         end
@@ -25,4 +25,4 @@ module Fog
       end # Real
     end # Storage
   end # AWS
-end # Fog
+end # Fog

lib/fog/aws/requests/storage/complete_multipart_upload.rb

@@ -34,10 +34,10 @@ module Fog
             :body       => data,
             :expects    => 200,
             :headers    => { 'Content-Length' => data.length },
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :method     => 'POST',
             :parser     => Fog::Parsers::Storage::AWS::CompleteMultipartUpload.new,
-            :path       => CGI.escape(object_name),
             :query      => {'uploadId' => upload_id}
           })
         end

lib/fog/aws/requests/storage/copy_object.rb

@@ -33,10 +33,10 @@ module Fog
           request({
             :expects  => 200,
             :headers  => headers,
-            :host     => "#{target_bucket_name}.#{@host}",
+            :bucket_name => target_bucket_name,
+            :object_name => target_object_name,
             :method   => 'PUT',
             :parser   => Fog::Parsers::Storage::AWS::CopyObject.new,
-            :path     => CGI.escape(target_object_name)
           })
         end
 

lib/fog/aws/requests/storage/delete_bucket.rb

@@ -16,7 +16,7 @@ module Fog
           request({
             :expects  => 204,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'DELETE'
           })
         end

lib/fog/aws/requests/storage/delete_bucket_cors.rb

@@ -16,7 +16,7 @@ module Fog
           request({
             :expects  => 204,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'DELETE',
             :query    => {'cors' => nil}
           })

lib/fog/aws/requests/storage/delete_bucket_lifecycle.rb

@@ -16,7 +16,7 @@ module Fog
           request({
                     :expects  => 204,
                     :headers  => {},
-                    :host     => "#{bucket_name}.#{@host}",
+                    :bucket_name => bucket_name,
                     :method   => 'DELETE',
                     :query    => {'lifecycle' => nil}
                   })

lib/fog/aws/requests/storage/delete_bucket_policy.rb

@@ -16,7 +16,7 @@ module Fog
           request({
             :expects  => 204,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'DELETE',
             :query    => {'policy' => nil}
           })

lib/fog/aws/requests/storage/delete_bucket_website.rb

@@ -16,7 +16,7 @@ module Fog
           request({
             :expects  => 204,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'DELETE',
             :query    => {'website' => nil}
           })

lib/fog/aws/requests/storage/delete_multiple_objects.rb

@@ -55,7 +55,7 @@ module Fog
             :body       => data,
             :expects    => 200,
             :headers    => headers,
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method     => 'POST',
             :parser     => Fog::Parsers::Storage::AWS::DeleteMultipleObjects.new,
             :query      => {'delete' => nil}

lib/fog/aws/requests/storage/delete_object.rb

@@ -24,7 +24,7 @@ module Fog
           request({
             :expects    => 204,
             :headers    => headers,
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method     => 'DELETE',
             :path       => path

lib/fog/aws/requests/storage/get_bucket.rb

@@ -44,7 +44,7 @@ module Fog
           request({
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method   => 'GET',
             :parser   => Fog::Parsers::Storage::AWS::GetBucket.new,

lib/fog/aws/requests/storage/get_bucket_acl.rb

@@ -33,7 +33,7 @@ module Fog
           request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method     => 'GET',
             :parser     => Fog::Parsers::Storage::AWS::AccessControlList.new,

lib/fog/aws/requests/storage/get_bucket_cors.rb

@@ -29,7 +29,7 @@ module Fog
           request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method     => 'GET',
             :parser     => Fog::Parsers::Storage::AWS::CorsConfiguration.new,

lib/fog/aws/requests/storage/get_bucket_lifecycle.rb

@@ -23,7 +23,7 @@ module Fog
           request({
                     :expects  => 200,
                     :headers  => {},
-                    :host     => "#{bucket_name}.#{@host}",
+                    :bucket_name => bucket_name,
                     :idempotent => true,
                     :method   => 'GET',
                     :parser   => Fog::Parsers::Storage::AWS::GetBucketLifecycle.new,

lib/fog/aws/requests/storage/get_bucket_location.rb

@@ -19,7 +19,7 @@ module Fog
           request({
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method   => 'GET',
             :parser   => Fog::Parsers::Storage::AWS::GetBucketLocation.new,

lib/fog/aws/requests/storage/get_bucket_logging.rb

@@ -33,7 +33,7 @@ module Fog
           request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method     => 'GET',
             :parser     => Fog::Parsers::Storage::AWS::GetBucketLogging.new,

lib/fog/aws/requests/storage/get_bucket_object_versions.rb

@@ -55,7 +55,7 @@ module Fog
           request({
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method   => 'GET',
             :parser   => Fog::Parsers::Storage::AWS::GetBucketObjectVersions.new,

lib/fog/aws/requests/storage/get_bucket_policy.rb

@@ -19,7 +19,7 @@ module Fog
           response = request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method     => 'GET',
             :query      => {'policy' => nil}

lib/fog/aws/requests/storage/get_bucket_versioning.rb

@@ -23,7 +23,7 @@ module Fog
           request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method     => 'GET',
             :parser     => Fog::Parsers::Storage::AWS::GetBucketVersioning.new,

lib/fog/aws/requests/storage/get_bucket_website.rb

@@ -26,7 +26,7 @@ module Fog
           request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method     => 'GET',
             :parser     => Fog::Parsers::Storage::AWS::GetBucketWebsite.new,

lib/fog/aws/requests/storage/get_object.rb

@@ -55,10 +55,10 @@ module Fog
 
           request(params.merge!({
             :expects  => [ 200, 206 ],
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :idempotent => true,
             :method   => 'GET',
-            :path     => CGI.escape(object_name)
           }))
         end
 

lib/fog/aws/requests/storage/get_object_acl.rb

@@ -43,11 +43,11 @@ module Fog
           request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :idempotent => true,
             :method     => 'GET',
             :parser     => Fog::Parsers::Storage::AWS::AccessControlList.new,
-            :path       => CGI.escape(object_name),
             :query      => query
           })
         end

lib/fog/aws/requests/storage/get_object_http_url.rb

@@ -5,25 +5,7 @@ module Fog
       module GetObjectHttpUrl
 
         def get_object_http_url(bucket_name, object_name, expires, options = {})
-          unless bucket_name
+          get_object_url(bucket_name, object_name, expires, options.merge(:scheme => 'http'))
-            raise ArgumentError.new('bucket_name is required')
-          end
-          unless object_name
-            raise ArgumentError.new('object_name is required')
-          end
-          host, path = if bucket_name =~ /^(?:[a-z]|\d(?!\d{0,2}(?:\.\d{1,3}){3}$))(?:[a-z0-9]|\.(?![\.\-])|\-(?![\.])){1,61}[a-z0-9]$/
-            ["#{bucket_name}.#{@host}", object_name]
-          else
-            [@host, "#{bucket_name}/#{object_name}"]
-          end
-          http_url({
-            :headers  => {},
-            :host     => host,
-            :port     => @port,
-            :method   => 'GET',
-            :path     => path,
-            :query    => options[:query]
-          }, expires)
         end
 
       end

lib/fog/aws/requests/storage/get_object_torrent.rb

@@ -33,10 +33,10 @@ module Fog
           request({
             :expects    => 200,
             :headers    => {},
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :idempotent => true,
             :method     => 'GET',
-            :path       => CGI.escape(object_name),
             :query      => {'torrent' => nil}
           })
         end

lib/fog/aws/requests/storage/get_object_url.rb

@@ -11,20 +11,11 @@ module Fog
           unless object_name
             raise ArgumentError.new('object_name is required')
           end
-          host, path = if bucket_name =~ Fog::AWS::COMPLIANT_BUCKET_NAMES
+          signed_url(options.merge({
-            ["#{bucket_name}.#{@host}", object_name]
+            :bucket_name => bucket_name,
-          else
+            :object_name => object_name,
-            [@host, "#{bucket_name}/#{object_name}"]
+            :method => 'GET'
-          end
+          }), expires)
-          scheme_host_path_query({
-            :scheme   => options[:scheme],
-            :headers  => {},
-            :host     => host,
-            :port     => @port,
-            :method   => 'GET',
-            :path     => path,
-            :query    => options[:query]
-          }, expires)
         end
       end
 

lib/fog/aws/requests/storage/get_request_payment.rb

@@ -19,7 +19,7 @@ module Fog
           request({
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method   => 'GET',
             :parser   => Fog::Parsers::Storage::AWS::GetRequestPayment.new,

lib/fog/aws/requests/storage/head_object.rb

@@ -42,10 +42,10 @@ module Fog
           request({
             :expects    => 200,
             :headers    => headers,
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :idempotent => true,
             :method     => 'HEAD',
-            :path       => CGI.escape(object_name),
             :query      => query
           })
         end

lib/fog/aws/requests/storage/initiate_multipart_upload.rb

@@ -30,10 +30,10 @@ module Fog
           request({
             :expects    => 200,
             :headers    => options,
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :method     => 'POST',
             :parser     => Fog::Parsers::Storage::AWS::InitiateMultipartUpload.new,
-            :path       => CGI.escape(object_name),
             :query      => {'uploads' => nil}
           })
         end

lib/fog/aws/requests/storage/list_multipart_uploads.rb

@@ -41,7 +41,7 @@ module Fog
           request({
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :idempotent => true,
             :method   => 'GET',
             :parser   => Fog::Parsers::Storage::AWS::ListMultipartUploads.new,

lib/fog/aws/requests/storage/list_parts.rb

@@ -40,11 +40,11 @@ module Fog
           request({
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :idempotent => true,
             :method   => 'GET',
             :parser   => Fog::Parsers::Storage::AWS::ListParts.new,
-            :path     => CGI.escape(object_name),
             :query    => options.merge!({'uploadId' => upload_id})
           })
         end

lib/fog/aws/requests/storage/put_bucket.rb

@@ -31,7 +31,7 @@ DATA
             :body       => data,
             :headers    => options,
             :idempotent => true,
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method     => 'PUT'
           })
         end

lib/fog/aws/requests/storage/put_bucket_acl.rb

@@ -46,7 +46,7 @@ module Fog
             :body     => data,
             :expects  => 200,
             :headers  => headers,
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'PUT',
             :query    => {'acl' => nil}
           })

lib/fog/aws/requests/storage/put_bucket_cors.rb

@@ -31,7 +31,7 @@ module Fog
             :body     => data,
             :expects  => 200,
             :headers  => headers,
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'PUT',
             :query    => {'cors' => nil}
           })

lib/fog/aws/requests/storage/put_bucket_lifecycle.rb

@@ -66,7 +66,7 @@ module Fog
                     :expects  => 200,
                     :headers  => {'Content-MD5' => Base64.encode64(Digest::MD5.digest(body)).chomp!,
                       'Content-Type' => 'application/xml'},
-                    :host     => "#{bucket_name}.#{@host}",
+                    :bucket_name => bucket_name,
                     :method   => 'PUT',
                     :query    => {'lifecycle' => nil}
                   })

lib/fog/aws/requests/storage/put_bucket_logging.rb

@@ -69,7 +69,7 @@ DATA
             :body     => data,
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'PUT',
             :query    => {'logging' => nil}
           })

lib/fog/aws/requests/storage/put_bucket_policy.rb

@@ -15,7 +15,7 @@ module Fog
             :body     => Fog::JSON.encode(policy),
             :expects  => 204,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'PUT',
             :query    => {'policy' => nil}
           })

lib/fog/aws/requests/storage/put_bucket_versioning.rb

@@ -22,7 +22,7 @@ DATA
             :body     => data,
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'PUT',
             :query    => {'versioning' => nil}
           })

lib/fog/aws/requests/storage/put_bucket_website.rb

@@ -35,7 +35,7 @@ DATA
             :body     => data,
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'PUT',
             :query    => {'website' => nil}
           })

lib/fog/aws/requests/storage/put_object.rb

@@ -33,10 +33,10 @@ module Fog
             :body       => data[:body],
             :expects    => 200,
             :headers    => headers,
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :idempotent => true,
             :method     => 'PUT',
-            :path       => CGI.escape(object_name)
           })
         end
 

lib/fog/aws/requests/storage/put_object_acl.rb

@@ -54,9 +54,9 @@ module Fog
             :body     => data,
             :expects  => 200,
             :headers  => headers,
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :method   => 'PUT',
-            :path       => CGI.escape(object_name),
             :query    => query
           })
         end

lib/fog/aws/requests/storage/put_object_url.rb

@@ -10,14 +10,12 @@ module Fog
           unless object_name
             raise ArgumentError.new('object_name is required')
           end
-          scheme_host_path_query({
+          signed_url(options.merge({
-            :scheme   => options[:scheme],
+            :bucket_name => bucket_name,
-            :headers  => headers,
+            :object_name => object_name,
-            :host     => @host,
-            :port     => @port,
             :method   => 'PUT',
-            :path     => "#{bucket_name}/#{object_name}"
+            :headers  => headers,
-          }, expires)
+          }), expires)
         end
       end
 

lib/fog/aws/requests/storage/put_request_payment.rb

@@ -21,7 +21,7 @@ DATA
             :body     => data,
             :expects  => 200,
             :headers  => {},
-            :host     => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
             :method   => 'PUT',
             :query    => {'requestPayment' => nil}
           })

lib/fog/aws/requests/storage/upload_part.rb

@@ -28,9 +28,9 @@ module Fog
             :expects    => 200,
             :idempotent => true,
             :headers    => headers,
-            :host       => "#{bucket_name}.#{@host}",
+            :bucket_name => bucket_name,
+            :object_name => object_name,
             :method     => 'PUT',
-            :path       => CGI.escape(object_name),
             :query      => {'uploadId' => upload_id, 'partNumber' => part_number}
           })
         end

lib/fog/aws/storage.rb

@@ -6,8 +6,42 @@ module Fog
     class AWS < Fog::Service
       extend Fog::AWS::CredentialFetcher::ServiceMethods
 
+      COMPLIANT_BUCKET_NAMES = /^(?:[a-z]|\d(?!\d{0,2}(?:\.\d{1,3}){3}$))(?:[a-z0-9]|\.(?![\.\-])|\-(?![\.])){1,61}[a-z0-9]$/
+
       DEFAULT_REGION = 'us-east-1'
 
+      DEFAULT_SCHEME = 'https'
+      DEFAULT_SCHEME_PORT = {
+        'http' => 80,
+        'https' => 443
+      }
+
+      VALID_QUERY_KEYS = %w[
+        acl
+        cors
+        delete
+        lifecycle
+        location
+        logging
+        notification
+        partNumber
+        policy
+        requestPayment
+        response-cache-control
+        response-content-disposition
+        response-content-encoding
+        response-content-language
+        response-content-type
+        response-expires
+        torrent
+        uploadId
+        uploads
+        versionId
+        versioning
+        versions
+        website
+      ]
+
       requires :aws_access_key_id, :aws_secret_access_key
       recognizes :endpoint, :region, :host, :path, :port, :scheme, :persistent, :use_iam_profile, :aws_session_token, :aws_credentials_expire_at, :path_style
 
@@ -81,11 +115,11 @@ module Fog
         end
 
         def http_url(params, expires)
-          scheme_host_path_query(params.merge(:scheme => 'http', :port => 80), expires)
+          signed_url(params.merge(:scheme => 'http'), expires)
         end
 
         def https_url(params, expires)
-          scheme_host_path_query(params.merge(:scheme => 'https', :port => 443), expires)
+          signed_url(params.merge(:scheme => 'https'), expires)
         end
 
         def url(params, expires)
@@ -93,32 +127,120 @@ module Fog
           https_url(params, expires)
         end
 
+        def request_url(params)
+          params = request_params(params)
+          params_to_url(params)
+        end
+
+        def signed_url(params, expires)
+          expires = expires.to_i
+          signature = signature(params, expires)
+          params = request_params(params)
+
+          params[:query] = (params[:query] || {}).merge({
+            'AWSAccessKeyId' => @aws_access_key_id,
+            'Signature' => signature,
+            'Expires' => expires,
+          })
+          params[:query]['x-amz-security-token'] = @aws_session_token if @aws_session_token
+
+          params_to_url(params)
+        end
+
         private
 
-        def scheme_host_path_query(params, expires)
+        def region_to_host(region=nil)
-          params[:scheme] ||= @scheme
+          case region.to_s
-          if params[:port] == 80 && params[:scheme] == 'http'
+          when DEFAULT_REGION, ''
-            params.delete(:port)
+            's3.amazonaws.com'
+          else
+            "s3-#{region}.amazonaws.com"
           end
-          if params[:port] == 443 && params[:scheme] == 'https'
+        end
-            params.delete(:port)
+
+        def object_to_path(object_name=nil)
+          '/' + Fog::AWS.escape(object_name.to_s).gsub('%2F','/')
+        end
+
+        # Transforms things like bucket_name, object_name, region
+        #
+        # Should yield the same result when called f*f
+        def request_params(params)
+          headers  = params[:headers] || {}
+
+          if params[:scheme]
+            scheme = params[:scheme]
+            port   = params[:port]
+          else
+            scheme = @scheme || DEFAULT_SCHEME
+            port   = @port
           end
-          params[:headers] ||= {}
+          if DEFAULT_SCHEME_PORT[scheme] == port
-          params[:headers]['Date'] = expires.to_i
+            port = nil
-          params[:path] = Fog::AWS.escape(params[:path]).gsub('%2F', '/')
+          end
-          query = []
+
-          params[:headers]['x-amz-security-token'] = @aws_session_token if @aws_session_token
+          if params[:region]
-          if params[:query]
+            region = params[:region]
-            for key, value in params[:query]
+            host   = params[:host] || region_to_host(region)
-              query << "#{key}=#{Fog::AWS.escape(value)}"
+          else
+            region = @region       || DEFAULT_REGION
+            host   = params[:host] || @host || region_to_host(region)
+          end
+
+          path     = params[:path] || object_to_path(params[:object_name])
+          path     = '/' + path if path[0..0] != '/'
+
+          if params[:bucket_name]
+            bucket_name = params[:bucket_name]
+
+            path_style = params.fetch(:path_style, @path_style)
+            if !path_style && COMPLIANT_BUCKET_NAMES !~ bucket_name
+              Fog::Logger.warning("fog: the specified s3 bucket name(#{bucket_name}) is not a valid dns name, which will negatively impact performance.  For details see: http://docs.amazonwebservices.com/AmazonS3/latest/dev/BucketRestrictions.html")
+              path_style = true
+            elsif bucket_name.include?('.')
+              Fog::Logger.warning("fog: the specified s3 bucket name(#{bucket_name}) might fail with https.")
+            end
+
+            if path_style
+              path = "/#{bucket_name}#{path}"
+            else
+              host = [bucket_name, host].join('.')
             end
           end
-          query << "AWSAccessKeyId=#{@aws_access_key_id}"
+
-          query << "Signature=#{Fog::AWS.escape(signature(params))}"
+          ret = params.merge({
-          query << "Expires=#{params[:headers]['Date']}"
+            :scheme       => scheme,
-          query << "x-amz-security-token=#{Fog::AWS.escape(@aws_session_token)}" if @aws_session_token
+            :host         => host,
-          port_part = params[:port] && ":#{params[:port]}"
+            :port         => port,
-          "#{params[:scheme]}://#{params[:host]}#{port_part}/#{params[:path]}?#{query.join('&')}"
+            :path         => path,
+            :headers      => headers,
+          })
+          
+          #
+          ret.delete(:path_style)
+          ret.delete(:bucket_name)
+          ret.delete(:object_name)
+          ret.delete(:region)
+
+          ret
+        end
+
+        def params_to_url(params)
+          query = params[:query] && params[:query].map do |key, value|
+            if value
+              [key, Fog::AWS.escape(value.to_s)].join('=')
+            else
+              key
+            end
+          end.join('&')
+
+          URI::Generic.build({
+            :scheme => params[:scheme],
+            :host   => params[:host],
+            :port   => params[:port],
+            :path   => params[:path],
+            :query  => query,
+          }).to_s
         end
 
       end
@@ -212,15 +334,9 @@ module Fog
           require 'mime/types'
           @use_iam_profile = options[:use_iam_profile]
           setup_credentials(options)
-          options[:region] ||= 'us-east-1'
+          @region = options[:region] || DEFAULT_REGION
-          @host = options[:host] || case options[:region]
+          @host   = options[:host]   || region_to_host(@region)
-          when 'us-east-1'
+          @scheme = options[:scheme] || DEFAULT_SCHEME
-            's3.amazonaws.com'
-          else
-            "s3-#{options[:region]}.amazonaws.com"
-          end
-          @scheme = options[:scheme] || 'https'
-          @region = options[:region]
         end
 
         def data
@@ -231,7 +347,7 @@ module Fog
           self.class.data[@region].delete(@aws_access_key_id)
         end
 
-        def signature(params)
+        def signature(params, expires)
           "foo"
         end
 
@@ -272,6 +388,7 @@ module Fog
           @use_iam_profile = options[:use_iam_profile]
           setup_credentials(options)
           @connection_options     = options[:connection_options] || {}
+          @persistent = options.fetch(:persistent, false)
 
           if @endpoint = options[:endpoint]
             endpoint = URI.parse(@endpoint)
@@ -281,23 +398,17 @@ module Fog
             else
               endpoint.path
             end
-            @port = endpoint.port
             @scheme = endpoint.scheme
+            @port = endpoint.port
           else
-            options[:region] ||= 'us-east-1'
+            @region     = options[:region]      || DEFAULT_REGION
-            @region = options[:region]
+            @host       = options[:host]        || region_to_host(@region)
-            @host = options[:host] || case options[:region]
-            when 'us-east-1'
-              's3.amazonaws.com'
-            else
-              "s3-#{options[:region]}.amazonaws.com"
-            end
             @path       = options[:path]        || '/'
-            @persistent = options.fetch(:persistent, false)
+            @scheme     = options[:scheme]      || DEFAULT_SCHEME
-            @port       = options[:port]        || 443
+            @port       = options[:port]        || DEFAULT_SCHEME_PORT[@scheme]
-            @scheme     = options[:scheme]      || 'https'
             @path_style = options[:path_style]  || false
           end
+
           @connection = Fog::Connection.new("#{@scheme}://#{@host}:#{@port}#{@path}", @persistent, @connection_options)
         end
 
@@ -305,17 +416,19 @@ module Fog
           @connection.reset
         end
 
-        def signature(params)
+        def signature(params, expires)
+          headers = params[:headers] || {}
+
           string_to_sign =
 <<-DATA
 #{params[:method].to_s.upcase}
-#{params[:headers]['Content-MD5']}
+#{headers['Content-MD5']}
-#{params[:headers]['Content-Type']}
+#{headers['Content-Type']}
-#{params[:headers]['Date']}
+#{expires}
 DATA
 
           amz_headers, canonical_amz_headers = {}, ''
-          for key, value in params[:headers]
+          for key, value in headers
             if key[0..5] == 'x-amz-'
               amz_headers[key] = value
             end
@@ -326,57 +439,33 @@ DATA
           end
           string_to_sign << canonical_amz_headers
 
-          subdomain = params[:host].split(".#{@host}").first
+
-          valid_dns = !!(subdomain =~ /^(?:[a-z]|\d(?!\d{0,2}(?:\.\d{1,3}){3}$))(?:[a-z0-9]|\.(?![\.\-])|\-(?![\.])){1,61}[a-z0-9]$/)
+          query_string = ''
-          if !valid_dns || @path_style
+          if params[:query]
-            Fog::Logger.warning("fog: the specified s3 bucket name(#{subdomain}) is not a valid dns name, which will negatively impact performance.  For details see: http://docs.amazonwebservices.com/AmazonS3/latest/dev/BucketRestrictions.html") unless valid_dns
+            query_args = []
-            params[:host] = params[:host].split("#{subdomain}.")[-1]
+            for key in params[:query].keys.sort
-            unless subdomain == @host
+              if VALID_QUERY_KEYS.include?(key)
-              if params[:path]
+                value = params[:query][key]
-                params[:path] = "#{subdomain}/#{params[:path]}"
+                if value
-              else
+                  query_args << "#{key}=#{Fog::AWS.escape(value.to_s)}"
-                params[:path] = subdomain
+                else
+                  query_args << key
+                end
               end
             end
-            subdomain = nil
+            if query_args.any?
-          end
+              query_string = '?' + query_args.join('&')
-
-          canonical_resource  = @path.dup
-          unless subdomain.nil? || subdomain == @host
-            canonical_resource << "#{Fog::AWS.escape(subdomain).downcase}/"
-          end
-          canonical_resource << params[:path].to_s
-          canonical_resource << '?'
-          for key in (params[:query] || {}).keys.sort
-            if %w{
-              acl
-              cors
-              delete
-              lifecycle
-              location
-              logging
-              notification
-              partNumber
-              policy
-              requestPayment
-              response-cache-control
-              response-content-disposition
-              response-content-encoding
-              response-content-language
-              response-content-type
-              response-expires
-              torrent
-              uploadId
-              uploads
-              versionId
-              versioning
-              versions
-              website
-            }.include?(key)
-              canonical_resource << "#{key}#{"=#{params[:query][key]}" unless params[:query][key].nil?}&"
             end
           end
-          canonical_resource.chop!
+
+          canonical_path = (params[:path] || object_to_path(params[:object_name])).to_s
+          canonical_path = '/' + canonical_path if canonical_path[0..0] != '/'
+          if params[:bucket_name]
+            canonical_resource = "/#{params[:bucket_name]}#{canonical_path}"
+          else
+            canonical_resource = canonical_path
+          end
+          canonical_resource << query_string
           string_to_sign << canonical_resource
 
           signed_string = @hmac.sign(string_to_sign)
@@ -397,16 +486,23 @@ DATA
         def request(params, &block)
           refresh_credentials_if_expired
 
-          params[:headers]['Date'] = Fog::Time.now.to_date_header
+          expires = Fog::Time.now.to_date_header
+          signature = signature(params, expires)
+          
+          params = request_params(params)
+          params.delete(:port) unless params[:port]
+
+          params[:headers]['Date'] = expires
           params[:headers]['x-amz-security-token'] = @aws_session_token if @aws_session_token
-          params[:headers]['Authorization'] = "AWS #{@aws_access_key_id}:#{signature(params)}"
+          params[:headers]['Authorization'] = "AWS #{@aws_access_key_id}:#{signature}"
           # FIXME: ToHashParser should make this not needed
           original_params = params.dup
 
           begin
             response = @connection.request(params, &block)
           rescue Excon::Errors::TemporaryRedirect => error
-            uri = URI.parse(error.response.is_a?(Hash) ? error.response[:headers]['Location'] : error.response.headers['Location'])
+            headers = (error.response.is_a?(Hash) ? error.response[:headers] : error.response.headers)
+            uri = URI.parse(headers['Location'])
             Fog::Logger.warning("fog: followed redirect to #{uri.host}, connecting to the matching region will be more performant")
             response = Fog::Connection.new("#{@scheme}://#{uri.host}:#{@port}", false, @connection_options).request(original_params, &block)
           end

tests/aws/models/storage/directory_tests.rb

@@ -13,7 +13,7 @@ Shindo.tests("Storage[:aws] | directory", ["aws"]) do
     @instance.save
 
     tests("#public_url").returns(true) do
-      if @instance.public_url =~ %r[\Ahttps://fogdirectorytests-[\da-f]+\.s3\.amazonaws\.com\z]
+      if @instance.public_url =~ %r[\Ahttps://fogdirectorytests-[\da-f]+\.s3\.amazonaws\.com/\z]
         true
       else
         @instance.public_url

tests/aws/models/storage/url_tests.rb

@@ -14,15 +14,17 @@ Shindo.tests('AWS | url', ["aws"]) do
   @file = @storage.directories.new(:key => 'fognonbucket').files.new(:key => 'test.txt')
 
   if Fog.mock?
-    signature = Fog::Storage::AWS.new.signature(nil)
+    signature = Fog::Storage::AWS.new.signature(nil, nil)
   else
     signature = 'tajHIhKHAdFYsigmzybCpaq8N0Q%3D'
   end
 
-  tests('#url w/ response-cache-control').returns(
+  if RUBY_VERSION > '1.8.7' # ruby 1.8.x doesn't provide hash ordering
-    "https://fognonbucket.s3.amazonaws.com/test.txt?response-cache-control=No-cache&AWSAccessKeyId=123&Signature=#{signature}&Expires=1356998400"
+    tests('#url w/ response-cache-control').returns(
-  ) do
+      "https://fognonbucket.s3.amazonaws.com/test.txt?response-cache-control=No-cache&AWSAccessKeyId=123&Signature=#{signature}&Expires=1356998400"
-    @file.url(@expires, :query => { 'response-cache-control' => 'No-cache' })
+    ) do
+      @file.url(@expires, :query => { 'response-cache-control' => 'No-cache' })
+    end
   end
 
 end