Merge pull request #22 from DFE-Digital/733-recover-from-malformed-st…

…rings-in-user-agents

lib/dfe/analytics/event.rb

@@ -33,11 +33,11 @@ def with_type(type)
       def with_request_details(rack_request)
         @event_hash.merge!(
           request_uuid: rack_request.uuid,
-          request_user_agent: rack_request.user_agent,
+          request_user_agent: ensure_utf8(rack_request.user_agent),
           request_method: rack_request.method,
-          request_path: rack_request.path,
+          request_path: ensure_utf8(rack_request.path),
           request_query: hash_to_kv_pairs(Rack::Utils.parse_query(rack_request.query_string)),
-          request_referer: rack_request.referer,
+          request_referer: ensure_utf8(rack_request.referer),
           anonymised_user_agent_and_ip: anonymised_user_agent_and_ip(rack_request)
         )
 
@@ -118,6 +118,10 @@ def anonymised_user_agent_and_ip(rack_request)
       def anonymise(text)
         Digest::SHA2.hexdigest(text)
       end
+
+      def ensure_utf8(str)
+        str&.scrub
+      end
     end
   end
 end

spec/dfe/analytics/event_spec.rb

@@ -75,6 +75,29 @@
     end
   end
 
+  describe 'handling invalid UTF-8' do
+    it 'coerces it to valid UTF-8' do
+      invalid_string = "hello \xbf\xef hello"
+
+      request = fake_request(
+        user_agent: invalid_string
+      )
+
+      event = described_class.new
+      expect(event.with_request_details(request).as_json['request_user_agent']).not_to eq(invalid_string)
+      expect(event.with_request_details(request).as_json['request_user_agent']).to eq('hello �� hello')
+    end
+
+    it 'handles nil' do
+      request = fake_request(
+        user_agent: nil
+      )
+
+      event = described_class.new
+      expect(event.with_request_details(request).as_json['request_user_agent']).to be_nil
+    end
+  end
+
   def fake_request(overrides = {})
     attrs = {
       uuid: '123',

spec/requests/analytics_spec.rb

@@ -10,6 +10,10 @@
       include DfE::Analytics::Requests
 
       def index
+        render plain: 'Index page'
+      end
+
+      def create
         Candidate.create(
           email_address: 'a@b.com',
           first_name: 'Mr',
@@ -31,7 +35,8 @@ def index
 
   around do |ex|
     Rails.application.routes.draw do
-      get '/example/path' => 'test#index'
+      post '/example/create' => 'test#create'
+      get '/example/' => 'test#index'
     end
 
     DfE::Analytics::Testing.webmock! do
@@ -45,8 +50,8 @@ def index
   it 'works end-to-end' do
     request_event = { environment: 'test',
                       event_type: 'web_request',
-                      request_method: 'GET',
-                      request_path: '/example/path' }
+                      request_method: 'POST',
+                      request_path: '/example/create' }
     request_event_post = stub_analytics_event_submission.with(body: /web_request/)
 
     model_event = { environment: 'test',
@@ -55,7 +60,7 @@ def index
     model_event_post = stub_analytics_event_submission.with(body: /create_entity/)
 
     perform_enqueued_jobs do
-      get '/example/path'
+      post '/example/create'
     end
 
     request_uuid = nil # we'll compare this across requests
@@ -81,17 +86,31 @@ def index
     it 'uses the specified queue' do
       with_analytics_config(queue: :my_custom_queue) do
         expect do
-          get '/example/path'
-        end.to have_enqueued_job.twice.on_queue(:my_custom_queue)
+          get '/example'
+        end.to have_enqueued_job.on_queue(:my_custom_queue)
       end
     end
   end
 
   context 'when no queue is specified' do
     it 'uses the default queue' do
       expect do
-        get '/example/path'
-      end.to have_enqueued_job.twice.on_queue(:default)
+        get '/example'
+      end.to have_enqueued_job.on_queue(:default)
+    end
+  end
+
+  context 'when a non-UTF-8-encoded User Agent is supplied' do
+    it 'coerces it to UTF-8' do
+      stub_analytics_event_submission.with(body: /web_request/)
+
+      string = "\xbf\xef"
+
+      expect do
+        perform_enqueued_jobs do
+          get '/example', headers: { 'User-Agent' => string }
+        end
+      end.not_to raise_error
     end
   end
 end