You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2019/09/30 12:52:31 UTC
[mesos] branch master updated (aed0b87 -> f1789b0)
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from aed0b87 Fixed Javascript linting and IE compatibility of the UI roles tree.
new 76fe388 Cleaned up `HTTPTest.WWWAuthenticateHeader`.
new 19280bc Fixed parsing of HTTP authentication headers.
new f1789b0 Introduced automatic lifecycle management in `DynamicLibrary`.
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
3rdparty/libprocess/include/process/http.hpp | 2 +-
3rdparty/libprocess/src/http.cpp | 93 +++++++++++++++--
3rdparty/libprocess/src/tests/http_tests.cpp | 114 +++++++++++++--------
.../stout/include/stout/posix/dynamiclibrary.hpp | 35 ++++---
4 files changed, 174 insertions(+), 70 deletions(-)
[mesos] 01/03: Cleaned up `HTTPTest.WWWAuthenticateHeader`.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 76fe3881eedfe582a8ee4644056bebe879368408
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Mon Sep 23 10:23:27 2019 +0200
Cleaned up `HTTPTest.WWWAuthenticateHeader`.
This patch removes a number of error-prone temporaries previously reused
in the test.
Review: https://reviews.apache.org/r/71533
---
3rdparty/libprocess/src/tests/http_tests.cpp | 87 ++++++++++++++--------------
1 file changed, 44 insertions(+), 43 deletions(-)
diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp b/3rdparty/libprocess/src/tests/http_tests.cpp
index 8cb5f16..b0f1272 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -1614,11 +1614,9 @@ TEST_P(HTTPTest, CaseInsensitiveHeaders)
TEST_P(HTTPTest, WWWAuthenticateHeader)
{
- http::Headers headers;
- headers["Www-Authenticate"] = "Basic realm=\"basic-realm\"";
-
Result<http::header::WWWAuthenticate> header =
- headers.get<http::header::WWWAuthenticate>();
+ http::Headers({{"Www-Authenticate", "Basic realm=\"basic-realm\""}})
+ .get<http::header::WWWAuthenticate>();
ASSERT_SOME(header);
@@ -1626,17 +1624,14 @@ TEST_P(HTTPTest, WWWAuthenticateHeader)
EXPECT_EQ(1u, header->authParam().size());
EXPECT_EQ("basic-realm", header->authParam()["realm"]);
- headers.clear();
- header = headers.get<http::header::WWWAuthenticate>();
-
- EXPECT_NONE(header);
+ EXPECT_NONE(http::Headers().get<http::header::WWWAuthenticate>());
- headers["Www-Authenticate"] =
- "Bearer realm=\"https://auth.docker.io/token\","
- "service=\"registry.docker.io\","
- "scope=\"repository:gilbertsong/inky:pull\"";
-
- header = headers.get<http::header::WWWAuthenticate>();
+ header = http::Headers(
+ {{"Www-Authenticate",
+ "Bearer realm=\"https://auth.docker.io/token\","
+ "service=\"registry.docker.io\","
+ "scope=\"repository:gilbertsong/inky:pull\""}})
+ .get<http::header::WWWAuthenticate>();
ASSERT_SOME(header);
@@ -1646,35 +1641,41 @@ TEST_P(HTTPTest, WWWAuthenticateHeader)
EXPECT_EQ("registry.docker.io", header->authParam()["service"]);
EXPECT_EQ("repository:gilbertsong/inky:pull", header->authParam()["scope"]);
- headers["Www-Authenticate"] = "";
- header = headers.get<http::header::WWWAuthenticate>();
-
- EXPECT_ERROR(header);
-
- headers["Www-Authenticate"] = " ";
- header = headers.get<http::header::WWWAuthenticate>();
-
- EXPECT_ERROR(header);
-
- headers["Www-Authenticate"] = "Digest";
- header = headers.get<http::header::WWWAuthenticate>();
-
- EXPECT_ERROR(header);
-
- headers["Www-Authenticate"] = "Digest =";
- header = headers.get<http::header::WWWAuthenticate>();
-
- EXPECT_ERROR(header);
-
- headers["Www-Authenticate"] = "Digest ,,";
- header = headers.get<http::header::WWWAuthenticate>();
-
- EXPECT_ERROR(header);
-
- headers["Www-Authenticate"] = "Digest uri=\"/dir/index.html\",qop=auth";
- header = headers.get<http::header::WWWAuthenticate>();
-
- EXPECT_ERROR(header);
+ EXPECT_ERROR(
+ http::Headers(
+ {{"Www-Authenticate",
+ ""}})
+ .get<http::header::WWWAuthenticate>());
+
+ EXPECT_ERROR(
+ http::Headers(
+ {{"Www-Authenticate",
+ " "}})
+ .get<http::header::WWWAuthenticate>());
+
+ EXPECT_ERROR(
+ http::Headers(
+ {{"Www-Authenticate",
+ "Digest"}})
+ .get<http::header::WWWAuthenticate>());
+
+ EXPECT_ERROR(
+ http::Headers(
+ {{"Www-Authenticate",
+ "Digest ="}})
+ .get<http::header::WWWAuthenticate>());
+
+ EXPECT_ERROR(
+ http::Headers(
+ {{"Www-Authenticate",
+ "Digest ,,"}})
+ .get<http::header::WWWAuthenticate>());
+
+ EXPECT_ERROR(
+ http::Headers(
+ {{"Www-Authenticate",
+ "Digest uri=\"/dir/index.html\",qop=auth"}})
+ .get<http::header::WWWAuthenticate>());
}
[mesos] 03/03: Introduced automatic lifecycle management in
`DynamicLibrary`.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit f1789b0fe5cad221b79a0bc2adfe2036cce6f33d
Author: Benjamin Bannier <bb...@apache.org>
AuthorDate: Wed Sep 25 23:01:24 2019 +0200
Introduced automatic lifecycle management in `DynamicLibrary`.
This not only simplifies our implementation of `DynamicLibrary`, but
also removes the potential for accidental file descriptor leaks.
Review: https://reviews.apache.org/r/71388/
---
.../stout/include/stout/posix/dynamiclibrary.hpp | 35 ++++++++++++----------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp b/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp
index 6a50592..2e2726e 100644
--- a/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp
+++ b/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp
@@ -15,6 +15,7 @@
#include <dlfcn.h>
+#include <memory>
#include <string>
#include <stout/nothing.hpp>
@@ -28,20 +29,17 @@
class DynamicLibrary
{
public:
- DynamicLibrary() : handle_(nullptr) { }
+ DynamicLibrary()
+ : handle_(nullptr, [](void* handle) {
+ if (handle == nullptr) {
+ return 0;
+ }
- // Since this class manages a naked handle it cannot be copy- or
- // move-constructed.
- // TODO(bbannier): Allow for move-construction.
- DynamicLibrary(const DynamicLibrary&) = delete;
- DynamicLibrary(DynamicLibrary&&) = delete;
+ return dlclose(handle);
+ })
+ {}
- virtual ~DynamicLibrary()
- {
- if (handle_ != nullptr) {
- close();
- }
- }
+ virtual ~DynamicLibrary() = default;
Try<Nothing> open(const std::string& path)
{
@@ -50,7 +48,7 @@ public:
return Error("Library already opened");
}
- handle_ = dlopen(path.c_str(), RTLD_NOW);
+ handle_.reset(dlopen(path.c_str(), RTLD_NOW));
if (handle_ == nullptr) {
return Error("Could not load library '" + path + "': " + dlerror());
@@ -67,12 +65,17 @@ public:
return Error("Could not close library; handle was already `nullptr`");
}
- if (dlclose(handle_) != 0) {
+ if (dlclose(handle_.get()) != 0) {
return Error(
"Could not close library '" +
(path_.isSome() ? path_.get() : "") + "': " + dlerror());
}
+ // Release the handle so the default `dlclose` operation is not
+ // invoked anymore as after successful explicit `dlclose` it does
+ // not point to an open shared object anymore.
+ handle_.release();
+
handle_ = nullptr;
path_ = None();
@@ -86,7 +89,7 @@ public:
"Could not get symbol '" + name + "'; library handle was `nullptr`");
}
- void* symbol = dlsym(handle_, name.c_str());
+ void* symbol = dlsym(handle_.get(), name.c_str());
if (symbol == nullptr) {
return Error(
@@ -98,7 +101,7 @@ public:
}
private:
- void* handle_;
+ std::unique_ptr<void, int (*)(void*)> handle_;
Option<std::string> path_;
};
[mesos] 02/03: Fixed parsing of HTTP authentication headers.
Posted by bb...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
bbannier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 19280bccfc9d8a02216ec7b912f59973882b74a7
Author: Benjamin Bannier <bb...@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 654bbc2..a0065db 100644
--- a/3rdparty/libprocess/include/process/http.hpp
+++ b/3rdparty/libprocess/include/process/http.hpp
@@ -434,7 +434,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 0ed41aa..b487ce2 100644
--- a/3rdparty/libprocess/src/http.cpp
+++ b/3rdparty/libprocess/src/http.cpp
@@ -637,28 +637,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] = *value;
+ 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] = *value;
}
// 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 b0f1272..1433f3d 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -1628,7 +1628,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>();
@@ -1676,6 +1676,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"]);
}