This is an automated email from the ASF dual-hosted git repository.

asekretenko pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 38c292862cfe0911ddab0b8119380df97b4b465c
Author: Benjamin Bannier <bbann...@apache.org>
AuthorDate: Mon Sep 23 10:24:50 2019 +0200

    Fixed parsing of HTTP authentication headers.
    
    This patch adds support for quoted strings in `Www-Authenticate` headers
    and allows to use spaces when delimiting authentication attributes of
    the form `WWW-Autenticate: a=b, c=d`, both of with are allowed by
    RFC2617.
    
    Review: https://reviews.apache.org/r/71534
---
 3rdparty/libprocess/include/process/http.hpp |  2 +-
 3rdparty/libprocess/src/http.cpp             | 93 ++++++++++++++++++++++++----
 3rdparty/libprocess/src/tests/http_tests.cpp | 31 +++++++++-
 3 files changed, 113 insertions(+), 13 deletions(-)

diff --git a/3rdparty/libprocess/include/process/http.hpp 
b/3rdparty/libprocess/include/process/http.hpp
index fa66b0f..2e36419 100644
--- a/3rdparty/libprocess/include/process/http.hpp
+++ b/3rdparty/libprocess/include/process/http.hpp
@@ -431,7 +431,7 @@ public:
     : authScheme_(authScheme),
       authParam_(authParam) {}
 
-  static Try<WWWAuthenticate> create(const std::string& value);
+  static Try<WWWAuthenticate> create(const std::string& input);
 
   std::string authScheme();
   hashmap<std::string, std::string> authParam();
diff --git a/3rdparty/libprocess/src/http.cpp b/3rdparty/libprocess/src/http.cpp
index e9d4392..0726a8b 100644
--- a/3rdparty/libprocess/src/http.cpp
+++ b/3rdparty/libprocess/src/http.cpp
@@ -635,28 +635,99 @@ Future<Nothing> Pipe::Writer::readerClosed() const
 
 namespace header {
 
-Try<WWWAuthenticate> WWWAuthenticate::create(const string& value)
+Try<WWWAuthenticate> WWWAuthenticate::create(const string& input)
 {
   // Set `maxTokens` as 2 since auth-param quoted string may
   // contain space (e.g., "Basic realm="Registry Realm").
-  vector<string> tokens = strings::tokenize(value, " ", 2);
+  vector<string> tokens = strings::tokenize(input, " ", 2);
   if (tokens.size() != 2) {
-    return Error("Unexpected WWW-Authenticate header format: '" + value + "'");
+    return Error("Unexpected WWW-Authenticate header format: '" + input + "'");
   }
 
+  // Since the authentication parameters can contain quote values, we
+  // do not use `strings::split` here since the delimiter may occur in
+  // a quoted value which should not be split.
   hashmap<string, string> authParam;
-  foreach (const string& token, strings::split(tokens[1], ",")) {
-    vector<string> split = strings::split(token, "=");
-    if (split.size() != 2) {
-      return Error(
-          "Unexpected auth-param format: '" +
-          token + "' in '" + tokens[1] + "'");
-    }
+  Option<string> key, value;
+  bool inQuotes = false;
 
+  foreach (char c, tokens[1]) {
     // Auth-param values can be a quoted-string or directive values.
     // Please see section "3.2.2.4 Directive values and quoted-string":
     // https://tools.ietf.org/html/rfc2617.
-    authParam[split[0]] = strings::trim(split[1], strings::ANY, "\"");
+    //
+    // If we see a quote we know we must already be parsing `value`
+    // since `key` cannot be a quoted-string.
+    if (c != '"' && inQuotes) {
+      if (value.isNone()) {
+        return Error("Unexpected auth-param format: '" + tokens[1] + "'");
+      }
+
+      value->append({c});
+      continue;
+    }
+
+    // If we have not yet parsed `key` this character must belong to
+    // it if it is not a space, and cannot be a `,` delimiter.
+    if (key.isNone()) {
+      if (c == ',') {
+        return Error("Unexpected auth-param format: '" + tokens[1] + "'");
+      }
+
+      if (c == ' ') {
+        continue;
+      }
+
+      key = string({c});
+      continue;
+    }
+
+    // If the current character is `=` we must start parsing a new
+    // `value`. Since we have already handled `=` in quotes above we
+    // cannot already have started parsing `value`.
+    if (c == '=') {
+      if (value.isSome()) {
+        return Error("Unexpected auth-param format: '" + tokens[1] + "'");
+      }
+      value = "";
+      continue;
+    }
+
+    // If the current character is a quote, drop the
+    // character and toogle the quote state.
+    if (c == '"') {
+      inQuotes = !inQuotes;
+      continue;
+    }
+
+    // If the current character is a record delimiter and we are not
+    // in quotes, we should have parsed both a key and a value. Store
+    // them, drop the delimiter, and restart parsing.
+    if (c == ',' && !inQuotes) {
+      if (key.isNone() || value.isNone()) {
+        return Error("Unexpected auth-param format: '" + tokens[1] + "'");
+      }
+      authParam[key.get()] = value.get();
+      key = None();
+      value = None();
+
+      continue;
+    }
+
+    // If we have not started parsing `value` we are still parsing `key`.
+    if (value.isNone()) {
+      key->append({c});
+    } else {
+      value->append({c});
+    }
+  }
+
+  // Record the last parsed `(key, value)` pair.
+  if (key.isSome()) {
+    if (value.isNone() || inQuotes) {
+      return Error("Unexpected auth-param format: '" + tokens[1] + "'");
+    }
+    authParam[key.get()] = value.get();
   }
 
   // The realm directive (case-insensitive) is required for all
diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp 
b/3rdparty/libprocess/src/tests/http_tests.cpp
index 9451fed..7caaa2b 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -1580,7 +1580,7 @@ TEST_P(HTTPTest, WWWAuthenticateHeader)
 
   header = http::Headers(
       {{"Www-Authenticate",
-        "Bearer realm=\"https://auth.docker.io/token\",";
+        "Bearer realm=\"https://auth.docker.io/token\";, "
         "service=\"registry.docker.io\","
         "scope=\"repository:gilbertsong/inky:pull\""}})
     .get<http::header::WWWAuthenticate>();
@@ -1628,6 +1628,35 @@ TEST_P(HTTPTest, WWWAuthenticateHeader)
           {{"Www-Authenticate",
             "Digest uri=\"/dir/index.html\",qop=auth"}})
         .get<http::header::WWWAuthenticate>());
+
+  EXPECT_ERROR(
+      http::Headers(
+          {{"Www-Authenticate",
+            "Bearer =\"https://https://example.com\""}})
+        .get<http::header::WWWAuthenticate>());
+
+  // authParam keys cannot be quoted strings.
+  EXPECT_ERROR(
+      http::Headers(
+          {{"Www-Authenticate",
+            "Bearer \"realm\"=\"https://example.com\""}})
+        .get<http::header::WWWAuthenticate>());
+
+  // We do not incorrectly split if authParam values contain `=`
+  // delimiters inside quotes. This is a regression test for MESOS-9968.
+  header = http::Headers(
+               {{"Www-Authenticate",
+                 "Bearer realm="
+                 "\"https://nvcr.io/proxy_auth?scope=";
+                 "repository:nvidia/tensorflow:pull,push\""}})
+             .get<http::header::WWWAuthenticate>();
+
+  ASSERT_SOME(header);
+  EXPECT_EQ("Bearer", header->authScheme());
+  ASSERT_EQ(hashset<string>{"realm"}, header->authParam().keys());
+  EXPECT_EQ(
+      
"https://nvcr.io/proxy_auth?scope=repository:nvidia/tensorflow:pull,push";,
+      header->authParam()["realm"]);
 }
 
 

Reply via email to