Tt 3520 stop cloning env

CHANGELOG.md

@@ -5,6 +5,8 @@ This changelog adheres to [Keep a CHANGELOG](http://keepachangelog.com/).
 
 ## Unreleased
 ### Changed
+- [TT-3520] No longer clone the "env" middleware variable
+- [TT-3521] filter action dispatch parameter fields
 - [TT-3523] Update gem dependencies
 
 ## [0.3.0] - 2016-12-28

lib/sensitive_data_filter/middleware.rb

@@ -1,11 +1,12 @@
 # frozen_string_literal: true
 module SensitiveDataFilter
   module Middleware
+    FILTERABLE = %i(query_params body_params request_params).freeze
   end
 end
 
 require 'sensitive_data_filter/middleware/parameter_parser'
 require 'sensitive_data_filter/middleware/env_parser'
 require 'sensitive_data_filter/middleware/occurrence'
-require 'sensitive_data_filter/middleware/env_filter'
+require 'sensitive_data_filter/middleware/detect'
 require 'sensitive_data_filter/middleware/filter'

lib/sensitive_data_filter/middleware/detect.rb

@@ -0,0 +1,28 @@
+module SensitiveDataFilter
+  module Middleware
+    class Detect
+      def initialize(filter)
+        @filter = filter
+      end
+
+      def call
+        changeset = nil
+        scan = run_scan
+        if scan.matches?
+          changeset = OpenStruct.new(SensitiveDataFilter::Middleware::FILTERABLE.each_with_object({}) { |filterable, hash|
+            hash[filterable.to_s] = SensitiveDataFilter::Mask.mask(@filter.send(filterable))
+          })
+        end
+        [changeset, scan]
+      end
+
+      private
+
+      def run_scan
+        SensitiveDataFilter::Scan.new(
+          SensitiveDataFilter::Middleware::FILTERABLE.map { |filterable| @filter.send(filterable) }
+        )
+      end
+    end
+  end
+end

lib/sensitive_data_filter/middleware/env_parser.rb

@@ -6,6 +6,7 @@ module Middleware
     class EnvParser
       QUERY_STRING = 'QUERY_STRING'.freeze
       RACK_INPUT   = 'rack.input'.freeze
+      REQUEST_PARAMS = 'action_dispatch.request.request_parameters'.freeze
 
       extend Forwardable
 
@@ -28,6 +29,10 @@ def body_params
         @parameter_parser.parse(body)
       end
 
+      def request_params
+        @env[REQUEST_PARAMS]
+      end
+
       def query_params=(new_params)
         @env[QUERY_STRING] = Rack::Utils.build_query(new_params)
       end
@@ -36,13 +41,14 @@ def body_params=(new_params)
         @env[RACK_INPUT] = StringIO.new @parameter_parser.unparse(new_params)
       end
 
-      def copy
-        self.class.new(@env.clone)
+      def request_params=(new_params)
+        @env[REQUEST_PARAMS] = new_params
       end
 
-      def mask!
-        self.query_params = SensitiveDataFilter::Mask.mask(query_params)
-        self.body_params  = SensitiveDataFilter::Mask.mask(body_params)
+      def mutate(mutation)
+        SensitiveDataFilter::Middleware::FILTERABLE.each do |filterable|
+          self.send("#{filterable}=", mutation.send(filterable))
+        end
       end
 
       def_delegators :@request, :ip, :request_method, :url, :content_type, :session

lib/sensitive_data_filter/middleware/filter.rb

@@ -7,16 +7,20 @@ def initialize(app)
       end
 
       def call(env)
-        env_filter = EnvFilter.new env
-        handle_occurrence env_filter
-        @app.call env_filter.filtered_env
+        original_env = EnvParser.new(env)
+        changeset, scan = Detect.new(original_env).call
+        unless changeset.nil?
+          handle_occurrence(original_env, changeset, scan)
+          original_env.mutate(changeset)
+        end
+        @app.call(env)
       end
 
       private
 
-      def handle_occurrence(env_filter)
-        return unless env_filter.occurrence?
-        SensitiveDataFilter.handle_occurrence env_filter.occurrence
+      def handle_occurrence(filter, changeset, scan)
+        occurence = Occurrence.new(filter, changeset, scan.matches)
+        SensitiveDataFilter.handle_occurrence(occurence)
       end
     end
   end

lib/sensitive_data_filter/middleware/occurrence.rb

@@ -9,9 +9,9 @@ class Occurrence
 
       attr_reader :matches
 
-      def initialize(original_env_parser, filtered_env_parser, matches)
+      def initialize(original_env_parser, changeset, matches)
         @original_env_parser = original_env_parser
-        @filtered_env_parser = filtered_env_parser
+        @changeset           = changeset
         @matches             = matches
       end
 
@@ -28,22 +28,26 @@ def original_body_params
       end
 
       def filtered_query_params
-        @filtered_env_parser.query_params
+        @changeset.query_params
       end
 
       def filtered_body_params
-        @filtered_env_parser.body_params
+        @changeset.body_params
+      end
+
+      def changeset
+        @changeset
       end
 
       def original_env
         @original_env_parser.env
       end
 
-      def filtered_env
-        @filtered_env_parser.env
+      def url
+        SensitiveDataFilter::Mask.mask(@original_env_parser.url)
       end
 
-      def_delegators :@filtered_env_parser, :request_method, :url, :content_type, :session
+      def_delegators :@original_env_parser, :request_method, :content_type, :session
 
       def matches_count
         @matches.map { |type, matches| [type, matches.count] }.to_h

spec/sensitive_data_filter/middleware/env_parser_spec.rb

@@ -114,54 +114,43 @@
     specify { expect(env_parser.session).to eq 'session_id' => '01ab02cd' }
   end
 
-  describe '#copy' do
-    let(:masked_env_parser) { env_parser.copy }
-
-    before do
-      masked_env_parser.query_params = { id: 2 }
-      masked_env_parser.body_params = { test: 2 }
-
-      env_parser.query_params = { id: 1 }
-      env_parser.body_params = { test: 1 }
-    end
-
-    specify { expect(env_parser.query_params).to eq 'id' => '1' }
-    specify { expect(env_parser.body_params).to eq 'test' => 1 }
-
-    specify { expect(masked_env_parser.query_params).to eq 'id' => '2' }
-    specify { expect(masked_env_parser.body_params).to eq 'test' => 2 }
-  end
-
-  describe '#mask!' do
+  describe '#mutate!' do
     let(:query_params) { { 'sensitive_query' => 'sensitive_data' } }
     let(:body_params) { { 'sensitive_body' => 'sensitive_data' } }
+    let(:request_params) { { 'sensitive_request' => 'sensitive_request' } }
 
     before do
       env_parser.query_params = { sensitive_query: 'sensitive_data' }
       env_parser.body_params  = { sensitive_body: 'sensitive_data' }
+      env_parser.request_params  = { sensitive_request: 'sensitive_request' }
     end
 
-    context 'before masking' do
+    context 'before mutation' do
       specify { expect(env_parser.query_params).to eq 'sensitive_query' => 'sensitive_data' }
       specify { expect(env_parser.body_params).to eq 'sensitive_body' => 'sensitive_data' }
+      specify { expect(env_parser.request_params).to eq({ sensitive_request: 'sensitive_request' }) }
     end
 
-    context 'after masking' do
-      let(:mask) { double }
+    context 'after mutation' do
       let(:filtered_query_params) { { 'sensitive_query' => '[FILTERED]' } }
       let(:filtered_body_params) { { 'sensitive_body' => '[FILTERED]' } }
+      let(:filtered_request_params) { { 'sensitive_request' => '[FILTERED]' } }
+
+      let(:changeset) {
+        double(
+          query_params: filtered_query_params,
+          body_params: filtered_body_params,
+          request_params: filtered_request_params
+        )
+      }
 
       before do
-        stub_const 'SensitiveDataFilter::Mask', mask
-        allow(mask).to receive(:mask).with(query_params).and_return filtered_query_params
-        allow(mask).to receive(:mask).with(body_params).and_return filtered_body_params
-        env_parser.mask!
+        env_parser.mutate(changeset)
       end
 
-      specify { expect(mask).to have_received(:mask).with query_params }
-      specify { expect(mask).to have_received(:mask).with body_params }
       specify { expect(env_parser.query_params).to eq filtered_query_params }
       specify { expect(env_parser.body_params).to eq filtered_body_params }
+      specify { expect(env_parser.request_params).to eq filtered_request_params }
     end
   end
 end