From 58e7475b6461f85d67a3ad1f4742333f17302e8b Mon Sep 17 00:00:00 2001
From: abicky <takeshi.arabiki@gmail.com>
Date: Tue, 25 Sep 2018 22:36:27 +0900
Subject: [PATCH] Convert parameter keys to ASCII-8BIT before filtering

This commit resolves https://github.com/aserafin/grape_logging/issues/53.
---
 .../loggers/filter_parameters.rb              | 18 +++++-
 .../loggers/filter_parameters_spec.rb         | 58 ++++++++++---------
 2 files changed, 48 insertions(+), 28 deletions(-)

diff --git a/lib/grape_logging/loggers/filter_parameters.rb b/lib/grape_logging/loggers/filter_parameters.rb
index 84a17b7..e3f0dd9 100644
--- 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
index 9dfbe9d..8b0a031 100644
--- a/spec/lib/grape_logging/loggers/filter_parameters_spec.rb
+++ b/spec/lib/grape_logging/loggers/filter_parameters_spec.rb
@@ -7,11 +7,12 @@
 
   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
 
@@ -19,8 +20,8 @@
     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'),
           },
         },
       })