Filter deep parameters, Rails 5-style.

lib/bugsnag.rb

@@ -5,6 +5,7 @@
 require "bugsnag/configuration"
 require "bugsnag/meta_data"
 require "bugsnag/notification"
+require "bugsnag/cleaner"
 require "bugsnag/helpers"
 require "bugsnag/deploy"
 

lib/bugsnag/cleaner.rb

@@ -0,0 +1,122 @@
+require 'uri'
+
+module Bugsnag
+  class Cleaner
+    ENCODING_OPTIONS = {:invalid => :replace, :undef => :replace}.freeze
+    FILTERED = '[FILTERED]'.freeze
+    RECURSION = '[RECURSION]'.freeze
+    OBJECT = '[OBJECT]'.freeze
+
+    def initialize(filters)
+      @filters = Array(filters)
+      @deep_filters = @filters.any? {|f| f.kind_of?(Regexp) && f.to_s.include?("\\.".freeze) }
+    end
+
+    def clean_object(obj)
+      traverse_object(obj, {}, nil)
+    end
+
+    def traverse_object(obj, seen, scope)
+      return nil unless obj
+
+      # Protect against recursion of recursable items
+      protection = if obj.is_a?(Hash) || obj.is_a?(Array) || obj.is_a?(Set)
+        return seen[obj] if seen[obj]
+        seen[obj] = RECURSION
+      end
+
+      value = case obj
+      when Hash
+        clean_hash = {}
+        obj.each do |k,v|
+          if filters_match_deeply?(k, scope)
+            clean_hash[k] = FILTERED
+          else
+            clean_hash[k] = traverse_object(v, seen, [scope, k].compact.join('.'))
+          end
+        end
+        clean_hash
+      when Array, Set
+        obj.map { |el| traverse_object(el, seen, scope) }.compact
+      when Numeric, TrueClass, FalseClass
+        obj
+      when String
+        clean_string(obj)
+      else
+        str = obj.to_s
+        # avoid leaking potentially sensitive data from objects' #inspect output
+        if str =~ /#<.*>/
+          OBJECT
+        else
+          clean_string(str)
+        end
+      end
+
+      seen[obj] = value if protection
+      value
+    end
+
+    def clean_string(str)
+      if defined?(str.encoding) && defined?(Encoding::UTF_8)
+        if str.encoding == Encoding::UTF_8
+          str.valid_encoding? ? str : str.encode('utf-16', ENCODING_OPTIONS).encode('utf-8')
+        else
+          str.encode('utf-8', ENCODING_OPTIONS)
+        end
+      elsif defined?(Iconv)
+        Iconv.conv('UTF-8//IGNORE', 'UTF-8', str) || str
+      else
+        str
+      end
+    end
+
+    def self.clean_object_encoding(obj)
+      new(nil).clean_object(obj)
+    end
+
+    def clean_url(url)
+      return url if @filters.empty?
+
+      uri = URI(url)
+      return url unless uri.query
+
+      query_params = uri.query.split('&').map { |pair| pair.split('=') }
+      query_params.map! do |key, val|
+        if filters_match?(key)
+          "#{key}=#{FILTERED}"
+        else
+          "#{key}=#{val}"
+        end
+      end
+
+      uri.query = query_params.join('&')
+      uri.to_s
+    end
+
+    private
+
+    def filters_match?(key)
+      str = key.to_s
+
+      @filters.any? do |f|
+        case f
+        when Regexp
+          str.match(f)
+        else
+          str.include?(f.to_s)
+        end
+      end
+    end
+
+    # If someone has a Rails filter like /^stuff\.secret/, it won't match "request.params.stuff.secret",
+    # so we try it both with and without the "request.params." bit.
+    def filters_match_deeply?(key, scope)
+      return true if filters_match?(key)
+      return false unless @deep_filters
+
+      long = [scope, key].compact.join('.')
+      short = long.sub(/^request\.params\./, '')
+      filters_match?(long) || filters_match?(short)
+    end
+  end
+end

lib/bugsnag/helpers.rb

@@ -3,98 +3,6 @@
 module Bugsnag
   module Helpers
     MAX_STRING_LENGTH = 4096
-    ENCODING_OPTIONS = {:invalid => :replace, :undef => :replace}.freeze
-
-    def self.cleanup_obj(obj, filters = nil, seen = {})
-      return nil unless obj
-
-      # Protect against recursion of recursable items
-      protection = if obj.is_a?(Hash) || obj.is_a?(Array) || obj.is_a?(Set)
-        return seen[obj] if seen[obj]
-        seen[obj] = '[RECURSION]'.freeze
-      end
-
-      value = case obj
-      when Hash
-        clean_hash = {}
-        obj.each do |k,v|
-          if filters_match?(k, filters)
-            clean_hash[k] = '[FILTERED]'.freeze
-          else
-            clean_obj = cleanup_obj(v, filters, seen)
-            clean_hash[k] = clean_obj
-          end
-        end
-        clean_hash
-      when Array, Set
-        obj.map { |el| cleanup_obj(el, filters, seen) }.compact
-      when Numeric, TrueClass, FalseClass
-        obj
-      when String
-        cleanup_string(obj)
-      else
-        str = obj.to_s
-        # avoid leaking potentially sensitive data from objects' #inspect output
-        if str =~ /#<.*>/
-          '[OBJECT]'.freeze
-        else
-          cleanup_string(str)
-        end
-      end
-
-      seen[obj] = value if protection
-      value
-    end
-
-    def self.cleanup_string(str)
-      if defined?(str.encoding) && defined?(Encoding::UTF_8)
-        if str.encoding == Encoding::UTF_8
-          str.valid_encoding? ? str : str.encode('utf-16', ENCODING_OPTIONS).encode('utf-8')
-        else
-          str.encode('utf-8', ENCODING_OPTIONS)
-        end
-      elsif defined?(Iconv)
-        Iconv.conv('UTF-8//IGNORE', 'UTF-8', str) || str
-      else
-        str
-      end
-    end
-
-    def self.cleanup_obj_encoding(obj)
-      cleanup_obj(obj, nil)
-    end
-
-    def self.filters_match?(object, filters)
-      str = object.to_s
-
-      Array(filters).any? do |f|
-        case f
-        when Regexp
-          str.match(f)
-        else
-          str.include?(f.to_s)
-        end
-      end
-    end
-
-    def self.cleanup_url(url, filters = [])
-      return url if filters.empty?
-
-      uri = URI(url)
-      return url unless uri.query
-
-      query_params = uri.query.split('&').map { |pair| pair.split('=') }
-      query_params.map! do |key, val|
-        if filters_match?(key, filters)
-          "#{key}=[FILTERED]"
-        else
-          "#{key}=#{val}"
-        end
-      end
-
-      uri.query = query_params.join('&')
-      uri.to_s
-    end
 
     def self.reduce_hash_size(hash)
       return {} unless hash.is_a?(Hash)

lib/bugsnag/middleware/rack_request.rb

@@ -23,7 +23,7 @@ def call(notification)
         # Build the clean url (hide the port if it is obvious)
         url = "#{request.scheme}://#{request.host}"
         url << ":#{request.port}" unless [80, 443].include?(request.port)
-        url << Bugsnag::Helpers.cleanup_url(request.fullpath, notification.configuration.params_filters)
+        url << Bugsnag::Cleaner.new(notification.configuration.params_filters).clean_url(request.fullpath)
 
         headers = {}
 

lib/bugsnag/middleware/rails2_request.rb

@@ -19,7 +19,7 @@ def call(notification)
         # Build the clean url
         url = "#{request.protocol}#{request.host}"
         url << ":#{request.port}" unless [80, 443].include?(request.port)
-        url << Bugsnag::Helpers.cleanup_url(request.fullpath, notification.configuration.params_filters)
+        url << Bugsnag::Cleaner.new(notification.configuration.params_filters).clean_url(request.fullpath)
 
         # Add a request tab
         notification.add_tab(:request, {

lib/bugsnag/notification.rb

@@ -250,10 +250,10 @@ def build_exception_payload
       payload_event[:device] = {:hostname => @configuration.hostname} if @configuration.hostname
 
       # cleanup character encodings
-      payload_event = Bugsnag::Helpers.cleanup_obj_encoding(payload_event)
+      payload_event = Bugsnag::Cleaner.clean_object_encoding(payload_event)
 
       # filter out sensitive values in (and cleanup encodings) metaData
-      payload_event[:metaData] = Bugsnag::Helpers.cleanup_obj(@meta_data, @configuration.params_filters)
+      payload_event[:metaData] = Bugsnag::Cleaner.new(@configuration.params_filters).clean_object(@meta_data)
       payload_event.reject! {|k,v| v.nil? }
 
       # return the payload hash

spec/cleaner_spec.rb

@@ -0,0 +1,138 @@
+# encoding: utf-8
+
+require 'spec_helper'
+
+describe Bugsnag::Cleaner do
+  subject { described_class.new(nil) }
+
+  describe "#clean_object" do
+    it "cleans up recursive hashes" do
+      a = {:a => {}}
+      a[:a][:b] = a
+      expect(subject.clean_object(a)).to eq({:a => {:b => "[RECURSION]"}})
+    end
+
+    it "cleans up recursive arrays" do
+      a = []
+      a << a
+      a << "hello"
+      expect(subject.clean_object(a)).to eq(["[RECURSION]", "hello"])
+    end
+
+    it "allows multiple copies of the same string" do
+      a = {:name => "bugsnag"}
+      a[:second] = a[:name]
+      expect(subject.clean_object(a)).to eq({:name => "bugsnag", :second => "bugsnag"})
+    end
+
+    it "allows multiple copies of the same object" do
+      a = []
+      b = ["hello"]
+      a << b; a << b
+      expect(subject.clean_object(a)).to eq([["hello"], ["hello"]])
+    end
+
+    it "cleans up UTF8 strings properly" do
+      obj = "André"
+      expect(subject.clean_object(obj)).to eq("André")
+    end
+
+    it "cleans up binary strings properly" do
+      if RUBY_VERSION > "1.9"
+        obj = "Andr\xc7\xff"
+        if obj.respond_to? :force_encoding
+          obj = obj.force_encoding('BINARY')
+        end
+        expect(subject.clean_object(obj)).to eq("Andr��")
+      end
+    end
+
+    it "cleans up strings returned from #to_s properly" do
+      if RUBY_VERSION > "1.9"
+        str = "Andr\xc7\xff"
+        if str.respond_to? :force_encoding
+          str = str.force_encoding('BINARY')
+        end
+        obj = RuntimeError.new(str)
+        expect(subject.clean_object(obj)).to eq("Andr��")
+      end
+    end
+
+    it "filters by string inclusion" do
+      expect(described_class.new(['f']).clean_object({ :foo => 'bar' })).to eq({ :foo => '[FILTERED]' })
+      expect(described_class.new(['b']).clean_object({ :foo => 'bar' })).to eq({ :foo => 'bar' })
+    end
+
+    it "filters by regular expression" do
+      expect(described_class.new([/fb?/]).clean_object({ :foo => 'bar' })).to eq({ :foo => '[FILTERED]' })
+      expect(described_class.new([/fb+/]).clean_object({ :foo => 'bar' })).to eq({ :foo => 'bar' })
+    end
+
+    it "filters deeply nested keys" do
+      params = {:foo => {:bar => "baz"}}
+      expect(described_class.new([/^foo\.bar/]).clean_object(params)).to eq({:foo => {:bar => '[FILTERED]'}})
+    end
+
+    it "filters deeply nested request parameters" do
+      params = {:request => {:params => {:foo => {:bar => "baz"}}}}
+      expect(described_class.new([/^foo\.bar/]).clean_object(params)).to eq({:request => {:params => {:foo => {:bar => '[FILTERED]'}}}})
+    end
+  end
+
+  describe "#clean_url" do
+    let(:filters) { [] }
+    subject { described_class.new(filters).clean_url(url) }
+
+    context "with no filters configured" do
+      let(:url) { "/dir/page?param1=value1&param2=value2" }
+      it { should eq "/dir/page?param1=value1&param2=value2" }
+    end
+
+    context "with no get params" do
+      let(:url) { "/dir/page" }
+      it { should eq "/dir/page" }
+    end
+
+    context "with no matching parameters" do
+      let(:filters) { ["param3"] }
+      let(:url) { "/dir/page?param1=value1&param2=value2" }
+      it { should eq "/dir/page?param1=value1&param2=value2" }
+    end
+
+    context "with a single matching parameter" do
+      let(:filters) { ["param1"] }
+      let(:url) { "/dir/page?param1=value1&param2=value2" }
+      it { should eq "/dir/page?param1=[FILTERED]&param2=value2" }
+    end
+
+    context "with partially matching parameters" do
+      let(:filters) { ["param"] }
+      let(:url) { "/dir/page?param1=value1&param2=value2&bla=yes" }
+      it { should eq "/dir/page?param1=[FILTERED]&param2=[FILTERED]&bla=yes" }
+    end
+
+    context "with multiple matching filters" do
+      let(:filters) { ["param1", "param2"] }
+      let(:url) { "/dir/page?param1=value1&param2=value2&param3=value3" }
+      it { should eq "/dir/page?param1=[FILTERED]&param2=[FILTERED]&param3=value3" }
+    end
+
+    context "with both string and regexp filters" do
+      let(:filters) { ["param1", /param2/] }
+      let(:url) { "/dir/page?param1=value1&param2=value2&param3=value3" }
+      it { should eq "/dir/page?param1=[FILTERED]&param2=[FILTERED]&param3=value3" }
+    end
+
+    context "with matching regexp filters" do
+      let(:filters) { [/\Aaccess_token\z/] }
+      let(:url) { "https://host.example/sessions?access_token=abc123" }
+      it { should eq "https://host.example/sessions?access_token=[FILTERED]" }
+    end
+
+    context "with partially-matching regexp filters" do
+      let(:filters) { [/token/] }
+      let(:url) { "https://host.example/sessions?access_token=abc123" }
+      it { should eq "https://host.example/sessions?access_token=[FILTERED]" }
+    end
+  end
+end