diff --git a/lib/onebox/helpers.rb b/lib/onebox/helpers.rb index 0d20d187..3768a08c 100644 --- a/lib/onebox/helpers.rb +++ b/lib/onebox/helpers.rb @@ -51,17 +51,14 @@ def self.fetch_response(location, limit = nil, domain = nil, headers = nil) raise Net::HTTPError.new('HTTP redirect too deep', location) if limit == 0 - uri = URI(location) - uri = URI("#{domain}#{location}") if !uri.host + uri = Addressable::URI.parse(location) + uri = Addressable::URI.join(domain, uri) if !uri.host result = StringIO.new - Net::HTTP.start(uri.host, uri.port, use_ssl: uri.is_a?(URI::HTTPS)) do |http| + Net::HTTP.start(uri.host, uri.port, use_ssl: uri.normalized_scheme == 'https') do |http| http.open_timeout = Onebox.options.connect_timeout http.read_timeout = Onebox.options.timeout - if uri.is_a?(URI::HTTPS) - http.use_ssl = true - http.verify_mode = OpenSSL::SSL::VERIFY_NONE - end + http.verify_mode = OpenSSL::SSL::VERIFY_NONE # Work around path building bugs headers ||= {} @@ -76,10 +73,12 @@ def self.fetch_response(location, limit = nil, domain = nil, headers = nil) http.request(request) do |response| if cookie = response.get_fields('set-cookie') - header = { 'Cookie' => cookie.join } + # HACK: If this breaks again in the future, use HTTP::CookieJar from gem 'http-cookie' + # See test: it "does not send cookies to the wrong domain" + redir_header = { 'Cookie' => cookie.join('; ') } end - header = nil unless header.is_a? Hash + redir_header = nil unless redir_header.is_a? Hash code = response.code.to_i unless code === 200 @@ -88,7 +87,7 @@ def self.fetch_response(location, limit = nil, domain = nil, headers = nil) response['location'], limit - 1, "#{uri.scheme}://#{uri.host}", - header + redir_header ) end diff --git a/spec/lib/onebox/helpers_spec.rb b/spec/lib/onebox/helpers_spec.rb index df2e82aa..b5040bf4 100644 --- a/spec/lib/onebox/helpers_spec.rb +++ b/spec/lib/onebox/helpers_spec.rb @@ -44,6 +44,53 @@ end end + describe "redirects" do + describe "redirect limit" do + before do + (1..6).each do |i| + FakeWeb.register_uri(:get, "https://httpbin.org/redirect_count/#{i}", location: "https://httpbin.org/redirect_count/#{i - 1}", body: "", status: [302, "Found"]) + end + fake("https://httpbin.org/redirect_count/0", "

success

") + end + + it "can follow redirects" do + expect(described_class.fetch_response("https://httpbin.org/redirect_count/2")).to match("success") + end + + it "errors on long redirect chains" do + expect { + described_class.fetch_response("https://httpbin.org/redirect_count/6") + }.to raise_error(Net::HTTPError, /redirect too deep/) + end + end + + describe "cookie handling" do + it "naively forwards cookies to the next request" do + FakeWeb.register_uri(:get, "https://httpbin.org/cookies/set/a/b", + location: "/cookies", + 'set-cookie': "a=b; Path=/", + status: [302, "Found"]) + fake("https://httpbin.org/cookies", "success, cookie readback not implemented") + + described_class.fetch_response('https://httpbin.org/cookies/set/a/b') + expect(FakeWeb.last_request.to_hash['cookie'][0]).to match("a=b") + end + + it "does not send cookies to the wrong domain" do + skip("unimplemented") + + FakeWeb.register_uri(:get, "https://httpbin.org/cookies/set/a/b", + location: "https://evil.com/show_cookies", + 'set-cookie': "a=b; Path=/", + status: [302, "Found"]) + fake("https://evil.com/show_cookies", "success, cookie readback not implemented") + + described_class.fetch_response('https://httpbin.org/cookies/set/a/b') + expect(FakeWeb.last_request.to_hash['cookie']).to be_nil + end + end + end + describe "user_agent" do before do fake("http://example.com/some-resource", body: 'test') diff --git a/spec/support/html_spec_helper.rb b/spec/support/html_spec_helper.rb index de17615d..f0981cff 100644 --- a/spec/support/html_spec_helper.rb +++ b/spec/support/html_spec_helper.rb @@ -2,10 +2,10 @@ module HTMLSpecHelper def fake(uri, response, verb = :get) - FakeWeb.register_uri(verb, uri, response: header(response)) + FakeWeb.register_uri(verb, uri, response: http_ok(response)) end - def header(html) + def http_ok(html) "HTTP/1.1 200 OK\n\n#{html}" end