diff options
author | David Kalnischkies <david@kalnischkies.de> | 2017-05-29 13:28:01 +0200 |
---|---|---|
committer | David Kalnischkies <david@kalnischkies.de> | 2017-06-26 23:31:15 +0200 |
commit | 42654d08c2ca1bee18b6947a39228a35c2409deb (patch) | |
tree | c4a04897c2f77018840f201e1d9d89d62b10cdb2 | |
parent | cbbf185c3c55effe47f218a07e7b1f324973a8a6 (diff) |
deal with 3xx httpcodes as required by HTTP/1.1 spec
An unknown code should be handled the same as the x00 code of this
group, but for redirections we used to treat 300 (and a few others)
as an error while unknown codes were considered redirections.
Instead we check now explicitly for the redirection codes we support for
redirecting (and add the 308 defined in RFC 7538) to avoid future
problems if new 3xx codes are added expecting certain behaviours.
Potentially strange would have been e.g. "305 Use Proxy" sending a
Location for the proxy to use – which wouldn't have worked and resulted
in an error anyhow, but probably confused users in the process.
-rw-r--r-- | methods/basehttp.cc | 24 | ||||
-rwxr-xr-x | test/integration/test-bug-602412-dequote-redirect | 4 | ||||
-rw-r--r-- | test/interactive-helper/aptwebserver.cc | 1 |
3 files changed, 15 insertions, 14 deletions
diff --git a/methods/basehttp.cc b/methods/basehttp.cc index d7d9bccd0..5eb8a8e7e 100644 --- a/methods/basehttp.cc +++ b/methods/basehttp.cc @@ -286,18 +286,18 @@ BaseHttpMethod::DealWithHeaders(FetchResult &Res, RequestState &Req) return IMS_HIT; } - /* Redirect - * - * Note that it is only OK for us to treat all redirection the same - * because we *always* use GET, not other HTTP methods. There are - * three redirection codes for which it is not appropriate that we - * redirect. Pass on those codes so the error handling kicks in. - */ - if (AllowRedirect - && (Req.Result > 300 && Req.Result < 400) - && (Req.Result != 300 // Multiple Choices - && Req.Result != 304 // Not Modified - && Req.Result != 306)) // (Not part of HTTP/1.1, reserved) + /* Note that it is only OK for us to treat all redirection the same + because we *always* use GET, not other HTTP methods. + Codes not mentioned are handled as errors later as required by the + HTTP spec to handle unknown codes the same as the x00 code. */ + constexpr unsigned int RedirectCodes[] = { + 301, // Moved Permanently + 302, // Found + 303, // See Other + 307, // Temporary Redirect + 308, // Permanent Redirect + }; + if (AllowRedirect && std::find(std::begin(RedirectCodes), std::end(RedirectCodes), Req.Result) != std::end(RedirectCodes)) { if (Req.Location.empty() == true) ; diff --git a/test/integration/test-bug-602412-dequote-redirect b/test/integration/test-bug-602412-dequote-redirect index 9c6aa3945..3db7b5ea0 100755 --- a/test/integration/test-bug-602412-dequote-redirect +++ b/test/integration/test-bug-602412-dequote-redirect @@ -30,7 +30,7 @@ Reading package lists..." aptget update testsuccess --nomsg aptget install unrelated --download-only -y } -for CODE in 301 302 307; do +for CODE in 301 302 307 308; do webserverconfig 'aptwebserver::redirect::httpcode' "$CODE" rm -f aptarchive/webserver.log.client*.log testrun "$CODE" "http://localhost:${APTHTTPPORT}" @@ -40,7 +40,7 @@ done changetohttpswebserver -for CODE in 301 302 307; do +for CODE in 301 302 307 308; do webserverconfig 'aptwebserver::redirect::httpcode' "$CODE" rm -f aptarchive/webserver.log.client*.log testrun "$CODE" "https://localhost:${APTHTTPSPORT}" diff --git a/test/interactive-helper/aptwebserver.cc b/test/interactive-helper/aptwebserver.cc index 22fc4121b..cbefe48d9 100644 --- a/test/interactive-helper/aptwebserver.cc +++ b/test/interactive-helper/aptwebserver.cc @@ -54,6 +54,7 @@ static std::string httpcodeToStr(int const httpcode) /*{{{*/ case 304: return _config->Find("aptwebserver::httpcode::304", "304 Not Modified"); case 305: return _config->Find("aptwebserver::httpcode::305", "305 Use Proxy"); case 307: return _config->Find("aptwebserver::httpcode::307", "307 Temporary Redirect"); + case 308: return _config->Find("aptwebserver::httpcode::308", "308 Permanent Redirect"); // Client errors 4xx case 400: return _config->Find("aptwebserver::httpcode::400", "400 Bad Request"); case 401: return _config->Find("aptwebserver::httpcode::401", "401 Unauthorized"); |