Skip to content

Commit e0dbfb4

Browse files
committed
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.
1 parent 5b6bd27 commit e0dbfb4

File tree

4 files changed

+42
-0
lines changed

4 files changed

+42
-0
lines changed

app/lib/link_checker/uri_checker/http_checker.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ def initialize(options = {})
9595
end
9696
end
9797

98+
class CRLFArgumentError < Error
99+
def initialize(options = {})
100+
super(summary: :page_returns_unparseable_headers, message: :site_returns_crlf_headers, **options)
101+
end
102+
end
103+
98104
class HttpChecker < Checker
99105
def call
100106
if uri.host.blank?
@@ -276,6 +282,15 @@ def make_request(method, check_ssl: true)
276282
),
277283
)
278284
nil
285+
rescue ArgumentError => e
286+
raise e unless e.message =~ /(.*)header(.*) CR\/LF/
287+
288+
add_problem(
289+
CRLFArgumentError.new(
290+
from_redirect: from_redirect?,
291+
),
292+
)
293+
nil
279294
end
280295

281296
def run_connection_request(method, check_ssl: true)

app/lib/link_checker/uri_checker/problem.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ def get_string(symbol)
6060
PageWithRating
6161
PageContainsThreat
6262
SecurityProblem
63+
CRLFArgumentError
6364
].freeze
6465
end
6566
end

config/locales/en.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ en:
9090

9191
website_unavailable: Website unavailable
9292

93+
page_returns_unparseable_headers: Website returned a response header with CR/LF characters
94+
95+
site_returns_crlf_headers:
96+
singular: The page returns headers which contain CR/LF (Windows line break) characters, which we cannot parse
97+
redirect: This redirects to a web page which returns headers which contain CR/LF (Windows line break) characters, which we cannot handle
98+
9399
website_host_offline:
94100
singular: The website hosting this link is offline.
95101
redirect: This redirects to a website that is offline.

spec/lib/link_checker_spec.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,26 @@
142142
include_examples "has warnings"
143143
end
144144

145+
context "header with CR/LF character" do
146+
let(:uri) { "http://www.not-gov.uk/header_with_CRLF_character" }
147+
before do
148+
stub_request(:get, uri)
149+
.to_return(headers: { "Invalid" => "A header containing a carriage return \r character" })
150+
end
151+
152+
include_examples "has errors"
153+
include_examples "has a problem summary", "Website returned a response header with CR/LF characters"
154+
end
155+
156+
it "does not recue from other argument error" do
157+
uri = "http://www.not-gov.uk/raises_argument_error"
158+
error = ArgumentError.new("something that's nothing to do with headers and carriage return line feed chars")
159+
160+
stub_request(:get, uri).to_raise(error)
161+
162+
expect { described_class.new(uri).call }.to raise_error(error)
163+
end
164+
145165
context "slow response" do
146166
let(:uri) { "http://www.not-gov.uk/slow_response" }
147167

0 commit comments

Comments
 (0)