From 964f166320a27b336d1f06fe41133d287c3b7746 Mon Sep 17 00:00:00 2001 From: Alex Tomlins Date: Mon, 13 Oct 2014 13:46:40 +0100 Subject: [PATCH 01/14] Fix RestClient::Request docs when used with yard This moves a backwards compatibility comment. It's currently replacing the class documentation, which means that most of the docs for this class are missing on rubydoc.info - http://rubydoc.info/gems/rest-client/RestClient/Request --- lib/restclient/exceptions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/restclient/exceptions.rb b/lib/restclient/exceptions.rb index c8d80acf..46324446 100644 --- a/lib/restclient/exceptions.rb +++ b/lib/restclient/exceptions.rb @@ -195,8 +195,8 @@ def initialize(message) end end -# backwards compatibility class RestClient::Request + # backwards compatibility Redirect = RestClient::Redirect Unauthorized = RestClient::Unauthorized RequestFailed = RestClient::RequestFailed From 39ef45627983fce488facd6d52a692d5e898db3c Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Sun, 2 Nov 2014 17:45:01 -0800 Subject: [PATCH 02/14] Rename Platform.mac? to .mac_mri? for clarity. This better reflects the fact that the method is expected to return false on non-MRI engines even on OS X. --- lib/restclient/platform.rb | 2 +- lib/restclient/request.rb | 2 +- spec/integration/request_spec.rb | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/restclient/platform.rb b/lib/restclient/platform.rb index d96239a9..c3f321b4 100644 --- a/lib/restclient/platform.rb +++ b/lib/restclient/platform.rb @@ -4,7 +4,7 @@ module Platform # be false for jruby even on OS X. # # @return [Boolean] - def self.mac? + def self.mac_mri? RUBY_PLATFORM.include?('darwin') end diff --git a/lib/restclient/request.rb b/lib/restclient/request.rb index 2f664941..66a1f96d 100644 --- a/lib/restclient/request.rb +++ b/lib/restclient/request.rb @@ -334,7 +334,7 @@ def self.default_ssl_cert_store def print_verify_callback_warnings warned = false - if RestClient::Platform.mac? + if RestClient::Platform.mac_mri? warn('warning: ssl_verify_callback return code is ignored on OS X') warned = true end diff --git a/spec/integration/request_spec.rb b/spec/integration/request_spec.rb index aef0ab9b..5b0011db 100644 --- a/spec/integration/request_spec.rb +++ b/spec/integration/request_spec.rb @@ -34,7 +34,7 @@ # # On OS X, this test fails since Apple has patched OpenSSL to always fall # back on the system CA store. - it "is unsuccessful with an incorrect ca_file", :unless => RestClient::Platform.mac? do + it "is unsuccessful with an incorrect ca_file", :unless => RestClient::Platform.mac_mri? do request = RestClient::Request.new( :method => :get, :url => 'https://www.mozilla.org', @@ -45,7 +45,7 @@ # On OS X, this test fails since Apple has patched OpenSSL to always fall # back on the system CA store. - it "is unsuccessful with an incorrect ca_path", :unless => RestClient::Platform.mac? do + it "is unsuccessful with an incorrect ca_path", :unless => RestClient::Platform.mac_mri? do request = RestClient::Request.new( :method => :get, :url => 'https://www.mozilla.org', @@ -79,7 +79,7 @@ end it "fails verification when the callback returns false", - :unless => RestClient::Platform.mac? do + :unless => RestClient::Platform.mac_mri? do request = RestClient::Request.new( :method => :get, :url => 'https://www.mozilla.org', @@ -90,7 +90,7 @@ end it "succeeds verification when the callback returns true", - :unless => RestClient::Platform.mac? do + :unless => RestClient::Platform.mac_mri? do request = RestClient::Request.new( :method => :get, :url => 'https://www.mozilla.org', From 05ba65c47aa02344bed9c995b411d7c8956af3c3 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Tue, 27 Jan 2015 21:59:20 -0800 Subject: [PATCH 03/14] Drop MIME::Types monkey patch. There still isn't exactly a direct way to look up MIME types by extension, but we don't need one. --- lib/restclient/request.rb | 52 ++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/lib/restclient/request.rb b/lib/restclient/request.rb index 66a1f96d..9000107a 100644 --- a/lib/restclient/request.rb +++ b/lib/restclient/request.rb @@ -544,8 +544,7 @@ def stringify_headers headers key = key.to_s.split(/_/).map { |w| w.capitalize }.join('-') end if 'CONTENT-TYPE' == key.upcase - target_value = value.to_s - result[key] = MIME::Types.type_for_extension target_value + result[key] = maybe_convert_extension(value.to_s) elsif 'ACCEPT' == key.upcase # Accept can be composed of several comma-separated values if value.is_a? Array @@ -553,7 +552,9 @@ def stringify_headers headers else target_values = value.to_s.split ',' end - result[key] = target_values.map { |ext| MIME::Types.type_for_extension(ext.to_s.strip) }.join(', ') + result[key] = target_values.map { |ext| + maybe_convert_extension(ext.to_s.strip) + }.join(', ') else result[key] = value.to_s end @@ -571,21 +572,38 @@ def parser URI.const_defined?(:Parser) ? URI::Parser.new : URI end - end -end - -module MIME - class Types - - # Return the first found content-type for a value considered as an extension or the value itself - def type_for_extension ext - candidates = @extension_index[ext] - candidates.empty? ? ext : candidates[0].content_type - end + # Given a MIME type or file extension, return either a MIME type or, if + # none is found, the input unchanged. + # + # >> maybe_convert_extension('json') + # => 'application/json' + # + # >> maybe_convert_extension('unknown') + # => 'unknown' + # + # >> maybe_convert_extension('application/xml') + # => 'application/xml' + # + # @param ext [String] + # + # @return [String] + # + def maybe_convert_extension(ext) + unless ext =~ /\A[a-zA-Z0-9_@-]+\z/ + # Don't look up strings unless they look like they could be a file + # extension known to mime-types. + # + # There currently isn't any API public way to look up extensions + # directly out of MIME::Types, but the type_for() method only strips + # off after a period anyway. + return ext + end - class << self - def type_for_extension ext - @__types__.type_for_extension ext + types = MIME::Types.type_for(ext) + if types.empty? + ext + else + types.first.content_type end end end From a8b3f3c30bbac59f52e918668f0ef8ff4ef91178 Mon Sep 17 00:00:00 2001 From: Xavier Shay Date: Tue, 27 Jan 2015 13:53:01 -0800 Subject: [PATCH 04/14] Redact request password from logs. Fixes #349 and OSVDB-117461. --- lib/restclient/request.rb | 13 ++++++++++++- spec/unit/request_spec.rb | 12 ++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/restclient/request.rb b/lib/restclient/request.rb index 9000107a..7c5bde67 100644 --- a/lib/restclient/request.rb +++ b/lib/restclient/request.rb @@ -519,7 +519,18 @@ def log_request return unless RestClient.log out = [] - out << "RestClient.#{method} #{url.inspect}" + sanitized_url = begin + uri = URI.parse(url) + uri.password = "REDACTED" if uri.password + uri.to_s + rescue URI::InvalidURIError + # An attacker may be able to manipulate the URL to be + # invalid, which could force discloure of a password if + # we show any of the un-parsed URL here. + "[invalid uri]" + end + + out << "RestClient.#{method} #{sanitized_url.inspect}" out << payload.short_inspect if payload out << processed_headers.to_a.sort.map { |(k, v)| [k.inspect, v.inspect].join("=>") }.join(", ") RestClient.log << out.join(', ') + "\n" diff --git a/spec/unit/request_spec.rb b/spec/unit/request_spec.rb index c6d327c0..eef5fe62 100644 --- a/spec/unit/request_spec.rb +++ b/spec/unit/request_spec.rb @@ -414,6 +414,18 @@ @request.log_response res log[0].should eq "# => 200 OK | text/html 0 bytes\n" end + + it 'does not log request password' do + log = RestClient.log = [] + RestClient::Request.new(:method => :get, :url => 'http://user:password@url', :headers => {:user_agent => 'rest-client'}).log_request + log[0].should eq %Q{RestClient.get "http://user:REDACTED@url", "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "User-Agent"=>"rest-client"\n} + end + + it 'logs invalid URIs, even though they will fail elsewhere' do + log = RestClient.log = [] + RestClient::Request.new(:method => :get, :url => 'http://a@b:c', :headers => {:user_agent => 'rest-client'}).log_request + log[0].should eq %Q{RestClient.get "[invalid uri]", "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "User-Agent"=>"rest-client"\n} + end end it "strips the charset from the response content type" do From 10c3beb1ebb94211cfcbfa4e938d08c0f0894dc2 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Thu, 19 Feb 2015 18:10:32 -0800 Subject: [PATCH 05/14] Fix tests for 1.7.x backport of #352. --- spec/unit/request_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/unit/request_spec.rb b/spec/unit/request_spec.rb index eef5fe62..7c00873e 100644 --- a/spec/unit/request_spec.rb +++ b/spec/unit/request_spec.rb @@ -417,13 +417,13 @@ it 'does not log request password' do log = RestClient.log = [] - RestClient::Request.new(:method => :get, :url => 'http://user:password@url', :headers => {:user_agent => 'rest-client'}).log_request + RestClient::Request.new(:method => :get, :url => 'http://user:password@url', :headers => {:user_agent => 'rest-client', :accept => '*/*'}).log_request log[0].should eq %Q{RestClient.get "http://user:REDACTED@url", "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "User-Agent"=>"rest-client"\n} end it 'logs invalid URIs, even though they will fail elsewhere' do log = RestClient.log = [] - RestClient::Request.new(:method => :get, :url => 'http://a@b:c', :headers => {:user_agent => 'rest-client'}).log_request + RestClient::Request.new(:method => :get, :url => 'http://a@b:c', :headers => {:user_agent => 'rest-client', :accept => '*/*'}).log_request log[0].should eq %Q{RestClient.get "[invalid uri]", "Accept"=>"*/*", "Accept-Encoding"=>"gzip, deflate", "User-Agent"=>"rest-client"\n} end end From 778e1ea5c7a28350ad94a9742a46062c8eebd581 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Thu, 19 Feb 2015 18:15:18 -0800 Subject: [PATCH 06/14] Import history from 2.0.x branch. --- history.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/history.md b/history.md index 24dacb06..4714e2ba 100644 --- a/history.md +++ b/history.md @@ -1,3 +1,9 @@ +# 1.7.3 + +- Security: redact password in URI from logs (#349 / OSVDB-117461) +- Drop monkey patch on MIME::Types (added `type_for_extension` method, use + the public interface instead. + # 1.7.2 - Ignore duplicate certificates in CA store on Windows From 988d5a81ff3081c1c6a3ab267e81a9fff94bd9e4 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Thu, 19 Feb 2015 18:16:43 -0800 Subject: [PATCH 07/14] Tag version 1.7.3 --- lib/restclient/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/restclient/version.rb b/lib/restclient/version.rb index 7628cf67..a19f0691 100644 --- a/lib/restclient/version.rb +++ b/lib/restclient/version.rb @@ -1,5 +1,5 @@ module RestClient - VERSION = '1.7.2' unless defined?(self::VERSION) + VERSION = '1.7.3' unless defined?(self::VERSION) def self.version VERSION From 37d92c0a64ad656bfde11745293b5de7dccafb9b Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Mon, 16 Mar 2015 14:21:03 -0700 Subject: [PATCH 08/14] Add dependency on http-cookie gem. --- rest-client.gemspec | 1 + 1 file changed, 1 insertion(+) diff --git a/rest-client.gemspec b/rest-client.gemspec index ae08c51b..967457c2 100644 --- a/rest-client.gemspec +++ b/rest-client.gemspec @@ -22,6 +22,7 @@ Gem::Specification.new do |s| s.add_development_dependency('pry-doc') s.add_development_dependency('rdoc', '>= 2.4.2', '< 5.0') + s.add_dependency('http-cookie', '>= 1.0.2', '< 2.0') s.add_dependency('mime-types', '>= 1.16', '< 3.0') s.add_dependency('netrc', '~> 0.7') From 12e92312b97fce97d9d7fb3577beff5ac0c05fba Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Mon, 16 Mar 2015 19:39:53 -0700 Subject: [PATCH 09/14] Add cookie jar for handling HTTP redirection. Add new cookie_jar method to responses, and reimplement cookies using it. Also dup args when generating redirect options rather than mutating the args on the response object. --- lib/restclient/abstract_response.rb | 47 +++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/lib/restclient/abstract_response.rb b/lib/restclient/abstract_response.rb index 26645140..63e3b3bb 100644 --- a/lib/restclient/abstract_response.rb +++ b/lib/restclient/abstract_response.rb @@ -1,4 +1,5 @@ require 'cgi' +require 'http-cookie' module RestClient @@ -24,9 +25,28 @@ def raw_headers # Hash of cookies extracted from response headers def cookies - @cookies ||= (self.headers[:set_cookie] || {}).inject({}) do |out, cookie_content| - out.merge parse_cookie(cookie_content) + hash = {} + + cookie_jar.cookies.each do |out, cookie| + hash[cookie.name] = cookie.value end + + hash + end + + # Cookie jar extracted from response headers. + # + # @return [HTTP::CookieJar] + # + def cookie_jar + return @cookie_jar if @cookie_jar + + jar = HTTP::CookieJar.new + headers.fetch(:set_cookie, []).each do |cookie| + jar.parse(cookie, @args.fetch(:url)) + end + + @cookie_jar = jar end # Return the default behavior corresponding to the response code: @@ -61,25 +81,28 @@ def description # Follow a redirection def follow_redirection request = nil, result = nil, & block + new_args = @args.dup + url = headers[:location] if url !~ /^http/ url = URI.parse(args[:url]).merge(url).to_s end - args[:url] = url + new_args[:url] = url if request if request.max_redirects == 0 raise MaxRedirectsReached end - args[:password] = request.password - args[:user] = request.user - args[:headers] = request.headers - args[:max_redirects] = request.max_redirects - 1 - # pass any cookie set in the result - if result && result['set-cookie'] - args[:headers][:cookies] = (args[:headers][:cookies] || {}).merge(parse_cookie(result['set-cookie'])) - end + new_args[:password] = request.password + new_args[:user] = request.user + new_args[:headers] = request.headers + new_args[:max_redirects] = request.max_redirects - 1 + + # TODO: figure out what to do with original :cookie, :cookies values + new_args[:headers]['Cookie'] = HTTP::Cookie.cookie_value( + cookie_jar.cookies(new_args.fetch(:url))) end - Request.execute args, &block + + Request.execute(new_args, &block) end def self.beautify_headers(headers) From c215b22bdbcb988dcc40117ff45432b0db25175b Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Sat, 21 Mar 2015 19:08:21 -0700 Subject: [PATCH 10/14] Fix up cookie redirect functionality and tests. - Store the request object on responses. We need this in order to be able to correctly process the cookie jar with the right request URI. - Fix tests that rely on the old broken Set-Cookie processing code. - Add test that cookies are not passed across domains on redirect. --- lib/restclient/abstract_response.rb | 14 ++++++--- lib/restclient/raw_response.rb | 5 ++-- lib/restclient/request.rb | 4 +-- lib/restclient/response.rb | 7 ++--- spec/unit/abstract_response_spec.rb | 9 ++++-- spec/unit/raw_response_spec.rb | 3 +- spec/unit/response_spec.rb | 45 +++++++++++++++++------------ 7 files changed, 52 insertions(+), 35 deletions(-) diff --git a/lib/restclient/abstract_response.rb b/lib/restclient/abstract_response.rb index 63e3b3bb..1cc5657e 100644 --- a/lib/restclient/abstract_response.rb +++ b/lib/restclient/abstract_response.rb @@ -5,7 +5,7 @@ module RestClient module AbstractResponse - attr_reader :net_http_res, :args + attr_reader :net_http_res, :args, :request # HTTP status code def code @@ -23,11 +23,17 @@ def raw_headers @raw_headers ||= @net_http_res.to_hash end + def response_set_vars(net_http_res, args, request) + @net_http_res = net_http_res + @args = args + @request = request + end + # Hash of cookies extracted from response headers def cookies hash = {} - cookie_jar.cookies.each do |out, cookie| + cookie_jar.cookies.each do |cookie| hash[cookie.name] = cookie.value end @@ -43,7 +49,7 @@ def cookie_jar jar = HTTP::CookieJar.new headers.fetch(:set_cookie, []).each do |cookie| - jar.parse(cookie, @args.fetch(:url)) + jar.parse(cookie, @request.url) end @cookie_jar = jar @@ -85,7 +91,7 @@ def follow_redirection request = nil, result = nil, & block url = headers[:location] if url !~ /^http/ - url = URI.parse(args[:url]).merge(url).to_s + url = URI.parse(request.url).merge(url).to_s end new_args[:url] = url if request diff --git a/lib/restclient/raw_response.rb b/lib/restclient/raw_response.rb index 784003a7..69fc939b 100644 --- a/lib/restclient/raw_response.rb +++ b/lib/restclient/raw_response.rb @@ -13,12 +13,13 @@ class RawResponse include AbstractResponse - attr_reader :file + attr_reader :file, :request - def initialize tempfile, net_http_res, args + def initialize(tempfile, net_http_res, args, request) @net_http_res = net_http_res @args = args @file = tempfile + @request = request end def to_s diff --git a/lib/restclient/request.rb b/lib/restclient/request.rb index 7c5bde67..a9a0e600 100644 --- a/lib/restclient/request.rb +++ b/lib/restclient/request.rb @@ -484,9 +484,9 @@ def fetch_body(http_response) def process_result res, & block if @raw_response # We don't decode raw requests - response = RawResponse.new(@tf, res, args) + response = RawResponse.new(@tf, res, args, self) else - response = Response.create(Request.decode(res['content-encoding'], res.body), res, args) + response = Response.create(Request.decode(res['content-encoding'], res.body), res, args, self) end if block_given? diff --git a/lib/restclient/response.rb b/lib/restclient/response.rb index da92130a..47d8af54 100644 --- a/lib/restclient/response.rb +++ b/lib/restclient/response.rb @@ -6,17 +6,14 @@ module Response include AbstractResponse - attr_accessor :args, :net_http_res - def body self end - def self.create body, net_http_res, args + def self.create body, net_http_res, args, request result = body || '' result.extend Response - result.net_http_res = net_http_res - result.args = args + result.response_set_vars(net_http_res, args, request) result end diff --git a/spec/unit/abstract_response_spec.rb b/spec/unit/abstract_response_spec.rb index 83928cff..7e259a11 100644 --- a/spec/unit/abstract_response_spec.rb +++ b/spec/unit/abstract_response_spec.rb @@ -8,16 +8,18 @@ class MyAbstractResponse attr_accessor :size - def initialize net_http_res, args + def initialize net_http_res, args, request @net_http_res = net_http_res @args = args + @request = request end end before do @net_http_res = double('net http response') - @response = MyAbstractResponse.new(@net_http_res, {}) + @request = double('restclient request', :url => 'http://example.com') + @response = MyAbstractResponse.new(@net_http_res, {}, @request) end it "fetches the numeric response code" do @@ -53,7 +55,8 @@ def initialize net_http_res, args it "extract strange cookies" do @net_http_res.should_receive(:to_hash).and_return('set-cookie' => ['session_id=ZJ/HQVH6YE+rVkTpn0zvTQ==; path=/']) - @response.cookies.should eq({ 'session_id' => 'ZJ%2FHQVH6YE+rVkTpn0zvTQ%3D%3D' }) + @response.headers.should eq({:set_cookie => ['session_id=ZJ/HQVH6YE+rVkTpn0zvTQ==; path=/']}) + @response.cookies.should eq({ 'session_id' => 'ZJ/HQVH6YE+rVkTpn0zvTQ==' }) end it "doesn't escape cookies" do diff --git a/spec/unit/raw_response_spec.rb b/spec/unit/raw_response_spec.rb index d95e9653..0bc5fa51 100644 --- a/spec/unit/raw_response_spec.rb +++ b/spec/unit/raw_response_spec.rb @@ -4,7 +4,8 @@ before do @tf = double("Tempfile", :read => "the answer is 42", :open => true) @net_http_res = double('net http response') - @response = RestClient::RawResponse.new(@tf, @net_http_res, {}) + @request = double('http request') + @response = RestClient::RawResponse.new(@tf, @net_http_res, {}, @request) end it "behaves like string" do diff --git a/spec/unit/response_spec.rb b/spec/unit/response_spec.rb index ceef3184..25076763 100644 --- a/spec/unit/response_spec.rb +++ b/spec/unit/response_spec.rb @@ -3,8 +3,9 @@ describe RestClient::Response do before do @net_http_res = double('net http response', :to_hash => {"Status" => ["200 OK"]}, :code => 200) - @request = double('http request', :user => nil, :password => nil) - @response = RestClient::Response.create('abc', @net_http_res, {}) + @example_url = 'http://example.com' + @request = double('http request', :user => nil, :password => nil, :url => @example_url) + @response = RestClient::Response.create('abc', @net_http_res, {}, @request) end it "behaves like string" do @@ -14,7 +15,7 @@ end it "accepts nil strings and sets it to empty for the case of HEAD" do - RestClient::Response.create(nil, @net_http_res, {}).to_s.should eq "" + RestClient::Response.create(nil, @net_http_res, {}, @request).to_s.should eq "" end it "test headers and raw headers" do @@ -24,16 +25,18 @@ describe "cookie processing" do it "should correctly deal with one Set-Cookie header with one cookie inside" do - net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT"]}) - response = RestClient::Response.create('abc', net_http_res, {}) - response.headers[:set_cookie].should eq ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT"] + header_val = "main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT".freeze + + net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => [header_val]}) + response = RestClient::Response.create('abc', net_http_res, {}, @request) + response.headers[:set_cookie].should eq [header_val] response.cookies.should eq({ "main_page" => "main_page_no_rewrite" }) end it "should correctly deal with multiple cookies [multiple Set-Cookie headers]" do - net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT", "remember_me=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT", "user=somebody; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"]}) - response = RestClient::Response.create('abc', net_http_res, {}) - response.headers[:set_cookie].should eq ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT", "remember_me=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT", "user=somebody; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"] + net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT", "remember_me=; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT", "user=somebody; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT"]}) + response = RestClient::Response.create('abc', net_http_res, {}, @request) + response.headers[:set_cookie].should eq ["main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT", "remember_me=; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT", "user=somebody; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT"] response.cookies.should eq({ "main_page" => "main_page_no_rewrite", "remember_me" => "", @@ -42,8 +45,8 @@ end it "should correctly deal with multiple cookies [one Set-Cookie header with multiple cookies]" do - net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Tue, 20-Jan-2015 15:03:14 GMT, remember_me=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT, user=somebody; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT"]}) - response = RestClient::Response.create('abc', net_http_res, {}) + net_http_res = double('net http response', :to_hash => {"etag" => ["\"e1ac1a2df945942ef4cac8116366baad\""], "set-cookie" => ["main_page=main_page_no_rewrite; path=/; expires=Sat, 10-Jan-2037 15:03:14 GMT, remember_me=; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT, user=somebody; path=/; expires=Sat, 10-Jan-2037 00:00:00 GMT"]}) + response = RestClient::Response.create('abc', net_http_res, {}, @request) response.cookies.should eq({ "main_page" => "main_page_no_rewrite", "remember_me" => "", @@ -56,7 +59,7 @@ it "should return itself for normal codes" do (200..206).each do |code| net_http_res = double('net http response', :code => '200') - response = RestClient::Response.create('abc', net_http_res, {}) + response = RestClient::Response.create('abc', net_http_res, {}, @request) response.return! @request end end @@ -65,7 +68,7 @@ RestClient::Exceptions::EXCEPTIONS_MAP.each_key do |code| unless (200..207).include? code net_http_res = double('net http response', :code => code.to_i) - response = RestClient::Response.create('abc', net_http_res, {}) + response = RestClient::Response.create('abc', net_http_res, {}, @request) lambda { response.return!}.should raise_error end end @@ -88,32 +91,38 @@ end it "follows a redirection and keep the cookies" do + stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Set-Cookie' => 'Foo=Bar', 'Location' => 'http://some/new_resource', }) + stub_request(:get, 'http://some/new_resource').with(:headers => {'Cookie' => 'Foo=Bar'}).to_return(:body => 'Qux') + RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should eq 'Qux' + end + + it 'does not keep cookies across domains' do stub_request(:get, 'http://some/resource').to_return(:body => '', :status => 301, :headers => {'Set-Cookie' => 'Foo=Bar', 'Location' => 'http://new/resource', }) - stub_request(:get, 'http://new/resource').with(:headers => {'Cookie' => 'Foo=Bar'}).to_return(:body => 'Qux') + stub_request(:get, 'http://new/resource').with(:headers => {'Cookie' => ''}).to_return(:body => 'Qux') RestClient::Request.execute(:url => 'http://some/resource', :method => :get).body.should eq 'Qux' end it "doesn't follow a 301 when the request is a post" do net_http_res = double('net http response', :code => 301) - response = RestClient::Response.create('abc', net_http_res, {:method => :post}) + response = RestClient::Response.create('abc', net_http_res, {:method => :post}, @request) lambda { response.return!(@request)}.should raise_error(RestClient::MovedPermanently) end it "doesn't follow a 302 when the request is a post" do net_http_res = double('net http response', :code => 302) - response = RestClient::Response.create('abc', net_http_res, {:method => :post}) + response = RestClient::Response.create('abc', net_http_res, {:method => :post}, @request) lambda { response.return!(@request)}.should raise_error(RestClient::Found) end it "doesn't follow a 307 when the request is a post" do net_http_res = double('net http response', :code => 307) - response = RestClient::Response.create('abc', net_http_res, {:method => :post}) + response = RestClient::Response.create('abc', net_http_res, {:method => :post}, @request) lambda { response.return!(@request)}.should raise_error(RestClient::TemporaryRedirect) end it "doesn't follow a redirection when the request is a put" do net_http_res = double('net http response', :code => 301) - response = RestClient::Response.create('abc', net_http_res, {:method => :put}) + response = RestClient::Response.create('abc', net_http_res, {:method => :put}, @request) lambda { response.return!(@request)}.should raise_error(RestClient::MovedPermanently) end From 07e5aff5ccce20465aa9ba87572a13e1425e8ac5 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Sun, 22 Mar 2015 18:02:31 -0700 Subject: [PATCH 11/14] Version 1.8.0.rc1 --- lib/restclient/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/restclient/version.rb b/lib/restclient/version.rb index a19f0691..08e4b0ec 100644 --- a/lib/restclient/version.rb +++ b/lib/restclient/version.rb @@ -1,5 +1,5 @@ module RestClient - VERSION = '1.7.3' unless defined?(self::VERSION) + VERSION = '1.8.0.rc1' unless defined?(self::VERSION) def self.version VERSION From d2b624702b89141eb55d2ca4ee909e2bf243f739 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Sun, 22 Mar 2015 18:10:47 -0700 Subject: [PATCH 12/14] Update version spec test. --- spec/unit/restclient_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/unit/restclient_spec.rb b/spec/unit/restclient_spec.rb index 28a2cc93..190d1bf9 100644 --- a/spec/unit/restclient_spec.rb +++ b/spec/unit/restclient_spec.rb @@ -71,9 +71,9 @@ end describe 'version' do - it 'has a version ~> 1.7.0.alpha' do + it 'has a version ~> 1.8.0.alpha' do ver = Gem::Version.new(RestClient.version) - Gem::Requirement.new('~> 1.7.0.alpha').should be_satisfied_by(ver) + Gem::Requirement.new('~> 1.8.0.alpha').should be_satisfied_by(ver) end end end From 2038aa6fc5e9c0ec0ae492b31354b9b474c44c12 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Sun, 22 Mar 2015 18:10:06 -0700 Subject: [PATCH 13/14] Add history notes for 1.8.0. --- history.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/history.md b/history.md index 4714e2ba..56d9e873 100644 --- a/history.md +++ b/history.md @@ -1,3 +1,15 @@ +# 1.8.0 + +- Security: implement standards compliant cookie handling by adding a + dependency on http-cookie. This breaks compatibility, but was necessary to + address a session fixation / cookie disclosure vulnerability. + (#369 / CVE-2015-1820) + + Previously, any Set-Cookie headers found in an HTTP 30x response would be + sent to the redirection target, regardless of domain. Responses now expose a + cookie jar and respect standards compliant domain / path flags in Set-Cookie + headers. + # 1.7.3 - Security: redact password in URI from logs (#349 / OSVDB-117461) From 92bc04d18499eb20481ee3e3660e382fc377a430 Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Mon, 23 Mar 2015 19:58:33 -0700 Subject: [PATCH 14/14] Version 1.8.0 --- lib/restclient/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/restclient/version.rb b/lib/restclient/version.rb index 08e4b0ec..5f71842f 100644 --- a/lib/restclient/version.rb +++ b/lib/restclient/version.rb @@ -1,5 +1,5 @@ module RestClient - VERSION = '1.8.0.rc1' unless defined?(self::VERSION) + VERSION = '1.8.0' unless defined?(self::VERSION) def self.version VERSION