Fix upb's json decoder ignoring trailing characters after a successfully parsed object.

This is a breaking change since the JSON parser will now correctly reject certain bad inputs that it previously silently accepted (for example: json="{}x" was accepted).

PiperOrigin-RevId: 580493003
pull/14686/head
Protobuf Team Bot 1 year ago committed by Copybara-Service
parent 2ec703fcc1
commit fa15c2160e
  1. 1
      upb/json/BUILD
  2. 33
      upb/json/decode.c
  3. 19
      upb/json/decode_test.cc
  4. 2
      upb/util/required_fields_test.cc

@ -41,6 +41,7 @@ cc_test(
":test_upb_proto",
":test_upb_proto_reflection",
"@com_google_googletest//:gtest_main",
"//upb:base",
"//upb:mem",
"//upb:reflection",
],

@ -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,

@ -30,10 +30,15 @@
#include "upb/json/decode.h"
#include <string>
#include <vector>
#include "google/protobuf/struct.upb.h"
#include <gtest/gtest.h>
#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);
}

@ -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(
{

Loading…
Cancel
Save