Skip to content

Commit fa15c21

Browse files
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
1 parent 2ec703f commit fa15c21

File tree

4 files changed

+50
-5
lines changed

4 files changed

+50
-5
lines changed

upb/json/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ cc_test(
4141
":test_upb_proto",
4242
":test_upb_proto_reflection",
4343
"@com_google_googletest//:gtest_main",
44+
"//upb:base",
4445
"//upb:mem",
4546
"//upb:reflection",
4647
],

upb/json/decode.c

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,13 @@ static bool jsondec_isvalue(const upb_FieldDef* f) {
102102
jsondec_isnullvalue(f);
103103
}
104104

105-
UPB_NORETURN static void jsondec_err(jsondec* d, const char* msg) {
105+
static void jsondec_seterrmsg(jsondec* d, const char* msg) {
106106
upb_Status_SetErrorFormat(d->status, "Error parsing JSON @%d:%d: %s", d->line,
107107
(int)(d->ptr - d->line_begin), msg);
108+
}
109+
110+
UPB_NORETURN static void jsondec_err(jsondec* d, const char* msg) {
111+
jsondec_seterrmsg(d, msg);
108112
UPB_LONGJMP(d->err, 1);
109113
}
110114

@@ -119,7 +123,9 @@ UPB_NORETURN static void jsondec_errf(jsondec* d, const char* fmt, ...) {
119123
UPB_LONGJMP(d->err, 1);
120124
}
121125

122-
static void jsondec_skipws(jsondec* d) {
126+
// Advances d->ptr until the next non-whitespace character or to the end of
127+
// the buffer.
128+
static void jsondec_consumews(jsondec* d) {
123129
while (d->ptr != d->end) {
124130
switch (*d->ptr) {
125131
case '\n':
@@ -135,7 +141,16 @@ static void jsondec_skipws(jsondec* d) {
135141
return;
136142
}
137143
}
138-
jsondec_err(d, "Unexpected EOF");
144+
}
145+
146+
// Advances d->ptr until the next non-whitespace character. Postcondition that
147+
// d->ptr is pointing at a valid non-whitespace character (will err if end of
148+
// buffer is reached).
149+
static void jsondec_skipws(jsondec* d) {
150+
jsondec_consumews(d);
151+
if (d->ptr == d->end) {
152+
jsondec_err(d, "Unexpected EOF");
153+
}
139154
}
140155

141156
static bool jsondec_tryparsech(jsondec* d, char ch) {
@@ -1481,7 +1496,17 @@ static bool upb_JsonDecoder_Decode(jsondec* const d, upb_Message* const msg,
14811496
if (UPB_SETJMP(d->err)) return false;
14821497

14831498
jsondec_tomsg(d, msg, m);
1484-
return true;
1499+
1500+
// Consume any trailing whitespace before checking if we read the entire
1501+
// input.
1502+
jsondec_consumews(d);
1503+
1504+
if (d->ptr == d->end) {
1505+
return true;
1506+
} else {
1507+
jsondec_seterrmsg(d, "unexpected trailing characters");
1508+
return false;
1509+
}
14851510
}
14861511

14871512
bool upb_JsonDecode(const char* buf, size_t size, upb_Message* msg,

upb/json/decode_test.cc

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,15 @@
3030

3131
#include "upb/json/decode.h"
3232

33+
#include <string>
34+
#include <vector>
35+
3336
#include "google/protobuf/struct.upb.h"
3437
#include <gtest/gtest.h>
38+
#include "upb/base/status.hpp"
3539
#include "upb/json/test.upb.h"
3640
#include "upb/json/test.upbdefs.h"
41+
#include "upb/mem/arena.h"
3742
#include "upb/mem/arena.hpp"
3843
#include "upb/reflection/def.hpp"
3944

@@ -100,3 +105,17 @@ TEST(JsonTest, DecodeConflictJsonName) {
100105
EXPECT_EQ(2, upb_test_Box_new_value(box));
101106
EXPECT_EQ(0, upb_test_Box_value(box));
102107
}
108+
109+
TEST(JsonTest, RejectsBadTrailingCharacters) {
110+
upb::Arena a;
111+
std::string json_string = R"({}abc)";
112+
upb_test_Box* box = JsonDecode(json_string.c_str(), a.ptr());
113+
EXPECT_EQ(box, nullptr);
114+
}
115+
116+
TEST(JsonTest, AcceptsTrailingWhitespace) {
117+
upb::Arena a;
118+
std::string json_string = "{} \n \r\n \t\t";
119+
upb_test_Box* box = JsonDecode(json_string.c_str(), a.ptr());
120+
EXPECT_NE(box, nullptr);
121+
}

upb/util/required_fields_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ TYPED_TEST_SUITE(RequiredFieldsTest, MyTypes);
111111
// }
112112
TYPED_TEST(RequiredFieldsTest, TestRequired) {
113113
TestFixture::CheckRequired(R"json({})json", {"required_message"});
114-
TestFixture::CheckRequired(R"json({"required_message": {}}")json", {});
114+
TestFixture::CheckRequired(R"json({"required_message": {}})json", {});
115115
TestFixture::CheckRequired(
116116
R"json(
117117
{

0 commit comments

Comments
 (0)