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

@@ -95,6 +95,12 @@ def initialize(options = {})
     end
   end
 
+  class CRLFArgumentError < Error
+    def initialize(options = {})
+      super(summary: :page_returns_unparseable_headers, message: :site_returns_crlf_headers, **options)
+    end
+  end
+
   class HttpChecker < Checker
     def call
       if uri.host.blank?
@@ -276,6 +282,21 @@ def make_request(method, check_ssl: true)
         ),
       )
       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(
+        CRLFArgumentError.new(
+          from_redirect: from_redirect?,
+        ),
+      )
+      nil
     end
 
     def run_connection_request(method, check_ssl: true)

app/lib/link_checker/uri_checker/problem.rb

@@ -60,6 +60,7 @@ def get_string(symbol)
       PageWithRating
       PageContainsThreat
       SecurityProblem
+      CRLFArgumentError
     ].freeze
   end
 end

config/locales/en.yml

@@ -90,6 +90,12 @@ en:
 
   website_unavailable: Website unavailable
 
+  page_returns_unparseable_headers: Website returned a response header with CR/LF characters
+
+  site_returns_crlf_headers:
+    singular: The page returns headers which contain CR/LF (Windows line break) characters, which we cannot parse
+    redirect: This redirects to a web page which returns headers which contain CR/LF (Windows line break) characters, which we cannot handle
+
   website_host_offline:
     singular: The website hosting this link is offline.
     redirect: This redirects to a website that is offline.

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", "Website returned a response header with CR/LF characters"
+    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" }