From e14c15fcef34d46866957db903ad30172aa52983 Mon Sep 17 00:00:00 2001 From: Thomas Walpole Date: Fri, 4 Aug 2017 16:42:54 -0700 Subject: [PATCH] Don't override port if it's explicitly specified --- lib/capybara/session.rb | 13 ++++++++----- lib/capybara/spec/session/visit_spec.rb | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/capybara/session.rb b/lib/capybara/session.rb index 50fb1fca..aaa59ed8 100644 --- a/lib/capybara/session.rb +++ b/lib/capybara/session.rb @@ -247,15 +247,16 @@ module Capybara raise_server_error! @touched = true - visit_uri = URI.parse(visit_uri.to_s) + visit_uri = ::Addressable::URI.parse(visit_uri.to_s) uri_base = if @server - visit_uri.port = @server.port if config.always_include_port && (visit_uri.port == visit_uri.default_port) - URI.parse(config.app_host || "http://#{@server.host}:#{@server.port}") + ::Addressable::URI.parse(config.app_host || "http://#{@server.host}:#{@server.port}") else - config.app_host && URI.parse(config.app_host) + config.app_host && ::Addressable::URI.parse(config.app_host) end + uri_base.port ||= @server.port if @server && config.always_include_port + # TODO - this is only for compatability with previous 2.x behavior that concatenated # Capybara.app_host and a "relative" path - Consider removing in 3.0 # @abotalov brought up a good point about this behavior potentially being useful to people @@ -264,7 +265,9 @@ module Capybara visit_uri.path = uri_base.path + visit_uri.path end - visit_uri = uri_base.merge(visit_uri) unless uri_base.nil? + if uri_base && [nil, 'http', 'https'].include?(visit_uri.scheme) + visit_uri = uri_base.merge(visit_uri.to_hash.delete_if { |k,v| v.nil? }) + end driver.visit(visit_uri.to_s) end diff --git a/lib/capybara/spec/session/visit_spec.rb b/lib/capybara/spec/session/visit_spec.rb index 8367fc38..5bea5f46 100644 --- a/lib/capybara/spec/session/visit_spec.rb +++ b/lib/capybara/spec/session/visit_spec.rb @@ -63,6 +63,23 @@ Capybara::SpecHelper.spec '#visit' do expect(URI.parse(@session.current_url).port).to eq(root_uri.port) expect(@session).to have_content('Another World') end + + it "should add the server port to a visited url if no port specified", requires: [:server] do + expect(@session.driver).to receive(:visit).with("http://www.example.com:#{@session.server.port}") + @session.visit("http://www.example.com") + end + + it "should not override the visit specified port even if default for scheme", requires: [:server] do + expect(@session.driver).to receive(:visit).with("http://www.example.com:80") + @session.visit('http://www.example.com:80') + end + + it "should give preference to app_host port if specified", requires: [:server] do + Capybara.app_host = "http://www.example.com:6666" + expect(@session.driver).to receive(:visit).with("http://www.example.com:6666/random") + @session.visit('/random') + end + end context "without a server", requires: [:server] do