Skip to content

Commit

Permalink
Have JSONParser tests use the JSONParser API
Browse files Browse the repository at this point in the history 
They no longer use the JSONReader API, other than some enum and string
constant definitions that a later commit will hopefully move over from
JSONReader to JSONParser, once the JSONReader call sites have been
weaned off the JsonParseError enum (crbug.com/1070409).

Originally, there was a single JSON decoder implementation, and the
distinction between json_parser.h and json_reader.h was blurry. More
recently (crbug.com/c/1069271), we are experimenting with alternative
JSON decoder implementations. We may eventually settle on a winner, but
during the transition period, there may be several implementations. The
desired split is for json_reader.h to be the public API and for
json_parser.h to be *a* private implementation (the existing one).

In this new world, it is a layering violation for the lower level
JSONParser code and tests to rely on the higher level JSONReader API.
This commit helps address some of that concern: JSONParser tests now use
the JSONParser API, apart from the enum+string caveat mentioned above.

Bug: 1069271
Bug: 1070409
Change-Id: I5f261ae808b14997ddb748670b14ff4107f4dd33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2208837
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770834}
  • Loading branch information
Nigel Tao authored and Commit Bot committed May 20, 2020
1 parent 9469c29 commit 229a380
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 67 deletions.
161 changes: 94 additions & 67 deletions base/json/json_parser_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,76 +210,103 @@ TEST_F(JSONParserTest, ConsumeNumbers) {
}

TEST_F(JSONParserTest, ErrorMessages) {
JSONReader::ValueWithError root =
JSONReader::ReadAndReturnValueWithError("[42]", JSON_PARSE_RFC);
EXPECT_TRUE(root.error_message.empty());
EXPECT_EQ(0, root.error_code);
{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("[42]");
EXPECT_TRUE(value);
EXPECT_TRUE(parser.GetErrorMessage().empty());
EXPECT_EQ(0, parser.error_code());
}

// Test each of the error conditions
root = JSONReader::ReadAndReturnValueWithError("{},{}", JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(
1, 3, JSONReader::kUnexpectedDataAfterRoot),
root.error_message);
EXPECT_EQ(JSONReader::JSON_UNEXPECTED_DATA_AFTER_ROOT, root.error_code);

std::string nested_json;
for (int i = 0; i < 201; ++i) {
nested_json.insert(nested_json.begin(), '[');
nested_json.append(1, ']');
{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("{},{}");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(
1, 3, JSONReader::kUnexpectedDataAfterRoot),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_UNEXPECTED_DATA_AFTER_ROOT, parser.error_code());
}

{
std::string nested_json;
for (int i = 0; i < 201; ++i) {
nested_json.insert(nested_json.begin(), '[');
nested_json.append(1, ']');
}
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse(nested_json);
EXPECT_FALSE(value);
EXPECT_EQ(
JSONParser::FormatErrorMessage(1, 200, JSONReader::kTooMuchNesting),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_TOO_MUCH_NESTING, parser.error_code());
}

{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("[1,]");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 4, JSONReader::kTrailingComma),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_TRAILING_COMMA, parser.error_code());
}

{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("{foo:\"bar\"}");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(
1, 2, JSONReader::kUnquotedDictionaryKey),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_UNQUOTED_DICTIONARY_KEY, parser.error_code());
}

{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("{\"foo\":\"bar\",}");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 14, JSONReader::kTrailingComma),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_TRAILING_COMMA, parser.error_code());
}

{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("[nu]");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 2, JSONReader::kSyntaxError),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_SYNTAX_ERROR, parser.error_code());
}

{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("[\"xxx\\xq\"]");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 7, JSONReader::kInvalidEscape),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_INVALID_ESCAPE, parser.error_code());
}

{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("[\"xxx\\uq\"]");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 7, JSONReader::kInvalidEscape),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_INVALID_ESCAPE, parser.error_code());
}

{
JSONParser parser(JSON_PARSE_RFC);
Optional<Value> value = parser.Parse("[\"xxx\\q\"]");
EXPECT_FALSE(value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 7, JSONReader::kInvalidEscape),
parser.GetErrorMessage());
EXPECT_EQ(JSONReader::JSON_INVALID_ESCAPE, parser.error_code());
}
root = JSONReader::ReadAndReturnValueWithError(nested_json, JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 200, JSONReader::kTooMuchNesting),
root.error_message);
EXPECT_EQ(JSONReader::JSON_TOO_MUCH_NESTING, root.error_code);

root = JSONReader::ReadAndReturnValueWithError("[1,]", JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 4, JSONReader::kTrailingComma),
root.error_message);
EXPECT_EQ(JSONReader::JSON_TRAILING_COMMA, root.error_code);

root =
JSONReader::ReadAndReturnValueWithError("{foo:\"bar\"}", JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(
JSONParser::FormatErrorMessage(1, 2, JSONReader::kUnquotedDictionaryKey),
root.error_message);
EXPECT_EQ(JSONReader::JSON_UNQUOTED_DICTIONARY_KEY, root.error_code);

root = JSONReader::ReadAndReturnValueWithError("{\"foo\":\"bar\",}",
JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 14, JSONReader::kTrailingComma),
root.error_message);

root = JSONReader::ReadAndReturnValueWithError("[nu]", JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 2, JSONReader::kSyntaxError),
root.error_message);
EXPECT_EQ(JSONReader::JSON_SYNTAX_ERROR, root.error_code);

root =
JSONReader::ReadAndReturnValueWithError("[\"xxx\\xq\"]", JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 7, JSONReader::kInvalidEscape),
root.error_message);
EXPECT_EQ(JSONReader::JSON_INVALID_ESCAPE, root.error_code);

root =
JSONReader::ReadAndReturnValueWithError("[\"xxx\\uq\"]", JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 7, JSONReader::kInvalidEscape),
root.error_message);
EXPECT_EQ(JSONReader::JSON_INVALID_ESCAPE, root.error_code);

root =
JSONReader::ReadAndReturnValueWithError("[\"xxx\\q\"]", JSON_PARSE_RFC);
EXPECT_FALSE(root.value);
EXPECT_EQ(JSONParser::FormatErrorMessage(1, 7, JSONReader::kInvalidEscape),
root.error_message);
EXPECT_EQ(JSONReader::JSON_INVALID_ESCAPE, root.error_code);
}

} // namespace internal
Expand Down
6 changes: 6 additions & 0 deletions base/json/json_reader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ enum JSONParserOptions {
class BASE_EXPORT JSONReader {
public:
// Error codes during parsing.
//
// TODO(crbug.com/1069271, crbug.com/1070409): move this enum from JSONReader
// (a higher level API, which can be backed by multiple implementations) to
// JSONParser (a lower level API, a single implementation). Such a move would
// also remove the ValueWithError.error_code field and move the
// kInvalidEscape, kSyntaxError, etc. strings defined in this .h file.
enum JsonParseError {
JSON_NO_ERROR = 0,
JSON_INVALID_ESCAPE,
Expand Down

0 comments on commit 229a380

Please sign in to comment.