Skip to content

Commit 0d6adc1

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 0d6adc1

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

app/lib/link_checker/uri_checker/http_checker.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,23 @@ def make_request(method, check_ssl: true)
276276
),
277277
)
278278
nil
279+
# Ruby net-http cannot handle responses with headers with CR/LF characters in, and such responses raise
280+
# an argument error. We want to catch these and add to the report for now, rather than continuing to raise these
281+
# as errors, which can trigger alerting. We are planning on raising this to be fixed at the net-http level which may
282+
# mean we do not have to handle these here.
283+
# Doing some fuzzy matching on the error message to try and ensure a bit of resilience, allowing for changes to the
284+
# exact error message in the exception.
285+
rescue ArgumentError => e
286+
raise e unless e.message =~ /(.*)header(.*) CR\/LF/
287+
288+
add_problem(
289+
FaradayError.new(
290+
summary: :page_unavailable,
291+
message: :page_failing_to_load,
292+
from_redirect: from_redirect?,
293+
),
294+
)
295+
nil
279296
end
280297

281298
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

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", "Page unavailable"
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)