diff --git a/CHANGELOG.md b/CHANGELOG.md index e445e5550..8d3d85f6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ All notable changes to this project will be documented in this file. ## [unreleased] +- JSON parser only falls back to the raw body on `MultiJson::DecodeError`; other errors (e.g. an incompatible `json`/`multi_json` pairing) now raise instead of surfacing as a missing token (@dpep) ## [1.4.11] - 2022-09-16 - Complete migration to main branch as default (@pboling) diff --git a/lib/oauth2/response.rb b/lib/oauth2/response.rb index 9a4f5d5c3..764a2dcd4 100644 --- a/lib/oauth2/response.rb +++ b/lib/oauth2/response.rb @@ -13,7 +13,13 @@ class Response # Procs that, when called, will parse a response body according # to the specified format. @@parsers = { - :json => lambda { |body| MultiJson.load(body) rescue body }, # rubocop:disable Style/RescueModifier + :json => lambda do |body| + begin + MultiJson.load(body) + rescue MultiJson::DecodeError + body + end + end, :query => lambda { |body| Rack::Utils.parse_query(body) }, :text => lambda { |body| body }, } diff --git a/spec/oauth2/client_spec.rb b/spec/oauth2/client_spec.rb index a27cda839..b72204176 100644 --- a/spec/oauth2/client_spec.rb +++ b/spec/oauth2/client_spec.rb @@ -362,6 +362,17 @@ expect(token.token).to eq('the-token') end + it 'raises unexpected JSON parser errors' do + client = stubbed_client do |stub| + stub.post('/oauth/token') do + [200, {'Content-Type' => 'application/json'}, MultiJson.encode('access_token' => 'the-token')] + end + end + allow(MultiJson).to receive(:load).and_raise(ArgumentError) + + expect { client.get_token({}) }.to raise_error(ArgumentError) + end + it 'authenticates with request parameters' do client = stubbed_client(:auth_scheme => :request_body) do |stub| stub.post('/oauth/token', 'client_id' => 'abc', 'client_secret' => 'def') do |env| diff --git a/spec/oauth2/response_spec.rb b/spec/oauth2/response_spec.rb index 8e703aa16..33b83edf8 100644 --- a/spec/oauth2/response_spec.rb +++ b/spec/oauth2/response_spec.rb @@ -61,6 +61,21 @@ expect(subject.parsed['answer']).to eq(42) end + it 'returns the raw body when application/json body is malformed' do + headers = {'Content-Type' => 'application/json'} + response = double('response', :headers => headers, :body => 'BLOOP') + subject = described_class.new(response) + expect(subject.parsed).to eq('BLOOP') + end + + it 'raises unexpected JSON parser errors' do + headers = {'Content-Type' => 'application/json'} + response = double('response', :headers => headers, :body => '{}') + allow(MultiJson).to receive(:load).and_raise(ArgumentError) + subject = described_class.new(response) + expect { subject.parsed }.to raise_error(ArgumentError) + end + it "doesn't try to parse other content-types" do headers = {'Content-Type' => 'text/html'} body = ''