Add support for non-ascii URIs

Resolves #196

http.gemspec

@@ -23,7 +23,8 @@ Gem::Specification.new do |gem|
   gem.version       = HTTP::VERSION
 
   gem.add_runtime_dependency "http_parser.rb", "~> 0.6.0"
-  gem.add_runtime_dependency "http-form_data", "~> 1.0.0"
+  gem.add_runtime_dependency "http-form_data", "~> 1.0.1"
+  gem.add_runtime_dependency "addressable",    "~> 2.3"
 
   gem.add_development_dependency "bundler", "~> 1.0"
 end

lib/http/client.rb

@@ -1,9 +1,11 @@
 require "cgi"
 require "uri"
+
 require "http/form_data"
 require "http/options"
 require "http/connection"
 require "http/redirector"
+require "http/uri"
 
 module HTTP
   # Clients make requests and receive responses
@@ -14,6 +16,8 @@ class Client
     KEEP_ALIVE         = "Keep-Alive".freeze
     CLOSE              = "close".freeze
 
+    HTTP_OR_HTTPS_RE   = %r{^https?://}i
+
     attr_reader :default_options
 
     def initialize(default_options = {})
@@ -109,33 +113,23 @@ def verify_connection!(uri)
 
     # Strips out query/path to give us a consistent way of comparing hosts
     def base_host(uri)
-      base = uri.dup
-      base.query = nil
-      base.path = ""
-      base.to_s
+      uri.omit(:query, :path).to_s
     end
 
     # Merges query params if needed
+    #
+    # @param [#to_str] uri
+    # @return [URI]
     def make_request_uri(uri, options)
-      uri = normalize_uri uri
-
-      if options.params && !options.params.empty?
-        params    = CGI.parse(uri.query.to_s).merge(options.params || {})
-        uri.query = URI.encode_www_form params
+      if default_options.persistent? && uri !~ HTTP_OR_HTTPS_RE
+        uri = "#{default_options.persistent}#{uri}"
       end
 
-      uri
-    end
+      uri = HTTP::URI.parse uri
 
-    # Normalize URI
-    #
-    # @param [#to_s] uri
-    # @return [URI]
-    def normalize_uri(uri)
-      if default_options.persistent? && uri !~ /^http|https/
-        uri = URI("#{default_options.persistent}#{uri}")
-      else
-        uri = URI(uri.to_s)
+      if options.params && !options.params.empty?
+        params    = CGI.parse(uri.query.to_s).merge(options.params || {})
+        uri.query = ::URI.encode_www_form params
       end
 
       # Some proxies (seen on WEBRick) fail if URL has

lib/http/connection.rb

@@ -129,7 +129,7 @@ def expired?
     # @param (see #initialize)
     # @return [void]
     def start_tls(req, options)
-      return unless req.uri.is_a?(URI::HTTPS) && !req.using_proxy?
+      return unless req.uri.https? && !req.using_proxy?
 
       ssl_context = options[:ssl_context]
 

lib/http/request.rb

@@ -1,11 +1,12 @@
+require "base64"
+require "time"
+
 require "http/errors"
 require "http/headers"
 require "http/request/caching"
 require "http/request/writer"
 require "http/version"
-require "base64"
-require "uri"
-require "time"
+require "http/uri"
 
 module HTTP
   class Request
@@ -70,7 +71,7 @@ def __method__(*args)
     # :nodoc:
     def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") # rubocop:disable ParameterLists
       @verb   = verb.to_s.downcase.to_sym
-      @uri    = uri.is_a?(URI) ? uri : URI(uri.to_s)
+      @uri    = HTTP::URI.parse uri
       @scheme = @uri.scheme && @uri.scheme.to_s.downcase.to_sym
 
       fail(UnsupportedMethodError, "unknown method: #{verb}") unless METHODS.include?(@verb)
@@ -86,8 +87,7 @@ def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1")
 
     # Returns new Request with updated uri
     def redirect(uri, verb = @verb)
-      uri = @uri.merge uri.to_s
-      req = self.class.new(verb, uri, headers, proxy, body, version)
+      req = self.class.new(verb, @uri.join(uri), headers, proxy, body, version)
       req["Host"] = req.uri.host
       req
     end
@@ -157,11 +157,11 @@ def uri_has_query?
     #
     # @return [String]
     def default_host
-      if PORTS[@scheme] == @uri.port
-        @uri.host
-      else
-        "#{@uri.host}:#{@uri.port}"
+      if @uri.port && PORTS[@scheme] != @uri.port
+        return "#{@uri.host}:#{@uri.port}"
       end
+
+      @uri.host
     end
   end
 end

lib/http/uri.rb

@@ -0,0 +1,23 @@
+require "addressable/uri"
+
+module HTTP
+  class URI < Addressable::URI
+    # HTTP scheme
+    HTTP_SCHEME = "http".freeze
+
+    # HTTPS scheme
+    HTTPS_SCHEME = "https".freeze
+
+    # @return [True] if URI is HTTP
+    # @return [False] otherwise
+    def http?
+      HTTP_SCHEME == scheme
+    end
+
+    # @return [True] if URI is HTTPS
+    # @return [False] otherwise
+    def https?
+      HTTPS_SCHEME == scheme
+    end
+  end
+end

spec/lib/http/client_spec.rb

@@ -1,3 +1,5 @@
+# coding: utf-8
+
 require "support/http_handling_shared"
 require "support/dummy_server"
 require "support/ssl_helper"
@@ -9,15 +11,18 @@
 
   StubbedClient = Class.new(HTTP::Client) do
     def make_request(request, options)
-      stubs.fetch(request.uri.to_s) { super(request, options) }
+      stubs.fetch(request.uri) { super(request, options) }
     end
 
     def stubs
       @stubs ||= {}
     end
 
     def stub(stubs)
-      @stubs = stubs
+      @stubs = stubs.each_with_object({}) do |(k, v), o|
+        o[HTTP::URI.parse k] = v
+      end
+
       self
     end
   end
@@ -73,6 +78,15 @@ def simple_response(body, status = 200)
       expect { client.get("http://example.com/") }
         .to raise_error(HTTP::Redirector::TooManyRedirectsError)
     end
+
+    it "works with non-ASCII URIs" do
+      client = StubbedClient.new(:follow => true).stub(
+        "http://example.com/"      => redirect_response("/könig"),
+        "http://example.com/könig" => simple_response("OK")
+      )
+
+      expect(client.get("http://example.com/").to_s).to eq "OK"
+    end
   end
 
   describe "caching" do
@@ -154,6 +168,11 @@ def simple_response(body, status = 200)
   end
 
   describe "#request" do
+    it "works with non-ASCII URIs" do
+      client = HTTP::Client.new
+      expect { client.get "#{dummy.endpoint}/könig" }.not_to raise_error
+    end
+
     context "with explicitly given `Host` header" do
       let(:headers) { {"Host" => "another.example.com"} }
       let(:client)  { described_class.new :headers => headers }

spec/lib/http/request_spec.rb

@@ -71,7 +71,7 @@
 
     subject(:redirected) { request.redirect "http://blog.example.com/" }
 
-    its(:uri)     { is_expected.to eq URI.parse "http://blog.example.com/" }
+    its(:uri)     { is_expected.to eq HTTP::URI.parse "http://blog.example.com/" }
 
     its(:verb)    { is_expected.to eq request.verb }
     its(:body)    { is_expected.to eq request.body }
@@ -84,7 +84,7 @@
     context "with schema-less absolute URL given" do
       subject(:redirected) { request.redirect "//another.example.com/blog" }
 
-      its(:uri)     { is_expected.to eq URI.parse "http://another.example.com/blog" }
+      its(:uri)     { is_expected.to eq HTTP::URI.parse "http://another.example.com/blog" }
 
       its(:verb)    { is_expected.to eq request.verb }
       its(:body)    { is_expected.to eq request.body }
@@ -98,7 +98,7 @@
     context "with relative URL given" do
       subject(:redirected) { request.redirect "/blog" }
 
-      its(:uri)     { is_expected.to eq URI.parse "http://example.com/blog" }
+      its(:uri)     { is_expected.to eq HTTP::URI.parse "http://example.com/blog" }
 
       its(:verb)    { is_expected.to eq request.verb }
       its(:body)    { is_expected.to eq request.body }
@@ -110,14 +110,14 @@
 
       context "with original URI having non-standard port" do
         let(:request) { HTTP::Request.new(:post, "http://example.com:8080/", headers, proxy, body) }
-        its(:uri)     { is_expected.to eq URI.parse "http://example.com:8080/blog" }
+        its(:uri)     { is_expected.to eq HTTP::URI.parse "http://example.com:8080/blog" }
       end
     end
 
     context "with relative URL that misses leading slash given" do
       subject(:redirected) { request.redirect "blog" }
 
-      its(:uri)     { is_expected.to eq URI.parse "http://example.com/blog" }
+      its(:uri)     { is_expected.to eq HTTP::URI.parse "http://example.com/blog" }
 
       its(:verb)    { is_expected.to eq request.verb }
       its(:body)    { is_expected.to eq request.body }
@@ -129,7 +129,7 @@
 
       context "with original URI having non-standard port" do
         let(:request) { HTTP::Request.new(:post, "http://example.com:8080/", headers, proxy, body) }
-        its(:uri)     { is_expected.to eq URI.parse "http://example.com:8080/blog" }
+        its(:uri)     { is_expected.to eq HTTP::URI.parse "http://example.com:8080/blog" }
       end
     end
 

spec/lib/http_spec.rb

@@ -14,7 +14,7 @@
 
     context "with URI instance" do
       it "is easy" do
-        response = HTTP.get URI dummy.endpoint
+        response = HTTP.get HTTP::URI.parse dummy.endpoint
         expect(response.to_s).to match(/<!doctype html>/)
       end
     end