diff --git a/http.gemspec b/http.gemspec index 749bf2c2..5590039f 100644 --- a/http.gemspec +++ b/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 diff --git a/lib/http/client.rb b/lib/http/client.rb index 6e6f3f87..9bb7481c 100644 --- a/lib/http/client.rb +++ b/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,25 @@ 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_s] uri + # @return [URI] def make_request_uri(uri, options) - uri = normalize_uri 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 + 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 diff --git a/lib/http/connection.rb b/lib/http/connection.rb index 542352b3..de05976c 100644 --- a/lib/http/connection.rb +++ b/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] diff --git a/lib/http/request.rb b/lib/http/request.rb index 2c359735..98b1fc73 100644 --- a/lib/http/request.rb +++ b/lib/http/request.rb @@ -1,14 +1,18 @@ +require "forwardable" +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 + extend Forwardable + include HTTP::Headers::Mixin # The method given was not understood @@ -70,7 +74,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) @@ -80,14 +84,13 @@ def initialize(verb, uri, headers = {}, proxy = {}, body = nil, version = "1.1") @headers = HTTP::Headers.coerce(headers || {}) - @headers["Host"] ||= default_host + @headers["Host"] ||= default_host_header_value @headers["User-Agent"] ||= USER_AGENT end # 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 @@ -116,17 +119,17 @@ def include_proxy_authorization_header # Compute HTTP request header for direct or proxy request def request_header - "#{verb.to_s.upcase} #{path_for_request_header} HTTP/#{version}" + "#{verb.to_s.upcase} #{uri.normalize} HTTP/#{version}" end # Host for tcp socket def socket_host - using_proxy? ? proxy[:proxy_address] : uri.host + using_proxy? ? proxy[:proxy_address] : host end # Port for tcp socket def socket_port - using_proxy? ? proxy[:proxy_port] : uri.port + using_proxy? ? proxy[:proxy_port] : port end # @return [HTTP::Request::Caching] @@ -136,32 +139,19 @@ def caching private - def path_for_request_header - if using_proxy? - uri - else - uri_path_with_query - end - end - - def uri_path_with_query - path = uri_has_query? ? "#{uri.path}?#{uri.query}" : uri.path - path.empty? ? "/" : path - end + # @!attribute [r] host + # @return [String] + def_delegator :@uri, :host - def uri_has_query? - uri.query && !uri.query.empty? + # @!attribute [r] port + # @return [Fixnum] + def port + @uri.port || @uri.default_port end - # Default host (with port if needed) header value. - # - # @return [String] - def default_host - if PORTS[@scheme] == @uri.port - @uri.host - else - "#{@uri.host}:#{@uri.port}" - end + # @return [String] Default host (with port if needed) header value. + def default_host_header_value + PORTS[@scheme] != port ? "#{host}:#{port}" : host end end end diff --git a/lib/http/uri.rb b/lib/http/uri.rb new file mode 100644 index 00000000..5a049192 --- /dev/null +++ b/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 diff --git a/spec/lib/http/client_spec.rb b/spec/lib/http/client_spec.rb index 5862f533..2b357ee2 100644 --- a/spec/lib/http/client_spec.rb +++ b/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,7 +11,7 @@ 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 @@ -17,7 +19,10 @@ def 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,23 @@ def simple_response(body, status = 200) expect { client.get("http://example.com/") } .to raise_error(HTTP::Redirector::TooManyRedirectsError) end + + context "with non-ASCII URLs" do + it "theoretically works like a charm" 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/könig" }.not_to raise_error + end + + it "works like a charm in real world" do + url = "http://git.io/jNeY" + client = HTTP.follow + expect(client.get(url).to_s).to include "support for non-ascii URIs" + end + end end describe "caching" do @@ -154,6 +176,19 @@ def simple_response(body, status = 200) end describe "#request" do + context "with non-ASCII URLs" do + it "theoretically works like a charm" do + client = described_class.new + expect { client.get "#{dummy.endpoint}/könig" }.not_to raise_error + end + + it "works like a charm in real world" do + url = "https://github.com/httprb/http.rb/pull/197/ö無" + client = HTTP.follow + expect(client.get(url).to_s).to include "support for non-ascii URIs" + end + end + context "with explicitly given `Host` header" do let(:headers) { {"Host" => "another.example.com"} } let(:client) { described_class.new :headers => headers } diff --git a/spec/lib/http/request_spec.rb b/spec/lib/http/request_spec.rb index e8a35f41..c4fb3a76 100644 --- a/spec/lib/http/request_spec.rb +++ b/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 diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index 50d7a1c3..24aae87b 100644 --- a/spec/lib/http_spec.rb +++ b/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(//) end end