diff --git a/.travis.yml b/.travis.yml index b13650e4..15aa7472 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,6 +15,7 @@ rvm: - ruby-head matrix: allow_failures: + - rvm: 1.8.7 - rvm: jruby-head - rvm: ruby-head fast_finish: true diff --git a/Gemfile b/Gemfile index 62b9b85d..d638a93f 100644 --- a/Gemfile +++ b/Gemfile @@ -16,9 +16,15 @@ group :test do gem 'rack', '~> 1.2', :platforms => [:jruby_18, :jruby_19, :ruby_18, :ruby_19, :ruby_20, :ruby_21] gem 'rest-client', '~> 1.6.0', :platforms => [:jruby_18, :ruby_18] gem 'rspec', '>= 3' - gem 'rubocop', '>= 0.37', :platforms => [:ruby_19, :ruby_20, :ruby_21] + gem 'rubocop', '>= 0.37', :platforms => [:ruby_20, :ruby_21] gem 'simplecov', '>= 0.9' gem 'yardstick' + + platforms :ruby_18, :ruby_19 do + gem 'json', '< 2.0' + gem 'tins', '< 1.7' + gem 'term-ansicolor', '< 1.4.0' + end end gemspec diff --git a/lib/oauth2/client.rb b/lib/oauth2/client.rb index cb1b5648..1fbbdd8a 100644 --- a/lib/oauth2/client.rb +++ b/lib/oauth2/client.rb @@ -136,8 +136,10 @@ def get_token(params, access_token_opts = {}, access_token_class = AccessToken) opts[:params] = params end response = request(options[:token_method], token_url, opts) - error = Error.new(response) - raise(error) if options[:raise_errors] && !(response.parsed.is_a?(Hash) && response.parsed['access_token']) + if options[:raise_errors] && !(response.parsed.is_a?(Hash) && response.parsed['access_token']) + error = Error.new(response) + raise(error) + end access_token_class.from_hash(self, response.parsed.merge(access_token_opts)) end diff --git a/lib/oauth2/error.rb b/lib/oauth2/error.rb index c49a5535..c2618a18 100644 --- a/lib/oauth2/error.rb +++ b/lib/oauth2/error.rb @@ -27,7 +27,7 @@ def error_message(response_body, opts = {}) error_message = if opts[:error_description] && opts[:error_description].respond_to?(:encoding) script_encoding = opts[:error_description].encoding - response_body.encode(script_encoding) + response_body.encode(script_encoding, :invalid => :replace, :undef => :replace) else response_body end diff --git a/spec/oauth2/client_spec.rb b/spec/oauth2/client_spec.rb index 4facce22..f883461c 100644 --- a/spec/oauth2/client_spec.rb +++ b/spec/oauth2/client_spec.rb @@ -9,16 +9,17 @@ subject do OAuth2::Client.new('abc', 'def', :site => 'https://api.example.com') do |builder| builder.adapter :test do |stub| - stub.get('/success') { |env| [200, {'Content-Type' => 'text/awesome'}, 'yay'] } - stub.get('/reflect') { |env| [200, {}, env[:body]] } - stub.post('/reflect') { |env| [200, {}, env[:body]] } - stub.get('/unauthorized') { |env| [401, {'Content-Type' => 'application/json'}, MultiJson.encode(:error => error_value, :error_description => error_description_value)] } - stub.get('/conflict') { |env| [409, {'Content-Type' => 'text/plain'}, 'not authorized'] } - stub.get('/redirect') { |env| [302, {'Content-Type' => 'text/plain', 'location' => '/success'}, ''] } - stub.post('/redirect') { |env| [303, {'Content-Type' => 'text/plain', 'location' => '/reflect'}, ''] } - stub.get('/error') { |env| [500, {'Content-Type' => 'text/plain'}, 'unknown error'] } - stub.get('/empty_get') { |env| [204, {}, nil] } - stub.get('/different_encoding') { |env| [500, {'Content-Type' => 'application/json'}, NKF.nkf('-We', MultiJson.encode(:error => error_value, :error_description => '∞'))] } + stub.get('/success') { |env| [200, {'Content-Type' => 'text/awesome'}, 'yay'] } + stub.get('/reflect') { |env| [200, {}, env[:body]] } + stub.post('/reflect') { |env| [200, {}, env[:body]] } + stub.get('/unauthorized') { |env| [401, {'Content-Type' => 'application/json'}, MultiJson.encode(:error => error_value, :error_description => error_description_value)] } + stub.get('/conflict') { |env| [409, {'Content-Type' => 'text/plain'}, 'not authorized'] } + stub.get('/redirect') { |env| [302, {'Content-Type' => 'text/plain', 'location' => '/success'}, ''] } + stub.post('/redirect') { |env| [303, {'Content-Type' => 'text/plain', 'location' => '/reflect'}, ''] } + stub.get('/error') { |env| [500, {'Content-Type' => 'text/plain'}, 'unknown error'] } + stub.get('/empty_get') { |env| [204, {}, nil] } + stub.get('/different_encoding') { |env| [500, {'Content-Type' => 'application/json'}, NKF.nkf('-We', MultiJson.encode(:error => error_value, :error_description => '∞'))] } + stub.get('/ascii_8bit_encoding') { |env| [500, {'Content-Type' => 'application/json'}, MultiJson.encode(:error => 'invalid_request', :error_description => 'é').force_encoding('ASCII-8BIT')] } end end end @@ -167,12 +168,21 @@ expect(response.error).not_to be_nil end - %w(/unauthorized /conflict /error /different_encoding).each do |error_path| + %w(/unauthorized /conflict /error /different_encoding /ascii_8bit_encoding).each do |error_path| it "raises OAuth2::Error on error response to path #{error_path}" do expect { subject.request(:get, error_path) }.to raise_error(OAuth2::Error) end end + it 're-encodes response body in the error message' do + begin + subject.request(:get, '/ascii_8bit_encoding') + rescue => ex + expect(ex.message.encoding.name).to eq('UTF-8') + expect(ex.message).to eq("invalid_request: é\n{\"error\":\"invalid_request\",\"error_description\":\"��\"}") + end + end + it 'parses OAuth2 standard error response' do begin subject.request(:get, '/unauthorized') diff --git a/spec/oauth2/strategy/auth_code_spec.rb b/spec/oauth2/strategy/auth_code_spec.rb index a6f57683..a130576e 100644 --- a/spec/oauth2/strategy/auth_code_spec.rb +++ b/spec/oauth2/strategy/auth_code_spec.rb @@ -1,3 +1,5 @@ +# encoding: utf-8 + require 'helper' describe OAuth2::Strategy::AuthCode do @@ -50,6 +52,25 @@ end end + describe '#get_token (handling utf-8 data)' do + let(:json_token) { MultiJson.encode(:expires_in => 600, :access_token => 'salmon', :refresh_token => 'trout', :extra_param => 'André') } + + before do + @mode = 'json' + client.options[:token_method] = :post + end + + it 'should not raise an error' do + expect { subject.get_token(code) }.to_not raise_error + end + + it 'should not create an error instance' do + expect(OAuth2::Error).to_not receive(:new) + + subject.get_token(code) + end + end + %w(json formencoded from_facebook).each do |mode| [:get, :post].each do |verb| describe "#get_token (#{mode}, access_token_method=#{verb}" do