Release v6.27.1

CHANGELOG.md

@@ -1,6 +1,15 @@
 Changelog
 =========
 
+## v6.27.1 (18 June 2024)
+
+### Fixes
+
+* Only read Rack request body if it's rewindable
+  | [#829](https://github.com/bugsnag/bugsnag-ruby/pull/829)
+* Fix circular require warning
+  | [#828](https://github.com/bugsnag/bugsnag-ruby/pull/828)
+
 ## v6.27.0 (23 May 2024)
 
 ### Enhancements

VERSION

@@ -1 +1 @@
-6.27.0
+6.27.1

features/fixtures/docker-compose.yml

@@ -88,6 +88,7 @@ services:
       - BUGSNAG_API_KEY
       - BUGSNAG_ENDPOINT
       - BUGSNAG_METADATA_FILTERS
+      - BUGSNAG_RACK_NO_REWIND
     restart: "no"
     ports:
       - target: 3000

features/fixtures/rack/app/Gemfile

@@ -6,6 +6,4 @@ gem 'webrick' if Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('3.0.0')
 
 # Some functionality provided by Rack was moved to the 'rackup' gem in Rack v3
 # Specifically the test app uses Rack::Server, which is now Rackup::Server
-if ENV['RACK_VERSION'] == '3'
-  gem 'rackup', '~> 0.2.3'
-end
+gem 'rackup' if ENV['RACK_VERSION'] >= '3'

features/fixtures/rack/app/app.rb

@@ -71,12 +71,17 @@ def call(env)
   end
 end
 
+app = Bugsnag::Rack.new(BugsnagTests.new)
+
 Server =
   if defined?(Rack::Server)
     Rack::Server
   else
     require 'rackup'
+
+    app = Rack::RewindableInput::Middleware.new(app) unless ENV["BUGSNAG_RACK_NO_REWIND"] == "true"
+
     Rackup::Server
   end
 
-Server.start(app: Bugsnag::Rack.new(BugsnagTests.new), Host: '0.0.0.0', Port: 3000)
+Server.start(app: app, Host: '0.0.0.0', Port: 3000)

features/rack.feature

@@ -64,6 +64,9 @@ Scenario: A POST request with form data sends a report with the parsed request b
   And the event "metaData.request.httpVersion" matches "^HTTP/\d\.\d$"
   And the event "metaData.request.params.a" equals "123"
   And the event "metaData.request.params.b" equals "456"
+  And the event "metaData.request.params.name" equals "baba"
+  And the event "metaData.request.params.favourite_letter" equals "z"
+  And the event "metaData.request.params.password" equals "[FILTERED]"
   And the event "metaData.request.referer" is null
   And the event "metaData.request.url" ends with "/unhandled?a=123&b=456"
 
@@ -86,6 +89,9 @@ Scenario: A POST request with JSON sends a report with the parsed request body a
   And the event "metaData.request.httpVersion" matches "^HTTP/\d\.\d$"
   And the event "metaData.request.params.a" equals "123"
   And the event "metaData.request.params.b" equals "456"
+  And the event "metaData.request.params.name" is null
+  And the event "metaData.request.params.favourite_letter" is null
+  And the event "metaData.request.params.password" is null
   And the event "metaData.request.referer" is null
   And the event "metaData.request.url" ends with "/unhandled?a=123&b=456"
 
@@ -172,3 +178,55 @@ Scenario: clearing feature flags for an unhandled error
   And I wait to receive an error
   Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier
   And the event has no feature flags
+
+@not-rack-1
+@not-rack-2
+Scenario: An unrewindable POST request with form data does not attach request body
+  Given I set environment variable "BUGSNAG_RACK_NO_REWIND" to "true"
+  And I start the rack service
+  When I send a POST request to "/unhandled?a=123&b=456" in the rack app with the following form data:
+    | name             | baba      |
+    | favourite_letter | z         |
+    | password         | password1 |
+  And I wait to receive an error
+  Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier
+  And the event "metaData.request.body" is null
+  And the event "metaData.request.clientIp" is not null
+  And the event "metaData.request.cookies" is null
+  And the event "metaData.request.headers.Host" is not null
+  And the event "metaData.request.headers.User-Agent" is not null
+  And the event "metaData.request.httpMethod" equals "POST"
+  And the event "metaData.request.httpVersion" matches "^HTTP/\d\.\d$"
+  And the event "metaData.request.params.a" equals "123"
+  And the event "metaData.request.params.b" equals "456"
+  And the event "metaData.request.params.name" is null
+  And the event "metaData.request.params.favourite_letter" is null
+  And the event "metaData.request.params.password" is null
+  And the event "metaData.request.referer" is null
+  And the event "metaData.request.url" ends with "/unhandled?a=123&b=456"
+
+@not-rack-1
+@not-rack-2
+Scenario: An unrewindable POST request with JSON does not attach request body
+  Given I set environment variable "BUGSNAG_RACK_NO_REWIND" to "true"
+  And I start the rack service
+  When I send a POST request to "/unhandled?a=123&b=456" in the rack app with the following JSON:
+    | name             | baba      |
+    | favourite_letter | z         |
+    | password         | password1 |
+  And I wait to receive an error
+  Then the error is valid for the error reporting API version "4.0" for the "Ruby Bugsnag Notifier" notifier
+  And the event "metaData.request.body" is null
+  And the event "metaData.request.clientIp" is not null
+  And the event "metaData.request.cookies" is null
+  And the event "metaData.request.headers.Host" is not null
+  And the event "metaData.request.headers.User-Agent" is not null
+  And the event "metaData.request.httpMethod" equals "POST"
+  And the event "metaData.request.httpVersion" matches "^HTTP/\d\.\d$"
+  And the event "metaData.request.params.a" equals "123"
+  And the event "metaData.request.params.b" equals "456"
+  And the event "metaData.request.params.name" is null
+  And the event "metaData.request.params.favourite_letter" is null
+  And the event "metaData.request.params.password" is null
+  And the event "metaData.request.referer" is null
+  And the event "metaData.request.url" ends with "/unhandled?a=123&b=456"

features/support/env.rb

@@ -63,3 +63,11 @@ def current_ip
   Maze::Runner.environment["BUGSNAG_ENDPOINT"] = "http://#{host}:#{Maze.config.port}/notify"
   Maze::Runner.environment["BUGSNAG_SESSION_ENDPOINT"] = "http://#{host}:#{Maze.config.port}/sessions"
 end
+
+Before("@not-rack-1") do
+  skip_this_scenario if ENV["RACK_VERSION"] == "1"
+end
+
+Before("@not-rack-2") do
+  skip_this_scenario if ENV["RACK_VERSION"] == "2"
+end

lib/bugsnag.rb

@@ -1,5 +1,10 @@
 require "rubygems"
 require "thread"
+require "set"
+require "json"
+require "uri"
+require "socket"
+require "logger"
 
 require "bugsnag/version"
 require "bugsnag/utility/feature_data_store"
@@ -21,21 +26,9 @@
 # as it doesn't auto-configure when loaded
 require "bugsnag/integrations/rack"
 
-require "bugsnag/middleware/rack_request"
-require "bugsnag/middleware/warden_user"
-require "bugsnag/middleware/clearance_user"
-require "bugsnag/middleware/callbacks"
-require "bugsnag/middleware/rails3_request"
-require "bugsnag/middleware/sidekiq"
-require "bugsnag/middleware/mailman"
-require "bugsnag/middleware/rake"
-require "bugsnag/middleware/classify_error"
-require "bugsnag/middleware/delayed_job"
-
 require "bugsnag/breadcrumb_type"
 require "bugsnag/breadcrumbs/validator"
 require "bugsnag/breadcrumbs/breadcrumb"
-require "bugsnag/breadcrumbs/breadcrumbs"
 
 require "bugsnag/utility/duplicator"
 require "bugsnag/utility/metadata_delegate"

lib/bugsnag/breadcrumbs/on_breadcrumb_callback_list.rb

@@ -1,5 +1,3 @@
-require "set"
-
 module Bugsnag::Breadcrumbs
   class OnBreadcrumbCallbackList
     def initialize(configuration)

lib/bugsnag/breadcrumbs/validator.rb

@@ -1,5 +1,3 @@
-require 'bugsnag/breadcrumbs/breadcrumbs'
-
 module Bugsnag::Breadcrumbs
   ##
   # Validates a given breadcrumb before it is stored

lib/bugsnag/cleaner.rb

@@ -1,5 +1,3 @@
-require 'uri'
-
 module Bugsnag
   # @api private
   class Cleaner

lib/bugsnag/configuration.rb

@@ -1,20 +1,28 @@
-require "set"
-require "socket"
-require "logger"
-require "bugsnag/middleware_stack"
+require "bugsnag/breadcrumbs/on_breadcrumb_callback_list"
+
+require "bugsnag/endpoint_configuration"
+require "bugsnag/endpoint_validator"
+
+require "bugsnag/middleware/breadcrumbs"
 require "bugsnag/middleware/callbacks"
+require "bugsnag/middleware/classify_error"
+require "bugsnag/middleware/clearance_user"
+require "bugsnag/middleware/delayed_job"
 require "bugsnag/middleware/discard_error_class"
 require "bugsnag/middleware/exception_meta_data"
 require "bugsnag/middleware/ignore_error_class"
-require "bugsnag/middleware/suggestion_data"
-require "bugsnag/middleware/classify_error"
+require "bugsnag/middleware/mailman"
+require "bugsnag/middleware/rack_request"
+require "bugsnag/middleware/rails3_request"
+require "bugsnag/middleware/rake"
 require "bugsnag/middleware/session_data"
-require "bugsnag/middleware/breadcrumbs"
+require "bugsnag/middleware/sidekiq"
+require "bugsnag/middleware/suggestion_data"
+require "bugsnag/middleware/warden_user"
+
+require "bugsnag/middleware_stack"
+
 require "bugsnag/utility/circular_buffer"
-require "bugsnag/breadcrumbs/breadcrumbs"
-require "bugsnag/breadcrumbs/on_breadcrumb_callback_list"
-require "bugsnag/endpoint_configuration"
-require "bugsnag/endpoint_validator"
 
 module Bugsnag
   class Configuration

lib/bugsnag/delivery/synchronous.rb

@@ -1,5 +1,4 @@
 require "net/https"
-require "uri"
 
 module Bugsnag
   module Delivery

lib/bugsnag/delivery/thread_queue.rb

@@ -1,5 +1,3 @@
-require "thread"
-
 module Bugsnag
   module Delivery
     class ThreadQueue < Synchronous

lib/bugsnag/event.rb

@@ -1,5 +1,3 @@
-require "bugsnag/report"
-
 module Bugsnag
   # For now Event is just an alias of Report. This points to the same object so
   # any changes to Report will also affect Event

lib/bugsnag/helpers.rb

@@ -1,8 +1,3 @@
-require 'uri'
-require 'set'
-require 'json'
-
-
 module Bugsnag
   module Helpers # rubocop:todo Metrics/ModuleLength
     MAX_STRING_LENGTH = 3072

lib/bugsnag/integrations/mongo.rb

@@ -1,5 +1,4 @@
 require 'mongo'
-require 'bugsnag/breadcrumbs/breadcrumbs'
 
 module Bugsnag
   ##

lib/bugsnag/integrations/rails/active_job.rb

@@ -1,5 +1,3 @@
-require 'set'
-
 module Bugsnag::Rails
   module ActiveJob
     SEVERITY = 'error'

lib/bugsnag/integrations/rails/rails_breadcrumbs.rb

@@ -1,5 +1,3 @@
-require "bugsnag/breadcrumbs/breadcrumbs"
-
 module Bugsnag::Rails
   DEFAULT_RAILS_BREADCRUMBS = [
     {

lib/bugsnag/integrations/railtie.rb

@@ -1,10 +1,6 @@
 # Rails 3.x hooks
 
-require "json"
 require "rails"
-require "bugsnag"
-require "bugsnag/middleware/rails3_request"
-require "bugsnag/middleware/rack_request"
 require "bugsnag/integrations/rails/rails_breadcrumbs"
 
 module Bugsnag

lib/bugsnag/integrations/rake.rb

@@ -1,4 +1,7 @@
-require 'bugsnag'
+# this file can either be required manually by a user, in which case 'bugsnag'
+# needs to be required, or it can be required automatically in the railtie,
+# in which case 'bugsnag' has already been required
+require 'bugsnag' unless defined?(Bugsnag)
 
 Rake::TaskManager.record_task_metadata = true
 

lib/bugsnag/middleware/rack_request.rb

@@ -1,5 +1,3 @@
-require "json"
-
 module Bugsnag::Middleware
   ##
   # Extracts and attaches rack data to an error report
@@ -17,7 +15,15 @@ def call(report)
 
         request = ::Rack::Request.new(env)
 
-        params = request.params rescue {}
+        params =
+          # if the request body isn't rewindable then we can't read request.POST
+          # which is used internally by request.params
+          if request.body.respond_to?(:rewind)
+            request.params rescue {}
+          else
+            request.GET rescue {}
+          end
+
         client_ip = request.ip.to_s rescue SPOOF
         session = env["rack.session"]
 
@@ -106,7 +112,11 @@ def format_headers(env, referer)
     end
 
     def add_request_body(report, request, env)
-      body = parsed_request_body(request, env)
+      begin
+        body = parsed_request_body(request, env)
+      rescue StandardError
+        return nil
+      end
 
       # this request may not have a body
       return unless body.is_a?(Hash) && !body.empty?
@@ -115,26 +125,34 @@ def add_request_body(report, request, env)
     end
 
     def parsed_request_body(request, env)
-      return request.POST rescue nil if request.form_data?
+      # if the request is not rewindable then either:
+      # - it's been read already and so is impossible to read
+      # - it hasn't been read yet and us reading it will prevent the user from
+      #   reading it themselves
+      # in either case we should avoid attempting to
+      return nil unless request.body.respond_to?(:rewind)
+
+      if request.form_data?
+        begin
+          return request.POST
+        ensure
+          request.body.rewind
+        end
+      end
 
       content_type = env["CONTENT_TYPE"]
 
       return nil if content_type.nil?
+      return nil unless content_type.include?('/json') || content_type.include?('+json')
 
-      if content_type.include?('/json') || content_type.include?('+json')
-        begin
-          body = request.body
+      begin
+        body = request.body
 
-          return JSON.parse(body.read)
-        rescue StandardError
-          return nil
-        ensure
-          # the body must be rewound so other things can read it after we do
-          body.rewind
-        end
+        JSON.parse(body.read)
+      ensure
+        # the body must be rewound so other things can read it after we do
+        body.rewind
       end
-
-      nil
     end
 
     def add_cookies(report, request)

lib/bugsnag/report.rb

@@ -1,4 +1,3 @@
-require "json"
 require "pathname"
 require "bugsnag/error"
 require "bugsnag/stacktrace"