Commit `16f2c4e`

mo khan <mo@mokhan.ca>

2026-08-04 02:19:05

fix: treat an unparseable success body as a failure

A 200 with a non-JSON body returned { detail: <raw body> }, which Configuration#load_items then iterated. Report it instead of passing it on, and never call it a success.

cli

1 parent 56a3a36

Changed files (2)

lib

scim

kit

http.rb

spec

scim

kit

http_spec.rb

lib/scim/kit/http.rb

@@ -3,9 +3,9 @@
 module Scim
   module Kit
     class Http
-      Result = Struct.new(:status, :body) do
+      Result = Struct.new(:status, :body, :unparsed) do
         def ok?
-          !status.nil? && (200..299).cover?(status)
+          !status.nil? && (200..299).cover?(status) && !unparsed
         end
       end
 
@@ -25,8 +25,7 @@ module Scim
 
       def fetch(uri, headers: {})
         driver.with_retry(retries: retries) do |client|
-          response = get_following_redirects(client, uri, headers)
-          Result.new(response.code.to_i, parse(response.body))
+          result_for(get_following_redirects(client, uri, headers))
         end
       rescue *Net::Hippie::CONNECTION_ERRORS => error
         Scim::Kit.logger.error(error)
@@ -79,12 +78,19 @@ module Scim
         [uri.scheme, uri.host, uri.port]
       end
 
+      # An unparsed body is reported rather than raised, so the caller can
+      # show it, but it is never a successful result.
+      def result_for(response)
+        Result.new(response.code.to_i, parse(response.body))
+      rescue JSON::ParserError => error
+        Scim::Kit.logger.error(error)
+        Result.new(response.code.to_i, { detail: response.body }, true)
+      end
+
       def parse(body)
         return {} if body.nil?
 
         JSON.parse(body, symbolize_names: true)
-      rescue JSON::ParserError
-        { detail: body }
       end
     end
   end

spec/scim/kit/http_spec.rb

@@ -74,6 +74,13 @@ RSpec.describe Scim::Kit::Http do
       specify { expect(subject.fetch(uri).body).to eql(detail: 'boom') }
     end
 
+    context 'when a successful response body is not json' do
+      before { stub_request(:get, uri).to_return(status: 200, body: '<html>') }
+
+      specify { expect(subject.fetch(uri)).not_to be_ok }
+      specify { expect(subject.fetch(uri).body).to eql(detail: '<html>') }
+    end
+
     context 'when the response has no body' do
       before { stub_request(:get, uri).to_return(status: 204, body: nil) }
 
@@ -133,4 +140,20 @@ RSpec.describe Scim::Kit::Http do
       end
     end
   end
+
+  describe '#get' do
+    context 'when the response is successful' do
+      before { stub_request(:get, uri).to_return(status: 200, body: '{"a":1}') }
+
+      specify { expect(subject.get(uri)).to eql(a: 1) }
+    end
+
+    context 'when a successful response body is not json' do
+      before { stub_request(:get, uri).to_return(status: 200, body: '<html>') }
+
+      it 'does not hand the unparsed body to the caller' do
+        expect(subject.get(uri)).to eql({})
+      end
+    end
+  end
 end

Commit 16f2c4e

Commit `16f2c4e`