Skip to content

Fix JSON deserialization of empty containers and string-embedded delimiters #845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yungyuc merged 1 commit into from
Jun 1, 2026
+368 −86
Merged

Fix JSON deserialization of empty containers and string-embedded delimiters #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
235 changes: 149 additions & 86 deletions cpp/modmesh/serialization/SerializableItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,74 @@ std::string trim_string(const std::string & str)
column += 1; \
}

// Scan a balanced {...} or [...] starting at json[index], which must be the
// opening character. Appends the whole balanced expression to out and leaves
// index on the matching closing character. Characters inside quoted strings
// (with backslash escapes) are ignored when matching, so brackets or braces
// embedded in string values do not unbalance the scan. Throws when the
// container is not closed.
inline void scan_balanced_expression(const std::string & json, size_t & index, char open_char, char close_char, int & line, int & column, std::string & out)
Copy link
Copy Markdown
Member Author

@tigercosmos tigercosmos May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract the reused logic.

{
int depth = 0;
bool in_string = false;
bool escape = false;

while (index < json.size())
{
const char c_curr = json[index];
out.push_back(c_curr);

if (escape)
{
escape = false;
}
else if (in_string)
{
if (c_curr == '\\')
{
escape = true;
}
else if (c_curr == '"')
{
in_string = false;
}
}
else if (c_curr == '"')
{
in_string = true;
}
else if (c_curr == open_char)
{
depth += 1;
}
else if (c_curr == close_char)
{
depth -= 1;
}

if (!in_string && depth == 0)
{
break;
}

MM_DECL_CACULATE_LINE_COLUMN(json[index])
index += 1;
}

if (depth != 0)
{
throw_serialization_error("Invalid JSON format: missing closing bracket.", line, column);
}
}

// NOLINTBEGIN(readability-function-cognitive-complexity)
// NOLINTNEXTLINE(misc-no-recursion)
JsonArray JsonNode::parse_array(const std::string & json)
{
JsonArray json_array;
JsonState state = JsonState::Start;
std::string value_expression;

int depth = 0;
bool after_comma = false;

int line = 1;
int column = 1;
Expand Down Expand Up @@ -146,35 +205,24 @@ JsonArray JsonNode::parse_array(const std::string & json)
case JsonState::ObjectValue:
value_expression.clear();

if (c == '{')
if (c == ']')
{
// go to the end of the object
while (index < json.size())
if (after_comma)
{
value_expression.push_back(json[index]);

if (json[index] == '{')
{
depth += 1;
}
else if (json[index] == '}')
{
depth -= 1;
}

if (depth == 0)
{
break;
}

MM_DECL_CACULATE_LINE_COLUMN(json[index])
index += 1;
throw_serialization_error("Invalid JSON format: trailing comma before closing bracket.", line, column);
}
// Empty array.
state = JsonState::End;
break;
}

if (depth != 0)
{
throw_serialization_error("Invalid JSON format: missing closing bracket.", line, column);
}
after_comma = false;

if (c == '{')
{
// go to the end of the object (string-aware so braces inside
// string values do not unbalance the scan)
scan_balanced_expression(json, index, '{', '}', line, column, value_expression);

// NOLINTNEXTLINE(misc-no-recursion)
json_array.emplace_back(std::make_unique<JsonNode>(JsonType::Object, value_expression));
Expand All @@ -196,12 +244,36 @@ JsonArray JsonNode::parse_array(const std::string & json)

// we assume the value is a string, number, boolean, or null, and the expression is correct
// if the expression is not correct, the exception will be thrown when parsing the value later
// Track quoted-string state so that ',' or ']' inside a
// string value does not terminate the scan early.
Comment on lines +247 to +248
Copy link
Copy Markdown
Member Author

@tigercosmos tigercosmos May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the issue.

bool in_string = false;
bool escape = false;
while (index < json.size() - 1)
{
value_expression.push_back(json[index]);
const char c_curr = json[index];
value_expression.push_back(c_curr);
if (escape)
{
escape = false;
}
else if (in_string)
{
if (c_curr == '\\')
{
escape = true;
}
else if (c_curr == '"')
{
in_string = false;
}
}
else if (c_curr == '"')
{
in_string = true;
}

const char c_next = json[index + 1];
if (c_next == ',' || c_next == ']')
if (!in_string && (c_next == ',' || c_next == ']'))
{
break;
}
Expand Down Expand Up @@ -259,6 +331,7 @@ JsonArray JsonNode::parse_array(const std::string & json)
if (c == ',')
{
state = JsonState::ObjectValue;
after_comma = true;
}
else if (c == ']')
{
Expand Down Expand Up @@ -297,8 +370,8 @@ JsonMap JsonNode::parse_object(const std::string & json)
JsonState state = JsonState::Start;
std::string key;
std::string value_expression;
bool after_comma = false;

int depth = 0;
int line = 1;
int column = 1;

Expand Down Expand Up @@ -328,6 +401,19 @@ JsonMap JsonNode::parse_object(const std::string & json)
state = JsonState::ObjectKey;
break; /* end case JsonState::Start */
case JsonState::ObjectKey:
if (c == '}')
{
if (after_comma)
{
throw_serialization_error("Invalid JSON format: trailing comma before closing bracket.", line, column);
}
// Empty object.
state = JsonState::End;
break;
}

after_comma = false;

if (c == '"')
{
key.clear();
Expand Down Expand Up @@ -369,66 +455,18 @@ JsonMap JsonNode::parse_object(const std::string & json)

if (c == '{')
{
// go to the end of the object
while (index < json.size())
{
value_expression.push_back(json[index]);

if (json[index] == '{')
{
depth += 1;
}
else if (json[index] == '}')
{
depth -= 1;
}

if (depth == 0)
{
break;
}

MM_DECL_CACULATE_LINE_COLUMN(json[index])
index += 1;
}

if (depth != 0)
{
throw_serialization_error("Invalid JSON format: missing closing bracket.", line, column);
}
// go to the end of the object (string-aware so braces inside
// string values do not unbalance the scan)
scan_balanced_expression(json, index, '{', '}', line, column, value_expression);

// NOLINTNEXTLINE(misc-no-recursion)
json_map.emplace(key, std::make_unique<JsonNode>(JsonType::Object, value_expression));
}
else if (c == '[')
{
// go to the end of the array
while (index < json.size())
{
value_expression.push_back(json[index]);

if (json[index] == '[')
{
depth += 1;
}
else if (json[index] == ']')
{
depth -= 1;
}

if (depth == 0)
{
break;
}

MM_DECL_CACULATE_LINE_COLUMN(json[index])
index += 1;
}

if (depth != 0)
{
throw_serialization_error("Invalid JSON format: missing closing bracket.", line, column);
}
// go to the end of the array (string-aware so brackets inside
// string values do not unbalance the scan)
scan_balanced_expression(json, index, '[', ']', line, column, value_expression);

// NOLINTNEXTLINE(misc-no-recursion)
json_map.emplace(key, std::make_unique<JsonNode>(JsonType::Array, value_expression));
Expand All @@ -445,12 +483,36 @@ JsonMap JsonNode::parse_object(const std::string & json)

// we assume the value is a string, number, boolean, or null, and the expression is correct
// if the expression is not correct, the exception will be thrown when parsing the value later
// Track quoted-string state so that ',' or '}' inside a
// string value does not terminate the scan early.
Comment on lines +486 to +487
Copy link
Copy Markdown
Member Author

@tigercosmos tigercosmos May 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the issue.

bool in_string = false;
bool escape = false;
while (index < json.size() - 1)
{
value_expression.push_back(json[index]);
const char c_curr = json[index];
value_expression.push_back(c_curr);
if (escape)
{
escape = false;
}
else if (in_string)
{
if (c_curr == '\\')
{
escape = true;
}
else if (c_curr == '"')
{
in_string = false;
}
}
else if (c_curr == '"')
{
in_string = true;
}

const char c_next = json[index + 1];
if (c_next == ',' || c_next == '}')
if (!in_string && (c_next == ',' || c_next == '}'))
{
break;
}
Expand Down Expand Up @@ -508,6 +570,7 @@ JsonMap JsonNode::parse_object(const std::string & json)
if (c == ',')
{
state = JsonState::ObjectKey;
after_comma = true;
}
else if (c == '}')
{
Expand Down
Loading
Loading