Convert parameter keys to ASCII-8BIT before filtering

This commit resolves aserafin#53.
abicky · Sep 25, 2018 · 58e7475 · 58e7475
1 parent f4502d6
commit 58e7475
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 28 deletions.
diff --git a/lib/grape_logging/loggers/filter_parameters.rb b/lib/grape_logging/loggers/filter_parameters.rb
@@ -29,7 +29,23 @@ def safe_parameters(request)
       end
 
       def clean_parameters(parameters)
-        parameter_filter.filter(parameters).reject{ |key, _value| @exceptions.include?(key) }
+        original_encoding_map = build_encoding_map(parameters)
+        params = transform_key_encoding(parameters, Hash.new{ |h, _| [Encoding::ASCII_8BIT, h] })
+        cleaned_params = parameter_filter.filter(params).reject{ |key, _value| @exceptions.include?(key) }
+        transform_key_encoding(cleaned_params, original_encoding_map)
+      end
+
+      def build_encoding_map(parameters)
+        parameters.each_with_object({}) do |(k, v), h|
+          h[k.dup.force_encoding(Encoding::ASCII_8BIT)] = [k.encoding, v.is_a?(Hash) ? build_encoding_map(v) : nil]
+        end
+      end
+
+      def transform_key_encoding(parameters, encoding_map)
+        parameters.each_with_object({}) do |(k, v), h|
+          encoding, children_encoding_map = encoding_map[k]
+          h[k.dup.force_encoding(encoding)] = v.is_a?(Hash) ? transform_key_encoding(v, children_encoding_map) : v
+        end
       end
     end
   end

diff --git a/spec/lib/grape_logging/loggers/filter_parameters_spec.rb b/spec/lib/grape_logging/loggers/filter_parameters_spec.rb
@@ -7,20 +7,21 @@
 
   let(:mock_request) do
     OpenStruct.new(params: {
-      this_one: 'this one',
-      that_one: 'one',
-      two: 'two',
-      three: 'three',
-      four: 'four'
+      'this_one' => 'this one',
+      'that_one' => 'one',
+      'two' => 'two',
+      'three' => 'three',
+      'four' => 'four',
+      "\xff" => 'invalid utf8',
     })
   end
 
   let(:mock_request_with_deep_nesting) do
     deep_clone = lambda { Marshal.load Marshal.dump mock_request.params }
     OpenStruct.new(
       params: deep_clone.call.merge(
-        five: deep_clone.call.merge(
-          deep_clone.call.merge({six: {seven: 'seven', eight: 'eight', one: 'another one'}})
+        'five' => deep_clone.call.merge(
+          deep_clone.call.merge({'six' => {'seven' => 'seven', 'eight' => 'eight', 'one' => 'another one'}})
         )
       )
     )
@@ -35,31 +36,34 @@
   shared_examples 'filtering' do
     it 'filters out sensitive parameters' do
       expect(subject.parameters(mock_request, nil)).to eq(params: {
-        this_one: subject.instance_variable_get('@replacement'),
-        that_one: subject.instance_variable_get('@replacement'),
-        two: 'two',
-        three: 'three',
-        four: subject.instance_variable_get('@replacement'),
+        'this_one' => subject.instance_variable_get('@replacement'),
+        'that_one' => subject.instance_variable_get('@replacement'),
+        'two' => 'two',
+        'three' => 'three',
+        'four' => subject.instance_variable_get('@replacement'),
+        "\xff" => 'invalid utf8',
       })
     end
 
     it 'deeply filters out sensitive parameters' do
       expect(subject.parameters(mock_request_with_deep_nesting, nil)).to eq(params: {
-        this_one: subject.instance_variable_get('@replacement'),
-        that_one: subject.instance_variable_get('@replacement'),
-        two: 'two',
-        three: 'three',
-        four: subject.instance_variable_get('@replacement'),
-        five: {
-          this_one: subject.instance_variable_get('@replacement'),
-          that_one: subject.instance_variable_get('@replacement'),
-          two: 'two',
-          three: 'three',
-          four: subject.instance_variable_get('@replacement'),
-          six: {
-            seven: 'seven',
-            eight: 'eight',
-            one: subject.instance_variable_get('@replacement'),
+        'this_one' => subject.instance_variable_get('@replacement'),
+        'that_one' => subject.instance_variable_get('@replacement'),
+        'two' => 'two',
+        'three' => 'three',
+        'four' => subject.instance_variable_get('@replacement'),
+        "\xff" => 'invalid utf8',
+        'five' => {
+          'this_one' => subject.instance_variable_get('@replacement'),
+          'that_one' => subject.instance_variable_get('@replacement'),
+          'two' => 'two',
+          'three' => 'three',
+          'four' => subject.instance_variable_get('@replacement'),
+          "\xff" => 'invalid utf8',
+          'six' => {
+            'seven' => 'seven',
+            'eight' => 'eight',
+            'one' => subject.instance_variable_get('@replacement'),
           },
         },
       })