From c2f22d92c342ccb14efbdada28d98beb621ec460 Mon Sep 17 00:00:00 2001 From: Daniel Pepper Date: Wed, 14 Oct 2026 13:11:21 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Only=20rescue=20JSON=20decode=20?= =?UTF-8?q?errors=20in=20the=20response=20parser?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSON parser rescued every StandardError and returned the raw body, so a broken JSON stack (e.g. multi_json 1.21 with json 3 raising ArgumentError) looked like a token-less response and get_token reported invalid credentials. Rescue only MultiJson::DecodeError, which exists across multi_json ~> 1.3. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + lib/oauth2/response.rb | 8 +++++++- spec/oauth2/client_spec.rb | 11 +++++++++++ spec/oauth2/response_spec.rb | 15 +++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) 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 = ''