[2/2] mesos git commit: Added check for API endpoints not supporting streaming responses.

[al...@apache.org]

Added check for API endpoints not supporting streaming responses.

A client can request any endpoint to stream its response using
the RecordIO format. However, we never checked whether the selected
endpoint actually supports streaming, leading to an agent crash
when it was used for endpoints that did not expect such a request.

Review: https://reviews.apache.org/r/67547/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e25385a2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e25385a2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e25385a2

Branch: refs/heads/master
Commit: e25385a2f9337194164786d60a5e107140a78ae6
Parents: ee14ed1
Author: Benno Evers <be...@mesosphere.com>
Authored: Thu Jun 14 15:34:42 2018 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Thu Jun 14 15:34:42 2018 +0200

----------------------------------------------------------------------
 src/slave/http.cpp      | 15 ++++++++++++---
 src/tests/api_tests.cpp | 21 +++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e25385a2/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index a6739e1..aa9cdc5 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -466,6 +466,8 @@ Future<Response> Http::api(
 
   Option<ContentType> messageAcceptType;
   if (streamingMediaType(acceptType)) {
+    // Note that `acceptsMediaType()` returns true if the given headers
+    // field does not exist, i.e. by default we return JSON here.
     if (request.acceptsMediaType(MESSAGE_ACCEPT, APPLICATION_JSON)) {
       messageAcceptType = ContentType::JSON;
     } else if (request.acceptsMediaType(MESSAGE_ACCEPT, APPLICATION_PROTOBUF)) {
@@ -541,8 +543,8 @@ Future<Response> Http::_api(
   if (streamingMediaType(mediaTypes.content) &&
       call.type() != mesos::agent::Call::ATTACH_CONTAINER_INPUT) {
     return UnsupportedMediaType(
-        "Streaming 'Content-Type' " + stringify(mediaTypes.content) + " is not "
-        "supported for " + stringify(call.type()) + " call");
+        "Streaming 'Content-Type' " + stringify(mediaTypes.content) + " is "
+        "not supported for " + stringify(call.type()) + " call");
   } else if (!streamingMediaType(mediaTypes.content) &&
              call.type() == mesos::agent::Call::ATTACH_CONTAINER_INPUT) {
     return UnsupportedMediaType(
@@ -550,6 +552,13 @@ Future<Response> Http::_api(
         " for " + stringify(call.type()) + " call");
   }
 
+  if (streamingMediaType(mediaTypes.accept) &&
+      call.type() != mesos::agent::Call::ATTACH_CONTAINER_OUTPUT &&
+      call.type() != mesos::agent::Call::LAUNCH_NESTED_CONTAINER_SESSION) {
+    return NotAcceptable("Streaming response is not supported for " +
+        stringify(call.type()) + " call");
+  }
+
   // Each handler must log separately to add context
   // it might extract from the nested message.
   switch (call.type()) {
@@ -2480,7 +2489,7 @@ Future<Response> Http::_launchContainer(
     const Option<Resources>& resources,
     const Option<ContainerInfo>& containerInfo,
     const Option<ContainerClass>& containerClass,
-    ContentType acceptType,
+    ContentType,
     const Owned<ObjectApprovers>& approvers) const
 {
   Option<string> user;

http://git-wip-us.apache.org/repos/asf/mesos/blob/e25385a2/src/tests/api_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/api_tests.cpp b/src/tests/api_tests.cpp
index 15dbbc1..85635a8 100644
--- a/src/tests/api_tests.cpp
+++ b/src/tests/api_tests.cpp
@@ -6897,6 +6897,27 @@ TEST_F(AgentAPITest, HeaderValidation)
     AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::UnsupportedMediaType().status,
                                     response);
   }
+
+  // Setting 'Accept: application/recordio' header for a non-streaming request.
+  {
+    v1::agent::Call call;
+    call.set_type(v1::agent::Call::GET_CONTAINERS);
+
+    ContentType contentType = ContentType::JSON;
+
+    http::Headers headers = createBasicAuthHeaders(DEFAULT_CREDENTIAL);
+    headers["Accept"] = "application/recordio";
+
+    Future<http::Response> response = http::post(
+        slave.get()->pid,
+        "api/v1",
+        headers,
+        serialize(contentType, call),
+        stringify(contentType));
+
+    AWAIT_EXPECT_RESPONSE_STATUS_EQ(http::NotAcceptable().status,
+                                    response);
+  }
 }