Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions app/lib/link_checker/uri_checker/http_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?,
Expand All @@ -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?,
Expand All @@ -269,9 +269,26 @@ 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,
message: :technical_error_on_page,
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: :technical_error_on_page,
from_redirect: from_redirect?,
),
)
Expand Down
2 changes: 1 addition & 1 deletion app/lib/link_checker/uri_checker/problem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def get_string(symbol)
UnusualUrl
NotAvailableOnline
NoHost
FaradayError
HttpCommunicationError
PageNotFound
PageRequiresLogin
PageIsUnavailable
Expand Down
6 changes: 3 additions & 3 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ en:
singular: This page has a security problem that users may be alerted to.
redirect: This redirects to a page with a security problem.

page_failing_to_load:
singular: This page is failing to load.
redirect: This redirects to a page that isn't loading.
technical_error_on_page:
singular: This page has a technical error.
redirect: This redirects to a page that has a technical error.

determine_if_temporary: Determine if this is a temporary issue or the resource is no longer available.

Expand Down
20 changes: 20 additions & 0 deletions spec/lib/link_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

Expand Down