You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2018/09/04 21:36:19 UTC
[mesos] branch master updated (ac766c8 -> 269a973)
This is an automated email from the ASF dual-hosted git repository.
andschwa pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from ac766c8 Added --skip-verify flag to verify-reviews.py to skip verification.
new 9658e8e Added /files API test for queries with encoded sequences.
new dafb03e Added regression test for `http::internal::encode(Request)`.
new 269a973 Fixed encoding behavior of `http::internal::encode(Request)`.
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/src/http.cpp | 2 +-
3rdparty/libprocess/src/tests/http_tests.cpp | 34 ++++++++++++++++++++++
src/tests/files_tests.cpp | 42 ++++++++++++++++++++++++++++
3 files changed, 77 insertions(+), 1 deletion(-)
[mesos] 01/03: Added /files API test for queries with encoded
sequences.
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
andschwa pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 9658e8e0e05c8ec9332f40f0884914ccee8f3639
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
AuthorDate: Thu Aug 16 16:10:30 2018 -0700
Added /files API test for queries with encoded sequences.
Due to a bug in the HTTP client in libprocess (MESOS-9168), the use of
percent-encoded sequences in paths did not work when using the HTTP
client in libprocess. This adds a test to ensure that the /files API
correctly handles these sequences in file paths.
Review: https://reviews.apache.org/r/68420
---
src/tests/files_tests.cpp | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/src/tests/files_tests.cpp b/src/tests/files_tests.cpp
index d092790..7d29ebb 100644
--- a/src/tests/files_tests.cpp
+++ b/src/tests/files_tests.cpp
@@ -320,6 +320,48 @@ TEST_F(FilesTest, ResolveTest)
}
+// Tests paths with percent-encoded sequences in the HTTP request
+// query. Very specifically, this checks that a percent-encoded symbol
+// is not "double decoded." That is to say, say we have a literal path
+// `foo%3Abar` because we couldn't use `:` literally on Windows, and
+// so instead encoded it literally in the file path on Windows. This
+// demonstrated a bug in libprocess where the encoding `%3A` was
+// decoded too many times, and so the query was for `foo:bar` instead
+// of literally `foo%3Abar`.
+TEST_F(FilesTest, QueryWithEncodedSequence)
+{
+ Files files;
+ process::UPID upid("files", process::address());
+
+ // This path has the ASCII escape sequence `%3A` literally instead
+ // of `:` because the latter is a reserved character on Windows.
+ //
+ // NOTE: This is not just an arbitrary character such as `+`, it is
+ // explicitly a percent-encoded sequence, but it could be e.g. `+`
+ // percent-encoded as `%2B`. Hence the assertion that this could be
+ // decoded again.
+ const string filename = "foo%3Abar";
+ ASSERT_SOME_EQ("foo:bar", process::http::decode(filename));
+
+ ASSERT_SOME(os::write(filename, "body"));
+ ASSERT_SOME_EQ("body", os::read(filename));
+ AWAIT_EXPECT_READY(files.attach(sandbox.get(), "/"));
+
+ // NOTE: The query here has to be encoded because it is a `string`
+ // and not a `hashmap<string, string>`. The latter is automatically
+ // encoded, but the former is not.
+ Future<Response> response = process::http::get(
+ upid, "read", "path=/" + process::http::encode(filename) + "&offset=0");
+
+ JSON::Object expected;
+ expected.values["offset"] = 0;
+ expected.values["data"] = "body";
+
+ AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response);
+ AWAIT_EXPECT_RESPONSE_BODY_EQ(stringify(expected), response);
+}
+
+
TEST_F(FilesTest, BrowseTest)
{
Files files;
[mesos] 03/03: Fixed encoding behavior of
`http::internal::encode(Request)`.
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
andschwa pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 269a9735031566ddda47ccb8398fd0609af3a826
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
AuthorDate: Fri Aug 17 14:46:34 2018 -0700
Fixed encoding behavior of `http::internal::encode(Request)`.
Review: https://reviews.apache.org/r/68375
---
3rdparty/libprocess/src/http.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/3rdparty/libprocess/src/http.cpp b/3rdparty/libprocess/src/http.cpp
index e9d4392..3e73ee9 100644
--- a/3rdparty/libprocess/src/http.cpp
+++ b/3rdparty/libprocess/src/http.cpp
@@ -988,7 +988,7 @@ Pipe::Reader encode(const Request& request)
vector<string> query;
foreachpair (const string& key, const string& value, request.url.query) {
- query.push_back(key + "=" + value);
+ query.push_back(http::encode(key) + "=" + http::encode(value));
}
out << "?" << strings::join("&", query);
[mesos] 02/03: Added regression test for
`http::internal::encode(Request)`.
Posted by an...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
andschwa pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit dafb03e4194624e3148200a7f919bfb150b6afb7
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
AuthorDate: Fri Aug 17 13:54:45 2018 -0700
Added regression test for `http::internal::encode(Request)`.
This function is supposed to be encoding the query parameters, but
currently fails to do so, resulting in:
```
libprocess\src\tests\http_tests.cpp(655): error: Value of: read.get()
Expected: starts with "GET /?path=C%3A%5Cfoo%5Cbar%253Abaz"
Actual: "GET /?path=C:\\foo\\bar%3Abaz HTTP/1.1\r\nConnection:
close\r\nHost: mesos.apache.org\r\nContent-Length: 0\r\n\r\n"
```
The query parameter is going over the wire unencoded.
Review: https://reviews.apache.org/r/68422
---
3rdparty/libprocess/src/tests/http_tests.cpp | 34 ++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp b/3rdparty/libprocess/src/tests/http_tests.cpp
index 89d3bb2..97aaf3e 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -94,6 +94,7 @@ using testing::DoAll;
using testing::EndsWith;
using testing::Invoke;
using testing::Return;
+using testing::StartsWith;
using testing::WithParamInterface;
namespace process {
@@ -105,6 +106,16 @@ void reinitialize(
const Option<string>& readonlyAuthenticationRealm,
const Option<string>& readwriteAuthenticationRealm);
+namespace http {
+namespace internal {
+
+// TODO(bmahler): The client's encoding logic is currently not exposed
+// in headers, so we declare it here to test it. This should be
+// exposed in headers as a library.
+Pipe::Reader encode(const Request& request);
+
+} // namespace internal {
+} // namespace http {
} // namespace process {
class HttpProcess : public Process<HttpProcess>
@@ -627,6 +638,29 @@ TEST_P(HTTPTest, EncodeAdditionalChars)
}
+TEST_P(HTTPTest, QueryEncoding)
+{
+ // This query tests a literal ASCII encoding. The special characters
+ // `:`, `\`, and `%` should each be encoded over the wire, and when
+ // decoded should literally be `C:\foo\bar%3Abaz`.
+ hashmap<string, string> query = {{"path", "C:\\foo\\bar%3Abaz"}};
+ http::URL url = http::URL("http", "mesos.apache.org", 80, "/", query);
+ EXPECT_EQ(
+ stringify(url),
+ "http://mesos.apache.org:80/?path=C%3A%5Cfoo%5Cbar%253Abaz");
+
+ http::Request request;
+ request.method = "GET";
+ request.url = url;
+
+ // This should remain fully encoded.
+ http::Pipe::Reader reader = http::internal::encode(request);
+ Future<string> read = reader.readAll();
+ AWAIT_READY(read);
+ EXPECT_THAT(read.get(), StartsWith("GET /?path=C%3A%5Cfoo%5Cbar%253Abaz"));
+}
+
+
TEST_P(HTTPTest, PathParse)
{
const string pattern = "/books/{isbn}/chapters/{chapter}";