Rescue exceptions raised when calling to_s while sanitizing (#361)

lib/bugsnag/cleaner.rb

@@ -6,6 +6,7 @@ class Cleaner
     FILTERED = '[FILTERED]'.freeze
     RECURSION = '[RECURSION]'.freeze
     OBJECT = '[OBJECT]'.freeze
+    RAISED = '[RAISED]'.freeze
 
     def initialize(filters)
       @filters = Array(filters)
@@ -43,7 +44,7 @@ def traverse_object(obj, seen, scope)
       when String
         clean_string(obj)
       else
-        str = obj.to_s
+        str = obj.to_s rescue RAISED
         # avoid leaking potentially sensitive data from objects' #inspect output
         if str =~ /#<.*>/
           OBJECT

lib/bugsnag/middleware/rack_request.rb

@@ -1,5 +1,7 @@
 module Bugsnag::Middleware
   class RackRequest
+    SPOOF = "[SPOOF]".freeze
+
     def initialize(bugsnag)
       @bugsnag = bugsnag
     end
@@ -11,7 +13,7 @@ def call(notification)
         request = ::Rack::Request.new(env)
 
         params = request.params rescue {}
-
+        client_ip = request.ip.to_s rescue SPOOF
         session = env["rack.session"]
 
         # Set the context
@@ -51,7 +53,7 @@ def call(notification)
           :httpMethod => request.request_method,
           :params => params.to_hash,
           :referer => request.referer,
-          :clientIp => request.ip,
+          :clientIp => client_ip,
           :headers => headers
         })
 

lib/bugsnag/middleware/rails3_request.rb

@@ -1,5 +1,7 @@
 module Bugsnag::Middleware
   class Rails3Request
+    SPOOF = "[SPOOF]".freeze
+
     def initialize(bugsnag)
       @bugsnag = bugsnag
     end
@@ -8,6 +10,7 @@ def call(notification)
       if notification.request_data[:rack_env]
         env = notification.request_data[:rack_env]
         params = env["action_dispatch.request.parameters"]
+        client_ip = env["action_dispatch.remote_ip"].to_s rescue SPOOF
 
         if params
           # Set the context
@@ -22,11 +25,11 @@ def call(notification)
 
         # Use action_dispatch.remote_ip for IP address fields and send request id
         notification.add_tab(:request, {
-          :clientIp => env["action_dispatch.remote_ip"],
+          :clientIp => client_ip,
           :requestId => env["action_dispatch.request_id"]
         })
 
-        notification.user_id = env["action_dispatch.remote_ip"]
+        notification.user_id = client_ip
 
         # Add the rails version
         if notification.configuration.send_environment

spec/cleaner_spec.rb

@@ -82,6 +82,15 @@
       params = {:request => {:params => {:foo => {:bar => "baz"}}}}
       expect(described_class.new([/^foo\.bar/]).clean_object(params)).to eq({:request => {:params => {:foo => {:bar => '[FILTERED]'}}}})
     end
+
+    it "filters objects which can't be stringified" do
+      class StringRaiser
+        def to_s
+          raise 'Oh no you do not!'
+        end
+      end
+      expect(subject.clean_object({ :foo => StringRaiser.new })).to eq({ :foo => '[RAISED]' })
+    end
   end
 
   describe "#clean_url" do

spec/helper_spec.rb

@@ -39,6 +39,19 @@
       expect(value[4]).to be_a(String)
     end
 
+    context "an object will throw if `to_s` is called" do
+      class StringRaiser
+        def to_s
+          raise 'Oh no you do not!'
+        end
+      end
+
+      it "uses the string '[RAISED]' instead" do
+        value = Bugsnag::Helpers.trim_if_needed([1, 3, StringRaiser.new])
+        expect(value[2]).to eq "[RAISED]"
+      end
+    end
+
     context "payload length is less than allowed" do
 
       it "does not change strings" do

spec/rails3_request_spec.rb

@@ -0,0 +1,52 @@
+require 'spec_helper'
+
+describe Bugsnag::Middleware::Rails3Request do
+  before(:each) do
+    Bugsnag.configuration.middleware.use(described_class)
+  end
+
+  describe "#call" do
+    it "sets request metadata" do
+      Bugsnag.configuration.set_request_data(:rack_env, {
+        "action_dispatch.remote_ip" => "10.2.2.224",
+        "action_dispatch.request_id" => "5",
+      })
+      Bugsnag.notify(BugsnagTestException.new('Grimbles'))
+
+      expect(Bugsnag).to have_sent_notification { |payload|
+        event = get_event_from_payload(payload)
+        puts event["metaData"].inspect
+        expect(event["metaData"]["request"]).to eq({
+          "clientIp" => "10.2.2.224",
+          "requestId" => "5"
+        })
+      }
+    end
+
+    context "the Remote IP will throw when serialized" do
+
+      it "sets the client IP metdata to [SPOOF]" do
+        class SpecialIP
+          def to_s
+            raise BugsnagTestException.new('oh no')
+          end
+        end
+        Bugsnag.configuration.set_request_data(:rack_env, {
+          "action_dispatch.remote_ip" => SpecialIP.new,
+          "action_dispatch.request_id" => "5",
+        })
+
+        Bugsnag.notify(BugsnagTestException.new('Grimbles'))
+
+        expect(Bugsnag).to have_sent_notification { |payload|
+          event = get_event_from_payload(payload)
+          puts event["metaData"].inspect
+          expect(event["metaData"]["request"]).to eq({
+            "clientIp" => "[SPOOF]",
+            "requestId" => "5"
+          })
+        }
+      end
+    end
+  end
+end