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 2017/04/21 16:47:52 UTC

[1/2] mesos git commit: Marked decoder as failed when request/response parsing failed.

Repository: mesos
Updated Branches:
  refs/heads/master 316b433db -> 8d9101ec5


Marked decoder as failed when request/response parsing failed.

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


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

Branch: refs/heads/master
Commit: cfd37d0fa1067ee7b36c575e1259f6788ff53255
Parents: 316b433
Author: Anindya Sinha <an...@apple.com>
Authored: Fri Apr 21 09:44:04 2017 -0700
Committer: Anand Mazumdar <an...@apache.org>
Committed: Fri Apr 21 09:46:53 2017 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/src/decoder.hpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/cfd37d0f/3rdparty/libprocess/src/decoder.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/decoder.hpp b/3rdparty/libprocess/src/decoder.hpp
index 4c779d4..a026d16 100644
--- a/3rdparty/libprocess/src/decoder.hpp
+++ b/3rdparty/libprocess/src/decoder.hpp
@@ -205,6 +205,7 @@ private:
       http_parser_parse_url(decoder->url.data(), decoder->url.size(), 0, &url);
 
     if (parse_url != 0) {
+      decoder->failure = true;
       return parse_url;
     }
 
@@ -231,6 +232,7 @@ private:
       http::query::decode(decoder->query);
 
     if (decoded.isError()) {
+      decoder->failure = true;
       return 1;
     }
 
@@ -242,6 +244,7 @@ private:
     if (encoding.isSome() && encoding.get() == "gzip") {
       Try<std::string> decompressed = gzip::decompress(decoder->request->body);
       if (decompressed.isError()) {
+        decoder->failure = true;
         return 1;
       }
       decoder->request->body = decompressed.get();
@@ -435,7 +438,6 @@ private:
         http::Status::string(decoder->parser.status_code);
     } else {
       decoder->failure = true;
-
       return 1;
     }
 
@@ -666,7 +668,6 @@ private:
         http::Status::string(decoder->parser.status_code);
     } else {
       decoder->failure = true;
-
       return 1;
     }
 
@@ -919,6 +920,7 @@ private:
       http_parser_parse_url(decoder->url.data(), decoder->url.size(), 0, &url);
 
     if (parse_url != 0) {
+      decoder->failure = true;
       return parse_url;
     }
 
@@ -945,6 +947,7 @@ private:
       http::query::decode(decoder->query);
 
     if (decoded.isError()) {
+      decoder->failure = true;
       return 1;
     }
 
@@ -986,6 +989,7 @@ private:
         decoder->decompressor->decompress(std::string(data, length));
 
       if (decompressed.isError()) {
+        decoder->failure = true;
         return 1;
       }
 
@@ -1010,6 +1014,7 @@ private:
     if (decoder->decompressor.get() != nullptr &&
         !decoder->decompressor->finished()) {
       writer.fail("Failed to decompress body");
+      decoder->failure = true;
       return 1;
     }
 


[2/2] mesos git commit: Fixed a crash in the streaming request/response decoder.

Posted by an...@apache.org.
Fixed a crash in the streaming request/response decoder.

If the callback `on_headers_complete()` fails, the decoder's writer
object is `None()`. So, when the callback `on_message_complete()` is
called, we should not crash in that case.
Note that the `CHECK(decoder->writer)` is valid for the callback
`on_body()` is valid since this callback is called only on a success
in `on_headers_complete()` callback.

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


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

Branch: refs/heads/master
Commit: 8d9101ec5450b13e7512981e23e6dc6fa3e2b0e2
Parents: cfd37d0
Author: Anindya Sinha <an...@apple.com>
Authored: Fri Apr 21 09:45:47 2017 -0700
Committer: Anand Mazumdar <an...@apache.org>
Committed: Fri Apr 21 09:47:04 2017 -0700

----------------------------------------------------------------------
 3rdparty/libprocess/src/decoder.hpp             | 14 +++++++--
 3rdparty/libprocess/src/tests/decoder_tests.cpp | 33 ++++++++++++++++++++
 2 files changed, 45 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/8d9101ec/3rdparty/libprocess/src/decoder.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/decoder.hpp b/3rdparty/libprocess/src/decoder.hpp
index a026d16..31e8851 100644
--- a/3rdparty/libprocess/src/decoder.hpp
+++ b/3rdparty/libprocess/src/decoder.hpp
@@ -709,7 +709,12 @@ private:
   {
     StreamingResponseDecoder* decoder = (StreamingResponseDecoder*) p->data;
 
-    CHECK_SOME(decoder->writer);
+    // This can happen if the callback `on_headers_complete()` had failed
+    // earlier (e.g., due to invalid status code).
+    if (decoder->writer.isNone()) {
+      CHECK(decoder->failure);
+      return 1;
+    }
 
     http::Pipe::Writer writer = decoder->writer.get(); // Remove const.
     writer.close();
@@ -1007,7 +1012,12 @@ private:
   {
     StreamingRequestDecoder* decoder = (StreamingRequestDecoder*) p->data;
 
-    CHECK_SOME(decoder->writer);
+    // This can happen if the callback `on_headers_complete()` had failed
+    // earlier (e.g., due to invalid query parameters).
+    if (decoder->writer.isNone()) {
+      CHECK(decoder->failure);
+      return 1;
+    }
 
     http::Pipe::Writer writer = decoder->writer.get(); // Remove const.
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/8d9101ec/3rdparty/libprocess/src/tests/decoder_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/decoder_tests.cpp b/3rdparty/libprocess/src/tests/decoder_tests.cpp
index c0efef5..5742c83 100644
--- a/3rdparty/libprocess/src/tests/decoder_tests.cpp
+++ b/3rdparty/libprocess/src/tests/decoder_tests.cpp
@@ -134,6 +134,22 @@ TYPED_TEST(RequestDecoderTest, HeaderCaseInsensitive)
 }
 
 
+TYPED_TEST(RequestDecoderTest, InvalidQueryArgs)
+{
+  TypeParam decoder;
+
+  const string data =
+    "GET /path/file.json?%x=%y&key2=value2#fragment HTTP/1.1\r\n"
+    "Host: localhost\r\n"
+    "Connection: close\r\n"
+    "Accept-Encoding: compress, gzip\r\n"
+    "\r\n";
+
+  deque<http::Request*> requests = decoder.decode(data.data(), data.length());
+  EXPECT_TRUE(decoder.failed());
+}
+
+
 TEST(DecoderTest, Response)
 {
   ResponseDecoder decoder;
@@ -292,3 +308,20 @@ TEST(DecoderTest, StreamingResponseFailure)
   EXPECT_TRUE(read.isFailed());
   EXPECT_EQ("failed to decode body", read.failure());
 }
+
+
+TEST(DecoderTest, StreamingResponseInvalidHeader)
+{
+  StreamingResponseDecoder decoder;
+
+  const string headers =
+    "HTTP/1.1 999 OK\r\n"
+    "Date: Fri, 31 Dec 1999 23:59:59 GMT\r\n"
+    "Content-Type: text/plain\r\n"
+    "\r\n";
+
+  deque<http::Response*> responses =
+    decoder.decode(headers.data(), headers.length());
+
+  EXPECT_TRUE(decoder.failed());
+}