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/87b06df5 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/87b06df5 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/87b06df5 Branch: refs/heads/1.2.x Commit: 87b06df592b4d07e5a201ae151950ab30d59e1ba Parents: 459873a Author: Anindya Sinha <[email protected]> Authored: Fri Apr 21 09:45:47 2017 -0700 Committer: Anand Mazumdar <[email protected]> Committed: Fri Apr 21 09:59:48 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/87b06df5/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/87b06df5/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()); +}
