diff --git a/upb/json/BUILD b/upb/json/BUILD index 29ce7ae538..91c7dc1658 100644 --- a/upb/json/BUILD +++ b/upb/json/BUILD @@ -41,6 +41,7 @@ cc_test( ":test_upb_proto", ":test_upb_proto_reflection", "@com_google_googletest//:gtest_main", + "//upb:base", "//upb:mem", "//upb:reflection", ], diff --git a/upb/json/decode.c b/upb/json/decode.c index 683a721a50..ce48948634 100644 --- a/upb/json/decode.c +++ b/upb/json/decode.c @@ -102,9 +102,13 @@ static bool jsondec_isvalue(const upb_FieldDef* f) { jsondec_isnullvalue(f); } -UPB_NORETURN static void jsondec_err(jsondec* d, const char* msg) { +static void jsondec_seterrmsg(jsondec* d, const char* msg) { upb_Status_SetErrorFormat(d->status, "Error parsing JSON @%d:%d: %s", d->line, (int)(d->ptr - d->line_begin), msg); +} + +UPB_NORETURN static void jsondec_err(jsondec* d, const char* msg) { + jsondec_seterrmsg(d, msg); UPB_LONGJMP(d->err, 1); } @@ -119,7 +123,9 @@ UPB_NORETURN static void jsondec_errf(jsondec* d, const char* fmt, ...) { UPB_LONGJMP(d->err, 1); } -static void jsondec_skipws(jsondec* d) { +// Advances d->ptr until the next non-whitespace character or to the end of +// the buffer. +static void jsondec_consumews(jsondec* d) { while (d->ptr != d->end) { switch (*d->ptr) { case '\n': @@ -135,7 +141,16 @@ static void jsondec_skipws(jsondec* d) { return; } } - jsondec_err(d, "Unexpected EOF"); +} + +// Advances d->ptr until the next non-whitespace character. Postcondition that +// d->ptr is pointing at a valid non-whitespace character (will err if end of +// buffer is reached). +static void jsondec_skipws(jsondec* d) { + jsondec_consumews(d); + if (d->ptr == d->end) { + jsondec_err(d, "Unexpected EOF"); + } } static bool jsondec_tryparsech(jsondec* d, char ch) { @@ -1481,7 +1496,17 @@ static bool upb_JsonDecoder_Decode(jsondec* const d, upb_Message* const msg, if (UPB_SETJMP(d->err)) return false; jsondec_tomsg(d, msg, m); - return true; + + // Consume any trailing whitespace before checking if we read the entire + // input. + jsondec_consumews(d); + + if (d->ptr == d->end) { + return true; + } else { + jsondec_seterrmsg(d, "unexpected trailing characters"); + return false; + } } bool upb_JsonDecode(const char* buf, size_t size, upb_Message* msg, diff --git a/upb/json/decode_test.cc b/upb/json/decode_test.cc index 9d40b02b74..b196bf95a5 100644 --- a/upb/json/decode_test.cc +++ b/upb/json/decode_test.cc @@ -30,10 +30,15 @@ #include "upb/json/decode.h" +#include +#include + #include "google/protobuf/struct.upb.h" #include +#include "upb/base/status.hpp" #include "upb/json/test.upb.h" #include "upb/json/test.upbdefs.h" +#include "upb/mem/arena.h" #include "upb/mem/arena.hpp" #include "upb/reflection/def.hpp" @@ -100,3 +105,17 @@ TEST(JsonTest, DecodeConflictJsonName) { EXPECT_EQ(2, upb_test_Box_new_value(box)); EXPECT_EQ(0, upb_test_Box_value(box)); } + +TEST(JsonTest, RejectsBadTrailingCharacters) { + upb::Arena a; + std::string json_string = R"({}abc)"; + upb_test_Box* box = JsonDecode(json_string.c_str(), a.ptr()); + EXPECT_EQ(box, nullptr); +} + +TEST(JsonTest, AcceptsTrailingWhitespace) { + upb::Arena a; + std::string json_string = "{} \n \r\n \t\t"; + upb_test_Box* box = JsonDecode(json_string.c_str(), a.ptr()); + EXPECT_NE(box, nullptr); +} diff --git a/upb/util/required_fields_test.cc b/upb/util/required_fields_test.cc index 70f0a95bf6..396c3aa997 100644 --- a/upb/util/required_fields_test.cc +++ b/upb/util/required_fields_test.cc @@ -111,7 +111,7 @@ TYPED_TEST_SUITE(RequiredFieldsTest, MyTypes); // } TYPED_TEST(RequiredFieldsTest, TestRequired) { TestFixture::CheckRequired(R"json({})json", {"required_message"}); - TestFixture::CheckRequired(R"json({"required_message": {}}")json", {}); + TestFixture::CheckRequired(R"json({"required_message": {}})json", {}); TestFixture::CheckRequired( R"json( {