Strategy: Add --max-redirs to DEFAULT_CURL_ARGS
The default redirection maximum for `curl` is 50 but we should use something more reasonable in livecheck. It's rare but a misconfigured server with an endless redirection loop will hit the 50 redirection limit. Unfortunately, we've encountered this in the wild (e.g., the server for `getmail` and `memtester` endlessly redirects), so it's not an idle concern. This commit basically adds `--max-redirs 5` to `Livecheck::Strategy::DEFAULT_CURL_ARGS` to enforce a more reasonable redirection maximum. To be clear, the `max_iterations` logic in `#parse_curl_output` (which was previously found in `Strategy#page_content`) doesn't restrict the number of redirections that `curl` follows. At the point the `curl` output is being parsed, the requests have already been made and `max_iterations` simply restricts the number of responses `#parse_curl_output` is willing to parse. If we use `--max-redirs` and properly set `max_iterations` to `max-redirs + 1`, we shouldn't encounter the "Too many redirects" error in `#parse_curl_output`.
This commit is contained in:
parent
2722fbe30e
commit
ef5d8ed8b0
@ -35,13 +35,24 @@ module Homebrew
|
|||||||
# `Utils::Curl` method calls in {Strategy}.
|
# `Utils::Curl` method calls in {Strategy}.
|
||||||
CURL_PROCESS_TIMEOUT = CURL_MAX_TIME + 5
|
CURL_PROCESS_TIMEOUT = CURL_MAX_TIME + 5
|
||||||
|
|
||||||
|
# The maximum number of redirections that `curl` should allow.
|
||||||
|
MAX_REDIRECTIONS = 5
|
||||||
|
|
||||||
|
# This value is passed to `#parse_curl_output` to ensure that the limit
|
||||||
|
# for the number of responses it will parse corresponds to the maximum
|
||||||
|
# number of responses in this context. The `+ 1` here accounts for the
|
||||||
|
# situation where there are exactly `MAX_REDIRECTIONS` number of
|
||||||
|
# redirections, followed by a final `200 OK` response.
|
||||||
|
MAX_PARSE_ITERATIONS = MAX_REDIRECTIONS + 1
|
||||||
|
|
||||||
# Baseline `curl` arguments used in {Strategy} methods.
|
# Baseline `curl` arguments used in {Strategy} methods.
|
||||||
DEFAULT_CURL_ARGS = [
|
DEFAULT_CURL_ARGS = [
|
||||||
# Follow redirections to handle mirrors, relocations, etc.
|
# Follow redirections to handle mirrors, relocations, etc.
|
||||||
"--location",
|
"--location",
|
||||||
|
"--max-redirs", MAX_REDIRECTIONS.to_s,
|
||||||
# Avoid progress bar text, so we can reliably identify `curl` error
|
# Avoid progress bar text, so we can reliably identify `curl` error
|
||||||
# messages in output
|
# messages in output
|
||||||
"--silent",
|
"--silent"
|
||||||
].freeze
|
].freeze
|
||||||
|
|
||||||
# `curl` arguments used in `Strategy#page_headers` method.
|
# `curl` arguments used in `Strategy#page_headers` method.
|
||||||
@ -183,7 +194,7 @@ module Homebrew
|
|||||||
)
|
)
|
||||||
next unless status.success?
|
next unless status.success?
|
||||||
|
|
||||||
parsed_output = parse_curl_output(output)
|
parsed_output = parse_curl_output(output, max_iterations: MAX_PARSE_ITERATIONS)
|
||||||
parsed_output[:responses].each { |response| headers << response[:headers] }
|
parsed_output[:responses].each { |response| headers << response[:headers] }
|
||||||
break if headers.present?
|
break if headers.present?
|
||||||
end
|
end
|
||||||
@ -217,7 +228,7 @@ module Homebrew
|
|||||||
|
|
||||||
# Separate the head(s)/body and identify the final URL (after any
|
# Separate the head(s)/body and identify the final URL (after any
|
||||||
# redirections)
|
# redirections)
|
||||||
parsed_output = parse_curl_output(output)
|
parsed_output = parse_curl_output(output, max_iterations: MAX_PARSE_ITERATIONS)
|
||||||
final_url = curl_response_last_location(parsed_output[:responses], absolutize: true, base_url: url)
|
final_url = curl_response_last_location(parsed_output[:responses], absolutize: true, base_url: url)
|
||||||
|
|
||||||
data = { content: parsed_output[:body] }
|
data = { content: parsed_output[:body] }
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user