diff --git a/src/json.cc b/src/json.cc index 7053788e..1ad7b660 100644 --- a/src/json.cc +++ b/src/json.cc @@ -209,30 +209,17 @@ namespace { class Context { public: virtual void AddValue(std::unique_ptr value) = 0; - Context* parent() { return parent_; } + std::unique_ptr parent() { return std::move(parent_); } virtual ~Context() = default; protected: - Context(Context* parent) : parent_(parent) {} + Context(std::unique_ptr parent) : parent_(std::move(parent)) {} private: - Context* parent_; -}; - -class TopLevelContext : public Context { - public: - TopLevelContext() : Context(nullptr) {} - void AddValue(std::unique_ptr value) override { - values_.emplace_back(std::move(value)); - } - std::vector> values() { - return std::move(values_); - } - private: - std::vector> values_; + std::unique_ptr parent_; }; class ArrayContext : public Context { public: - ArrayContext(Context* parent) : Context(parent) {} + ArrayContext(std::unique_ptr parent) : Context(std::move(parent)) {} void AddValue(std::unique_ptr value) override { elements.emplace_back(std::move(value)); } @@ -241,7 +228,7 @@ class ArrayContext : public Context { class ObjectContext : public Context { public: - ObjectContext(Context* parent) : Context(parent) {} + ObjectContext(std::unique_ptr parent) : Context(std::move(parent)) {} void NewField(const std::string& name) { if (field_name_ != nullptr) { std::cerr << "Replacing " << *field_name_ @@ -276,48 +263,48 @@ class CallbackContext : public Context { // A builder context that allows building up a JSON object. class JSONBuilder { public: - JSONBuilder() : context_(new TopLevelContext()) {} JSONBuilder(std::function)> callback) : context_(new CallbackContext(callback)) {} - ~JSONBuilder() { delete context_; } + ~JSONBuilder() = default; void AddValue(std::unique_ptr value) { context_->AddValue(std::move(value)); } void PushArray() { - context_ = new ArrayContext(context_); + context_.reset(new ArrayContext(std::move(context_))); } void PushObject() { - context_ = new ObjectContext(context_); + context_.reset(new ObjectContext(std::move(context_))); } std::unique_ptr PopArray() { - std::unique_ptr array_context( - dynamic_cast(context_)); + ArrayContext* array_context = + dynamic_cast(context_.get()); if (array_context == nullptr) { std::cerr << "Not in array context" << std::endl; return nullptr; } - context_ = context_->parent(); - return array_context; + context_ = context_.release()->parent(); + return std::unique_ptr(array_context); } std::unique_ptr PopObject() { - std::unique_ptr object_context( - dynamic_cast(context_)); + ObjectContext* object_context = + dynamic_cast(context_.get()); if (object_context == nullptr) { std::cerr << "Not in object context" << std::endl; return nullptr; } - context_ = context_->parent(); - return object_context; + context_ = context_.release()->parent(); + return std::unique_ptr(object_context); } // Objects only. bool NewField(const std::string& name) { - ObjectContext* object_context = dynamic_cast(context_); + ObjectContext* object_context = + dynamic_cast(context_.get()); if (object_context == nullptr) { std::cerr << "NewField " << name << " outside of object" << std::endl; return false; @@ -326,17 +313,8 @@ class JSONBuilder { return true; } - // Top-level context only. - std::vector> values() throw(Exception) { - TopLevelContext* top_level = dynamic_cast(context_); - if (top_level == nullptr) { - throw Exception("values() called for an inner context"); - } - return top_level->values(); - } - private: - Context* context_; + std::unique_ptr context_; }; int handle_null(void* arg) { @@ -414,7 +392,7 @@ int handle_end_map(void* arg) { return 1; } -yajl_callbacks callbacks = { +const yajl_callbacks callbacks = { .yajl_null = &handle_null, .yajl_boolean = &handle_bool, .yajl_integer = &handle_integer, @@ -428,38 +406,51 @@ yajl_callbacks callbacks = { .yajl_end_array = &handle_end_array, }; +class YajlHandle { + public: + YajlHandle(JSONBuilder* builder) + : handle_(yajl_alloc(&callbacks, NULL, builder)) { + yajl_config(handle_, yajl_allow_comments, 1); + yajl_config(handle_, yajl_allow_multiple_values, 1); + //yajl_config(handle_, yajl_allow_partial_values, 1); + //yajl_config(handle_, yajl_allow_trailing_garbage, 1); + //yajl_config(handle_, yajl_dont_validate_strings, 1); + } + ~YajlHandle() { + yajl_free(handle_); + } + operator yajl_handle() { return handle_; } + private: + yajl_handle handle_; +}; + +class YajlError { + public: + YajlError(yajl_handle handle, bool verbose, + const unsigned char* json_text, size_t json_len) + : handle_(handle), + str_(yajl_get_error(handle_, (int)verbose, json_text, json_len)) {} + ~YajlError() { + yajl_free_error(handle_, str_); + } + const char* c_str() { return reinterpret_cast(str_); } + private: + yajl_handle handle_; + unsigned char* str_; +}; + } // namespace std::vector> Parser::AllFromStream(std::istream& stream) throw(Exception) { - JSONBuilder builder; - - const int kMax = 65536; - unsigned char data[kMax]; - yajl_handle handle = yajl_alloc(&callbacks, NULL, (void*) &builder); - yajl_config(handle, yajl_allow_comments, 1); - yajl_config(handle, yajl_allow_multiple_values, 1); - //yajl_config(handle, yajl_allow_trailing_garbage, 1); - //yajl_config(handle, yajl_dont_validate_strings, 1); - - while (!stream.eof()) { - stream.read(reinterpret_cast(&data[0]), kMax); - size_t count = stream.gcount(); - yajl_parse(handle, data, count); - } - - yajl_status stat = yajl_complete_parse(handle); - - if (stat != yajl_status_ok) { - unsigned char* str = yajl_get_error(handle, 1, data, kMax); - std::string error_str((const char*)str); - yajl_free_error(handle, str); - throw Exception(error_str); - } - - yajl_free(handle); - return builder.values(); + std::vector> values; + Parser p([&values](std::unique_ptr r){ + values.emplace_back(std::move(r)); + }); + p.ParseStream(stream); + p.NotifyEOF(); + return values; } std::unique_ptr Parser::FromStream(std::istream& stream) @@ -480,22 +471,19 @@ std::unique_ptr Parser::FromStream(std::istream& stream) std::vector> Parser::AllFromString( const std::string& input) throw(Exception) { - std::istringstream stream(input); - return AllFromStream(stream); + return AllFromStream(std::istringstream(input)); } std::unique_ptr Parser::FromString(const std::string& input) throw(Exception) { - std::istringstream stream(input); - return FromStream(stream); + return FromStream(std::istringstream(input)); } class Parser::ParseState { public: ParseState(std::function)> callback) - : builder_(callback), - handle_(yajl_alloc(&callbacks, NULL, (void*) &builder_)) { + : builder_(callback), handle_(&builder_) { yajl_config(handle_, yajl_allow_comments, 1); yajl_config(handle_, yajl_allow_multiple_values, 1); //yajl_config(handle_, yajl_allow_partial_values, 1); @@ -503,34 +491,31 @@ class Parser::ParseState { //yajl_config(handle_, yajl_dont_validate_strings, 1); } - ~ParseState() { + void Done() throw(Exception) { yajl_status stat = yajl_complete_parse(handle_); if (stat != yajl_status_ok) { - unsigned char* str = yajl_get_error(handle_, 0, nullptr, 0); - std::string error_str((const char*)str); - yajl_free_error(handle_, str); - throw Exception(error_str); + YajlError err(handle_, 0, nullptr, 0); + throw Exception(err.c_str()); } - yajl_free(handle_); } - yajl_handle& handle() { return handle_; } + yajl_handle handle() { return handle_; } private: JSONBuilder builder_; - yajl_handle handle_; + YajlHandle handle_; }; Parser::Parser(std::function)> callback) : state_(new ParseState(callback)) {} -Parser::~Parser() {} +Parser::~Parser() = default; std::size_t Parser::ParseStream(std::istream& stream) throw(Exception) { const int kMax = 65536; unsigned char data[kMax]; size_t total_bytes_consumed = 0; - yajl_handle& handle = state_->handle(); + yajl_handle handle = state_->handle(); while (!stream.eof()) { stream.read(reinterpret_cast(&data[0]), kMax); @@ -538,10 +523,8 @@ std::size_t Parser::ParseStream(std::istream& stream) throw(Exception) { yajl_status stat = yajl_parse(handle, data, count); if (stat != yajl_status_ok) { - unsigned char* str = yajl_get_error(handle, 1, data, kMax); - std::string error_str((const char*)str); - yajl_free_error(handle, str); - throw Exception(error_str); + YajlError err(handle, 1, data, kMax); + throw Exception(err.c_str()); } total_bytes_consumed += yajl_get_bytes_consumed(handle); @@ -550,4 +533,8 @@ std::size_t Parser::ParseStream(std::istream& stream) throw(Exception) { return total_bytes_consumed; } +void Parser::NotifyEOF() throw(Exception) { + state_->Done(); +} + } // json diff --git a/src/json.h b/src/json.h index b55d2a51..d3be11bc 100644 --- a/src/json.h +++ b/src/json.h @@ -346,7 +346,18 @@ class Parser { throw(Exception); size_t ParseStream(std::istream& stream) throw(Exception); + // Notifies the parser that no more data is available. + void NotifyEOF() throw(Exception); + // Used to accept inline construction of streams. + static std::vector> AllFromStream( + std::istream&& stream) throw(Exception) { + return AllFromStream(stream); + } + static std::unique_ptr FromStream(std::istream&& stream) + throw(Exception) { + return FromStream(stream); + } size_t ParseStream(std::istream&& stream) throw(Exception) { return ParseStream(stream); } diff --git a/src/kubernetes.cc b/src/kubernetes.cc index e1fe00bb..4fc3bc84 100644 --- a/src/kubernetes.cc +++ b/src/kubernetes.cc @@ -617,10 +617,10 @@ struct Watcher { const boost::system::error_code& error) { const std::string body(std::begin(range), std::end(range)); if (!body.empty()) { - try { #ifdef VERBOSE LOG(DEBUG) << name_ << " => Parsing '" << body << "'"; #endif + try { std::istringstream input(body); event_parser_.ParseStream(input); } catch (const json::Exception& e) { @@ -636,6 +636,11 @@ struct Watcher { #ifdef VERBOSE LOG(DEBUG) << name_ << " => Watch callback: EOF"; #endif + try { + event_parser_.NotifyEOF(); + } catch (const json::Exception& e) { + LOG(ERROR) << "Error while processing last event: " << e.what(); + } } else { LOG(ERROR) << name_ << " => Callback got error " << error; } diff --git a/test/json_unittest.cc b/test/json_unittest.cc index f1028217..6ceed457 100644 --- a/test/json_unittest.cc +++ b/test/json_unittest.cc @@ -370,6 +370,201 @@ TEST(TrivialToStringTest, ObjectOneField) { } +// Parse methods. + +TEST(AllFromStreamTest, SingleObject) { + GuardJsonException([](){ + std::vector v = json::Parser::AllFromStream(std::istringstream( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + )); + EXPECT_EQ(1, v.size()); + EXPECT_TOSTRING_EQ( + "{" + "\"bar\":{\"x\":0.0,\"y\":null}," + "\"baz\":true," + "\"foo\":[1.0,2.0,3.0]," + "\"str\":\"asdfasdf\"" + "}", + v[0]); + }); +} + +TEST(AllFromStreamTest, MultipleObjects) { + GuardJsonException([](){ + std::vector v = json::Parser::AllFromStream(std::istringstream( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + "{\n" + " \"foo1\": [1, 2, 3],\n" + " \"bar1\": {\"x\": 0, \"y\": null},\n" + " \"baz1\": true,\n" + " \"str1\": \"asdfasdf\"\n" + "}\n" + )); + EXPECT_EQ(2, v.size()); + EXPECT_TOSTRING_EQ( + "{" + "\"bar\":{\"x\":0.0,\"y\":null}," + "\"baz\":true," + "\"foo\":[1.0,2.0,3.0]," + "\"str\":\"asdfasdf\"" + "}", + v[0]); + EXPECT_TOSTRING_EQ( + "{" + "\"bar1\":{\"x\":0.0,\"y\":null}," + "\"baz1\":true," + "\"foo1\":[1.0,2.0,3.0]," + "\"str1\":\"asdfasdf\"" + "}", + v[1]); + }); +} + +TEST(AllFromStringTest, SingleObject) { + GuardJsonException([](){ + std::vector v = json::Parser::AllFromString( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + ); + EXPECT_EQ(1, v.size()); + EXPECT_TOSTRING_EQ( + "{" + "\"bar\":{\"x\":0.0,\"y\":null}," + "\"baz\":true," + "\"foo\":[1.0,2.0,3.0]," + "\"str\":\"asdfasdf\"" + "}", + v[0]); + }); +} + +TEST(AllFromStringTest, MultipleObjects) { + GuardJsonException([](){ + std::vector v = json::Parser::AllFromString( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + "{\n" + " \"foo1\": [1, 2, 3],\n" + " \"bar1\": {\"x\": 0, \"y\": null},\n" + " \"baz1\": true,\n" + " \"str1\": \"asdfasdf\"\n" + "}\n" + ); + EXPECT_EQ(2, v.size()); + EXPECT_TOSTRING_EQ( + "{" + "\"bar\":{\"x\":0.0,\"y\":null}," + "\"baz\":true," + "\"foo\":[1.0,2.0,3.0]," + "\"str\":\"asdfasdf\"" + "}", + v[0]); + EXPECT_TOSTRING_EQ( + "{" + "\"bar1\":{\"x\":0.0,\"y\":null}," + "\"baz1\":true," + "\"foo1\":[1.0,2.0,3.0]," + "\"str1\":\"asdfasdf\"" + "}", + v[1]); + }); +} + +TEST(FromStreamTest, SingleObject) { + GuardJsonException([](){ + json::value v = json::Parser::FromStream(std::istringstream( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + )); + EXPECT_TOSTRING_EQ( + "{" + "\"bar\":{\"x\":0.0,\"y\":null}," + "\"baz\":true," + "\"foo\":[1.0,2.0,3.0]," + "\"str\":\"asdfasdf\"" + "}", + v); + }); +} + +TEST(FromStreamTest, MultipleObjectsThrows) { + EXPECT_THROW(json::Parser::FromStream(std::istringstream( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + "{\n" + " \"foo1\": [1, 2, 3],\n" + " \"bar1\": {\"x\": 0, \"y\": null},\n" + " \"baz1\": true,\n" + " \"str1\": \"asdfasdf\"\n" + "}\n" + )), json::Exception); +} + +TEST(FromStringTest, SingleObject) { + GuardJsonException([](){ + json::value v = json::Parser::FromString( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + ); + EXPECT_TOSTRING_EQ( + "{" + "\"bar\":{\"x\":0.0,\"y\":null}," + "\"baz\":true," + "\"foo\":[1.0,2.0,3.0]," + "\"str\":\"asdfasdf\"" + "}", + v); + }); +} + +TEST(FromStringTest, MultipleObjectsThrows) { + EXPECT_THROW(json::Parser::FromString( + "{\n" + " \"foo\": [1, 2, 3],\n" + " \"bar\": {\"x\": 0, \"y\": null},\n" + " \"baz\": true,\n" + " \"str\": \"asdfasdf\"\n" + "}\n" + "{\n" + " \"foo1\": [1, 2, 3],\n" + " \"bar1\": {\"x\": 0, \"y\": null},\n" + " \"baz1\": true,\n" + " \"str1\": \"asdfasdf\"\n" + "}\n" + ), json::Exception); +} + + // String edge cases. #if 0 @@ -503,29 +698,6 @@ TEST(EdgeTest, NegativeNumbers) { // Big tests. -constexpr const char kComplexExample[] = - "{\n" - " \"foo\": [1.0, 2, 3],\n" - " \"bar\": {\"x\": 0, \"y\": null},\n" - " \"baz\": true,\n" - " \"str\": \"asdfasdf\"\n" - "}\n"; - -constexpr const char kComplexExampleExpected[] = - "{" - "\"bar\":{\"x\":0.0,\"y\":null}," - "\"baz\":true," - "\"foo\":[1.0,2.0,3.0]," - "\"str\":\"asdfasdf\"" - "}"; - -TEST(BigTest, RealisticParsing) { - GuardJsonException([](){ - json::value v = json::Parser::FromString(kComplexExample); - EXPECT_TOSTRING_EQ(kComplexExampleExpected, v); - }); -} - TEST(BigTest, RealisticConstruction) { GuardJsonException([](){ json::value v = json::object({ @@ -547,9 +719,21 @@ TEST(BigTest, RealisticConstruction) { TEST(BigTest, Clone) { GuardJsonException([](){ - json::value v = json::Parser::FromString(kComplexExample); + json::value v = json::object({ + {"foo", json::array({json::number(1), json::number(2), json::number(3)})}, + {"bar", json::object({{"x", json::number(0)}, {"y", json::null()}})}, + {"baz", json::boolean(true)}, + {"str", json::string("asdfasdf")}, + }); json::value cloned = v->Clone(); - EXPECT_TOSTRING_EQ(kComplexExampleExpected, cloned); + EXPECT_TOSTRING_EQ( + "{" + "\"bar\":{\"x\":0.0,\"y\":null}," + "\"baz\":true," + "\"foo\":[1.0,2.0,3.0]," + "\"str\":\"asdfasdf\"" + "}", + cloned); }); } @@ -576,52 +760,52 @@ TEST(BigTest, LotsOfObjectNesting) { // Parse errors. TEST(ParseError, Empty) { - ASSERT_THROW(json::Parser::FromString(""), json::Exception); + EXPECT_THROW(json::Parser::FromString(""), json::Exception); } TEST(ParseError, UnexpectedChar) { - ASSERT_THROW(json::Parser::FromString("x"), json::Exception); + EXPECT_THROW(json::Parser::FromString("x"), json::Exception); } TEST(ParseError, UnterminatedArray) { - ASSERT_THROW(json::Parser::FromString("["), json::Exception); + EXPECT_THROW(json::Parser::FromString("["), json::Exception); } TEST(ParseError, UnmatchedArrayClose) { - ASSERT_THROW(json::Parser::FromString("]"), json::Exception); + EXPECT_THROW(json::Parser::FromString("]"), json::Exception); } TEST(ParseError, UnterminatedObject) { - ASSERT_THROW(json::Parser::FromString("{"), json::Exception); + EXPECT_THROW(json::Parser::FromString("{"), json::Exception); } TEST(ParseError, UnmatchedObjectClose) { - ASSERT_THROW(json::Parser::FromString("}"), json::Exception); + EXPECT_THROW(json::Parser::FromString("}"), json::Exception); } TEST(ParseError, UnterminatedString) { - ASSERT_THROW(json::Parser::FromString("\""), json::Exception); + EXPECT_THROW(json::Parser::FromString("\""), json::Exception); } TEST(ParseError, UnterminatedEscape) { - ASSERT_THROW(json::Parser::FromString("\"\\\""), json::Exception); + EXPECT_THROW(json::Parser::FromString("\"\\\""), json::Exception); } TEST(ParseError, UnterminatedUnicodeEscape) { - ASSERT_THROW(json::Parser::FromString("\"\\u\""), json::Exception); - ASSERT_THROW(json::Parser::FromString("\"\\u0\""), json::Exception); - ASSERT_THROW(json::Parser::FromString("\"\\u00\""), json::Exception); - ASSERT_THROW(json::Parser::FromString("\"\\u000\""), json::Exception); + EXPECT_THROW(json::Parser::FromString("\"\\u\""), json::Exception); + EXPECT_THROW(json::Parser::FromString("\"\\u0\""), json::Exception); + EXPECT_THROW(json::Parser::FromString("\"\\u00\""), json::Exception); + EXPECT_THROW(json::Parser::FromString("\"\\u000\""), json::Exception); } TEST(ParseError, ObjectNoValue) { - ASSERT_THROW(json::Parser::FromString("{\"x\"}"), json::Exception); - ASSERT_THROW(json::Parser::FromString("{\"x\":}"), json::Exception); + EXPECT_THROW(json::Parser::FromString("{\"x\"}"), json::Exception); + EXPECT_THROW(json::Parser::FromString("{\"x\":}"), json::Exception); } // Streaming parsing test. -TEST(StreamingTest, CompleteStream) { +TEST(StreamingTest, Complete) { GuardJsonException([](){ json::value v; json::Parser p([&v](json::value r) { v = std::move(r); }); @@ -644,7 +828,7 @@ TEST(StreamingTest, CompleteStream) { }); } -TEST(StreamingTest, SplitStream) { +TEST(StreamingTest, Split) { GuardJsonException([](){ json::value v; json::Parser p([&v](json::value r) { v = std::move(r); }); @@ -671,7 +855,7 @@ TEST(StreamingTest, SplitStream) { }); } -TEST(StreamingTest, BrokenStream) { +TEST(StreamingTest, SplitMidValue) { GuardJsonException([](){ json::value v; json::Parser p([&v](json::value r) { v = std::move(r); }); @@ -703,7 +887,7 @@ TEST(StreamingTest, BrokenStream) { }); } -TEST(StreamingTest, MultipleObjectsStream) { +TEST(StreamingTest, MultipleObjects) { GuardJsonException([](){ std::vector v; json::Parser p([&v](json::value r) { v.emplace_back(std::move(r)); }); @@ -742,12 +926,48 @@ TEST(StreamingTest, MultipleObjectsStream) { } TEST(StreamingTest, ParseStreamReturnsByteCount) { + GuardJsonException([](){ + json::value v; + json::Parser p([&v](json::value r) { v = std::move(r); }); + size_t n = p.ParseStream(std::istringstream("[1]")); + EXPECT_TOSTRING_EQ("[1.0]", v); + EXPECT_EQ(3, n); + }); +} + +TEST(StreamingTest, ParseStreamNeedsEofToParseNumbers) { GuardJsonException([](){ json::value v; json::Parser p([&v](json::value r) { v = std::move(r); }); size_t n = p.ParseStream(std::istringstream("123")); - EXPECT_TOSTRING_EQ("123", v); EXPECT_EQ(3, n); + EXPECT_EQ(nullptr, v); + // The number is not known until EOF is reached. + p.NotifyEOF(); + EXPECT_TOSTRING_EQ("123.0", v); + }); +} + +TEST(StreamingTest, ParseStreamThrowsOnMidStreamParseError) { + GuardJsonException([](){ + json::value v; + json::Parser p([&v](json::value r) { v = std::move(r); }); + EXPECT_THROW(p.ParseStream(std::istringstream("400 xyz")), json::Exception); + EXPECT_TOSTRING_EQ("400.0", v); + }); +} + +TEST(StreamingTest, ParseStreamThrowsAtEofOnIncompleteStream) { + GuardJsonException([](){ + json::value v; + json::Parser p([&v](json::value r) { + std::cerr << "Callback invoked with " << *r << std::endl; + v = std::move(r); + }); + EXPECT_EQ(1, p.ParseStream(std::istringstream("["))); + EXPECT_EQ(nullptr, v); + // Not known to be incomplete until EOF is reached. + EXPECT_THROW(p.NotifyEOF(), json::Exception); }); }