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?
@@ -230,12 +236,24 @@ def make_request(method, check_ssl: true)
       if REDIRECT_STATUS_CODES.include?(response.status) && response.headers.include?("location") && !report.has_errors?
         target_uri = uri + response.headers["location"]
         subreport = ValidUriChecker
-          .new(target_uri.to_s, redirect_history: redirect_history + [uri], http_client: http_client)
-          .call
+                      .new(target_uri.to_s, redirect_history: redirect_history + [uri], http_client: http_client)
+                      .call
         report.merge(subreport)
       end
 
       response
+    rescue ArgumentError => e
+      case e.message
+      when /header (.*) CR\/LF/
+        add_problem(
+          CRLFArgumentError.new(
+            from_redirect: from_redirect?,
+          ),
+        )
+        nil
+      else
+        raise e
+      end
     rescue Faraday::ConnectionFailed
       add_problem(
         FaradayError.new(

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_raise(ArgumentError.new("header field value cannot include CR/LF"))
+      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" }
 
@@ -285,7 +305,7 @@
         subject
 
         expect(WebMock).to have_requested(:get, uri)
-          .with(headers: { "Rate-Limit-Token": Rails.application.secrets.govuk_rate_limit_token, "Accept-Encoding": "none" })
+                             .with(headers: { "Rate-Limit-Token": Rails.application.secrets.govuk_rate_limit_token, "Accept-Encoding": "none" })
       end
     end