From e1a5f40bc6d21f512845526a3de6fa29ef63637f Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 20 Oct 2025 16:37:12 +0200 Subject: [PATCH] Make request body reading safe to rewind --- CHANGELOG.md | 1 + sentry-ruby/lib/sentry/interfaces/request.rb | 2 + .../interfaces/request_interface_spec.rb | 15 ++++ .../sentry/rack/capture_exceptions_spec.rb | 86 +++++++++++++++++++ 4 files changed, 104 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f39b0baa8..7a48da56a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/sentry-ruby/lib/sentry/interfaces/request.rb b/sentry-ruby/lib/sentry/interfaces/request.rb index 88feded23..3290b7652 100644 --- a/sentry-ruby/lib/sentry/interfaces/request.rb +++ b/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 diff --git a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb b/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb index 1ca2a5b67..57b569621 100644 --- a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb +++ b/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") diff --git a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb index 28a2f2736..976d87984 100644 --- a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb +++ b/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