Repository: mesos Updated Branches: refs/heads/master afdcde5fc -> d472be0ff
Fixed a bug in http::Request::acceptsEncoding. Currently parsing only compares the begining of the header making 'gzipbug' match with candidate 'gzip'. Review: https://reviews.apache.org/r/37097 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d472be0f Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d472be0f Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d472be0f Branch: refs/heads/master Commit: d472be0ff74a39a30cdb97648078bb43b74f1a50 Parents: afdcde5 Author: Isabel Jimenez <[email protected]> Authored: Tue Aug 11 11:32:35 2015 -0700 Committer: Benjamin Mahler <[email protected]> Committed: Tue Aug 11 12:09:30 2015 -0700 ---------------------------------------------------------------------- 3rdparty/libprocess/include/process/http.hpp | 7 ++- 3rdparty/libprocess/src/http.cpp | 60 ++++++++++++-------- 3rdparty/libprocess/src/tests/encoder_tests.cpp | 5 +- 3 files changed, 44 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/d472be0f/3rdparty/libprocess/include/process/http.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/include/process/http.hpp b/3rdparty/libprocess/include/process/http.hpp index de124bf..3bcca9e 100644 --- a/3rdparty/libprocess/include/process/http.hpp +++ b/3rdparty/libprocess/include/process/http.hpp @@ -116,9 +116,10 @@ struct Request bool keepAlive; - // Returns whether the encoding is considered acceptable in the request. - // TODO(bmahler): Consider this logic being in decoder.hpp, and having the - // Request contain a member variable for each popular HTTP 1.0/1.1 header. + /** + * Returns whether the encoding is considered acceptable in the + * response. See RFC 2616 section 14 for details. + */ bool acceptsEncoding(const std::string& encoding) const; }; http://git-wip-us.apache.org/repos/asf/mesos/blob/d472be0f/3rdparty/libprocess/src/http.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/http.cpp b/3rdparty/libprocess/src/http.cpp index d8de44d..859a5dc 100644 --- a/3rdparty/libprocess/src/http.cpp +++ b/3rdparty/libprocess/src/http.cpp @@ -117,26 +117,41 @@ void initialize() bool Request::acceptsEncoding(const string& encoding) const { - // See RFC 2616, section 14.3 for the details. - Option<string> accepted = headers.get("Accept-Encoding"); - - if (accepted.isNone()) { - return false; - } - - // Remove spaces and tabs for easier parsing. - accepted = strings::remove(accepted.get(), " "); - accepted = strings::remove(accepted.get(), "\t"); - accepted = strings::remove(accepted.get(), "\n"); - // From RFC 2616: + // // 1. If the content-coding is one of the content-codings listed in // the Accept-Encoding field, then it is acceptable, unless it is // accompanied by a qvalue of 0. (As defined in section 3.9, a // qvalue of 0 means "not acceptable.") + // // 2. The special "*" symbol in an Accept-Encoding field matches any // available content-coding not explicitly listed in the header // field. + // + // 3. If multiple content-codings are acceptable, then the acceptable + // content-coding with the highest non-zero qvalue is preferred. + // + // 4. The "identity" content-coding is always acceptable, unless + // specifically refused because the Accept-Encoding field includes + // "identity;q=0", or because the field includes "*;q=0" and does + // not explicitly include the "identity" content-coding. If the + // Accept-Encoding field-value is empty, then only the "identity" + // encoding is acceptable. + // + // If no Accept-Encoding field is present in a request, the server + // MAY assume that the client will accept any content coding. In + // this case, if "identity" is one of the available content-codings, + // then the server SHOULD use the "identity" content-coding... + Option<string> accept = headers.get("Accept-Encoding"); + + if (accept.isNone() || accept.get().empty()) { + return false; + } + + // Remove spaces and tabs for easier parsing. + accept = strings::remove(accept.get(), " "); + accept = strings::remove(accept.get(), "\t"); + accept = strings::remove(accept.get(), "\n"); // First we'll look for the encoding specified explicitly, then '*'. vector<string> candidates; @@ -145,11 +160,17 @@ bool Request::acceptsEncoding(const string& encoding) const foreach (const string& candidate, candidates) { // Is the candidate one of the accepted encodings? - foreach (const string& _encoding, strings::tokenize(accepted.get(), ",")) { - if (strings::startsWith(_encoding, candidate)) { + foreach (const string& encoding_, strings::tokenize(accept.get(), ",")) { + vector<string> tokens = strings::tokenize(encoding_, ";"); + + if (tokens.empty()) { + continue; + } + + if (strings::lower(tokens[0]) == strings::lower(candidate)) { // Is there a 0 q value? Ex: 'gzip;q=0.0'. const map<string, vector<string>> values = - strings::pairs(_encoding, ";", "="); + strings::pairs(encoding_, ";", "="); // Look for { "q": ["0"] }. if (values.count("q") == 0 || values.find("q")->second.size() != 1) { @@ -164,15 +185,6 @@ bool Request::acceptsEncoding(const string& encoding) const } } - // NOTE: 3 and 4 are partially ignored since we can only provide gzip. - // 3. If multiple content-codings are acceptable, then the acceptable - // content-coding with the highest non-zero qvalue is preferred. - // 4. The "identity" content-coding is always acceptable, unless - // specifically refused because the Accept-Encoding field includes - // "identity;q=0", or because the field includes "*;q=0" and does - // not explicitly include the "identity" content-coding. If the - // Accept-Encoding field-value is empty, then only the "identity" - // encoding is acceptable. return false; } http://git-wip-us.apache.org/repos/asf/mesos/blob/d472be0f/3rdparty/libprocess/src/tests/encoder_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/tests/encoder_tests.cpp b/3rdparty/libprocess/src/tests/encoder_tests.cpp index 0032137..5ad5fd4 100644 --- a/3rdparty/libprocess/src/tests/encoder_tests.cpp +++ b/3rdparty/libprocess/src/tests/encoder_tests.cpp @@ -64,7 +64,7 @@ TEST(EncoderTest, Response) TEST(EncoderTest, AcceptableEncodings) { // Create requests that do not accept gzip encoding. - vector<Request> requests(7); + vector<Request> requests(10); requests[0].headers["Accept-Encoding"] = "gzip;q=0.0,*"; requests[1].headers["Accept-Encoding"] = "compress"; requests[2].headers["Accept-Encoding"] = "compress, gzip;q=0.0"; @@ -72,6 +72,9 @@ TEST(EncoderTest, AcceptableEncodings) requests[4].headers["Accept-Encoding"] = "*;q=0.0, compress"; requests[5].headers["Accept-Encoding"] = "\n compress"; requests[6].headers["Accept-Encoding"] = "compress,\tgzip;q=0.0"; + requests[7].headers["Accept-Encoding"] = "gzipbug;q=0.1"; + requests[8].headers["Accept-Encoding"] = ""; + requests[9].headers["Accept-Encoding"] = ","; foreach (const Request& request, requests) { EXPECT_FALSE(request.acceptsEncoding("gzip"))
