Make request body reading safe to rewind

Author: sl0thentr0py

CHANGELOG.md

@@ -45,6 +45,7 @@
   ```
 - Add `config.profiles_sample_interval` to control sampling frequency ([#2745](https://github.com/getsentry/sentry-ruby/pull/2745))
   - Both `stackprof` and `vernier` now get sampled at a default frequency of 101 Hz.
+- Request body reading checks for `:rewind` to match Rack 3 behavior. ([#2754](https://github.com/getsentry/sentry-ruby/pull/2754))
 
 ### Internal
 

sentry-ruby/lib/sentry/interfaces/request.rb

@@ -69,6 +69,8 @@ def initialize(env:, send_default_pii:, rack_env_whitelist:)
     private
 
     def read_data_from(request)
+      return "Skipped non-rewindable request body" unless request.body.respond_to?(:rewind)
+
       if request.form_data?
         request.POST
       elsif request.body # JSON requests, etc

sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb

@@ -182,6 +182,21 @@ def to_s
       expect(subject.data).to eq("catch me")
     end
 
+    it "does not try to read non rewindable body" do
+      env.merge!(::Rack::RACK_INPUT => double)
+
+      expect(subject.data).to eq("Skipped non-rewindable request body")
+    end
+
+    it "reads rewindable body" do
+      dbl = double
+      allow(dbl).to receive(:rewind)
+      allow(dbl).to receive(:read).and_return("stuff")
+      env.merge!(::Rack::RACK_INPUT => dbl)
+
+      expect(subject.data).to eq("stuff")
+    end
+
     it "stores Authorization header" do
       env.merge!("HTTP_AUTHORIZATION" => "Basic YWxhZGRpbjpvcGVuc2VzYW1l")
 

sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb

@@ -229,6 +229,92 @@ def inspect
         expect(Sentry.get_current_scope.tags).to eq(tag_1: "don't change me")
       end
     end
+
+    context "with send_default_pii" do
+      before do
+        perform_basic_setup do |config|
+          config.send_default_pii = true
+        end
+      end
+
+      context "with form data" do
+        let(:additional_headers) do
+          { "REQUEST_METHOD" => "POST", ::Rack::RACK_INPUT => StringIO.new("foo=bar") }
+        end
+
+        it "captures the exception with request form data" do
+          app = ->(_e) { raise exception }
+          stack = Sentry::Rack::CaptureExceptions.new(app)
+
+          expect { stack.call(env) }.to raise_error(ZeroDivisionError)
+
+          event = last_sentry_event.to_h
+          expect(event.dig(:request, :url)).to eq("http://example.org/test")
+          expect(event.dig(:request, :data)).to eq({ "foo" => "bar" })
+        end
+
+        it "allows later middlewares to read body" do
+          app = ->(_e) { raise exception }
+          stack = Sentry::Rack::CaptureExceptions.new(app)
+
+          expect { stack.call(env) }.to raise_error(ZeroDivisionError)
+          expect { ::Rack::Request.new(env).body.read }.not_to raise_error
+        end
+      end
+
+      context "with rewindable non form data" do
+        let(:additional_headers) do
+          { "REQUEST_METHOD" => "POST", "CONTENT_TYPE" => "application/text", ::Rack::RACK_INPUT => StringIO.new("stuff") }
+        end
+
+        it "captures the exception with request form data" do
+          app = ->(_e) { raise exception }
+          stack = Sentry::Rack::CaptureExceptions.new(app)
+
+          expect { stack.call(env) }.to raise_error(ZeroDivisionError)
+
+          event = last_sentry_event.to_h
+          expect(event.dig(:request, :url)).to eq("http://example.org/test")
+          expect(event.dig(:request, :data)).to eq("stuff")
+        end
+
+        it "allows later middlewares to read body" do
+          app = ->(_e) { raise exception }
+          stack = Sentry::Rack::CaptureExceptions.new(app)
+
+          expect { stack.call(env) }.to raise_error(ZeroDivisionError)
+          expect { ::Rack::Request.new(env).body.read }.not_to raise_error
+        end
+      end
+
+      context "with non rewindable non form data" do
+        let(:dbl) { double }
+        let(:additional_headers) do
+          { "REQUEST_METHOD" => "POST", "CONTENT_TYPE" => "application/text", ::Rack::RACK_INPUT => dbl }
+        end
+
+        it "does not try to read non rewindable body" do
+          app = ->(_e) { raise exception }
+          stack = Sentry::Rack::CaptureExceptions.new(app)
+
+          expect { stack.call(env) }.to raise_error(ZeroDivisionError)
+
+          event = last_sentry_event.to_h
+          expect(event.dig(:request, :url)).to eq("http://example.org/test")
+          expect(event.dig(:request, :data)).to eq("Skipped non-rewindable request body")
+        end
+
+        it "allows later middlewares to read body" do
+          allow(dbl).to receive(:read)
+
+          app = ->(_e) { raise exception }
+          stack = Sentry::Rack::CaptureExceptions.new(app)
+
+          expect { stack.call(env) }.to raise_error(ZeroDivisionError)
+          expect { ::Rack::Request.new(env).body.read }.not_to raise_error
+        end
+      end
+    end
   end
 
   describe "performance monitoring" do