Handle argument errors when http request raises argument error for headers having CR/LF characters

Faraday / ruby cannot parse headers which contain CR/LF characters. As this issue appears to exist deep down in the faraday / ruby stack, we should handle the error rather than letting the application alert. This change attempts to handle only those argument errors that roughly fuzzy match the exception string 'header field value cannot include CR/LF' (with a bit of fuzziness in case the exact message changes) but still raise for other exceptions.

Author: BeckaL

app/lib/link_checker/uri_checker/http_checker.rb

@@ -89,7 +89,7 @@ def initialize(options = {})
     end
   end
 
-  class FaradayError < Error
+  class HttpCommunicationError < Error
     def initialize(options = {})
       super(suggested_fix: :determine_if_temporary, **options)
     end
@@ -238,7 +238,7 @@ def make_request(method, check_ssl: true)
       response
     rescue Faraday::ConnectionFailed
       add_problem(
-        FaradayError.new(
+        HttpCommunicationError.new(
           summary: :website_unavailable,
           message: :website_host_offline,
           from_redirect: from_redirect?,
@@ -247,7 +247,7 @@ def make_request(method, check_ssl: true)
       nil
     rescue Faraday::TimeoutError
       add_problem(
-        FaradayError.new(
+        HttpCommunicationError.new(
           summary: :website_unavailable,
           message: :page_is_not_responding,
           from_redirect: from_redirect?,
@@ -269,13 +269,30 @@ def make_request(method, check_ssl: true)
       make_request(method, check_ssl: false)
     rescue Faraday::Error
       add_problem(
-        FaradayError.new(
+        HttpCommunicationError.new(
           summary: :page_unavailable,
           message: :page_failing_to_load,
           from_redirect: from_redirect?,
         ),
       )
       nil
+    # Ruby net-http cannot handle responses with headers with CR/LF characters in, and such responses raise
+    # an argument error. We want to catch these and add to the report for now, rather than continuing to raise these
+    # as errors, which can trigger alerting. We are planning on raising this to be fixed at the net-http level which may
+    # mean we do not have to handle these here.
+    # Doing some fuzzy matching on the error message to try and ensure a bit of resilience, allowing for changes to the
+    # exact error message in the exception.
+    rescue ArgumentError => e
+      raise e unless e.message =~ /(.*)header(.*) CR\/LF/
+
+      add_problem(
+        HttpCommunicationError.new(
+          summary: :page_unavailable,
+          message: :page_contains_unparseable_headers,
+          from_redirect: from_redirect?,
+        ),
+      )
+      nil
     end
 
     def run_connection_request(method, check_ssl: true)

app/lib/link_checker/uri_checker/problem.rb

@@ -45,7 +45,7 @@ def get_string(symbol)
       UnusualUrl
       NotAvailableOnline
       NoHost
-      FaradayError
+      HttpCommunicationError
       PageNotFound
       PageRequiresLogin
       PageIsUnavailable

config/locales/en.yml

@@ -107,6 +107,10 @@ en:
     singular: This page is failing to load.
     redirect: This redirects to a page that isn't loading.
 
+  page_contains_unparseable_headers:
+    singular: This page responds with headers that we cannot parse.
+    redirect: This redirects to a page that responds with headers that we cannot parse.
+
   determine_if_temporary: Determine if this is a temporary issue or the resource is no longer available.
 
   invalid_url: Invalid URL

spec/lib/link_checker_spec.rb

@@ -142,6 +142,26 @@
       include_examples "has warnings"
     end
 
+    context "header with CR/LF character" do
+      let(:uri) { "http://www.not-gov.uk/header_with_CRLF_character" }
+      before do
+        stub_request(:get, uri)
+          .to_return(headers: { "Invalid" => "A header containing a carriage return \r character" })
+      end
+
+      include_examples "has errors"
+      include_examples "has a problem summary", "Page unavailable"
+    end
+
+    it "does not recue from other argument error" do
+      uri = "http://www.not-gov.uk/raises_argument_error"
+      error = ArgumentError.new("something that's nothing to do with headers and carriage return line feed chars")
+
+      stub_request(:get, uri).to_raise(error)
+
+      expect { described_class.new(uri).call }.to raise_error(error)
+    end
+
     context "slow response" do
       let(:uri) { "http://www.not-gov.uk/slow_response" }