Address review notes in jsonpull itself

- json_stringify walked c_str(), so it truncated at an embedded NUL even
  though values are std::string now and carry one through faithfully.
  Range over the string instead; the existing control-character branch
  already escapes a NUL like any other, so the output stays valid JSON.
- Assert that the hash has an entry waiting before add_object() assigns to
  entries().back(). It always does -- JSON_VALUE is only set by a colon,
  which requires a pushed key -- but the derivation is not local.
- Drop fabricate_object(), a pass-through to make_object() with the
  arguments reordered, kept only to preserve the old C name.
- Inline the string_append / string_append_c wrappers over push_back and
  append, and note that json_print_one's JSON_HASH and JSON_ARRAY branches
  are unreachable, since json_print handles both itself.
- json_hash_get's comment said nullptr meant "the matching value is null",
  which reads as JSON null. A JSON null comes back as a JSON_NULL node;
  nullptr means the value slot is not filled in yet.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
This commit is contained in:
Claude
2026-08-14 00:32:43 +00:00
parent 5c61d85ff5
commit 6b73bd6261
2 changed files with 41 additions and 45 deletions
+37 -43
View File
@@ -110,10 +110,6 @@ static json_object_ptr make_object(json_type type, json_object *parent, json_pul
}
}
static json_object_ptr fabricate_object(json_pull *jp, json_object *parent, json_type type) {
return make_object(type, parent, jp);
}
static inline json_pull::parse_frame *current_frame(json_pull *j) {
return j->container_stack.empty() ? nullptr : &j->container_stack.back();
}
@@ -141,6 +137,10 @@ static json_object *add_object(json_pull *j, json_type type) {
}
} else if (c->type == JSON_HASH) {
if (f->expect == JSON_VALUE) {
// JSON_VALUE is only set by a colon, a colon requires
// JSON_COLON, and only pushing a key sets that, so there
// is always an entry waiting for its value here.
assert(!c->entries().empty());
c->entries().back().value = std::move(o);
f->expect = JSON_COMMA;
} else if (f->expect == JSON_KEY) {
@@ -736,7 +736,7 @@ static json_object_ptr take_from_owner(json_object *o) {
auto &e = entries[i];
if (e.key.get() == o) {
json_object_ptr taken = std::move(e.key);
e.key = fabricate_object(parent->parser, parent, JSON_NULL);
e.key = make_object(JSON_NULL, parent, parent->parser);
if (e.value != nullptr && e.value->type == JSON_NULL && e.key->type == JSON_NULL) {
entries.erase(entries.begin() + i);
}
@@ -744,7 +744,7 @@ static json_object_ptr take_from_owner(json_object *o) {
}
if (e.value.get() == o) {
json_object_ptr taken = std::move(e.value);
e.value = fabricate_object(parent->parser, parent, JSON_NULL);
e.value = make_object(JSON_NULL, parent, parent->parser);
if (e.key != nullptr && e.key->type == JSON_NULL && e.value->type == JSON_NULL) {
entries.erase(entries.begin() + i);
}
@@ -829,90 +829,84 @@ json_object_ptr json_disconnect(json_object *o) {
return taken;
}
static void string_append_c(std::string &val, char c) {
val.push_back(c);
}
static void string_append(std::string &val, const char *add) {
val.append(add);
}
static void json_print_one(std::string &val, const json_object *o) {
if (o == nullptr) {
string_append(val, "...");
val.append("...");
} else if (o->type == JSON_STRING) {
string_append_c(val, '\"');
val.push_back('\"');
for (const char *cp = o->string().c_str(); *cp != '\0'; cp++) {
if (*cp == '\\' || *cp == '"') {
string_append_c(val, '\\');
string_append_c(val, *cp);
} else if (*cp >= 0 && *cp < ' ') {
// Range over the string rather than walking c_str(): the value is a
// std::string now and may legitimately contain an embedded NUL, which
// the control-character branch below escapes as a \u sequence like any
// other control character.
for (char c : o->string()) {
if (c == '\\' || c == '"') {
val.push_back('\\');
val.push_back(c);
} else if (c >= 0 && c < ' ') {
char *s;
if (asprintf(&s, "\\u%04x", *cp) >= 0) {
string_append(val, s);
if (asprintf(&s, "\\u%04x", c) >= 0) {
val.append(s);
free(s);
}
} else {
string_append_c(val, *cp);
val.push_back(c);
}
}
string_append_c(val, '\"');
val.push_back('\"');
} else if (o->type == JSON_NUMBER) {
if (o->large_signed() != 0) {
char s[65];
snprintf(s, sizeof(s), "%lld", o->large_signed());
string_append(val, s);
val.append(s);
} else if (o->large_unsigned() != 0) {
char s[65];
snprintf(s, sizeof(s), "%llu", o->large_unsigned());
string_append(val, s);
val.append(s);
} else {
char *s = dtoa_milo(o->number());
string_append(val, s);
val.append(s);
free(s);
}
} else if (o->type == JSON_NULL) {
string_append(val, "null");
val.append("null");
} else if (o->type == JSON_TRUE) {
string_append(val, "true");
val.append("true");
} else if (o->type == JSON_FALSE) {
string_append(val, "false");
} else if (o->type == JSON_HASH) {
string_append_c(val, '}');
} else if (o->type == JSON_ARRAY) {
string_append_c(val, ']');
val.append("false");
}
// JSON_HASH and JSON_ARRAY never reach here: json_print handles both
// itself and only delegates to json_print_one for the scalar types.
}
static void json_print(std::string &val, const json_object *o) {
if (o == nullptr) {
// Hash value in incompletely read hash
string_append(val, "...");
val.append("...");
} else if (o->type == JSON_HASH) {
string_append_c(val, '{');
val.push_back('{');
const auto &entries = o->entries();
for (size_t i = 0; i < entries.size(); i++) {
json_print(val, entries[i].key.get());
string_append_c(val, ':');
val.push_back(':');
json_print(val, entries[i].value.get());
if (i + 1 < entries.size()) {
string_append_c(val, ',');
val.push_back(',');
}
}
string_append_c(val, '}');
val.push_back('}');
} else if (o->type == JSON_ARRAY) {
string_append_c(val, '[');
val.push_back('[');
const auto &arr = o->array();
for (size_t i = 0; i < arr.size(); i++) {
json_print(val, arr[i].get());
if (i + 1 < arr.size()) {
string_append_c(val, ',');
val.push_back(',');
}
}
string_append_c(val, ']');
val.push_back(']');
} else {
json_print_one(val, o);
}
+4 -2
View File
@@ -394,8 +394,10 @@ json_object_ptr json_disconnect(json_object *o);
// Look up `s` in the hash `o`. Returns a borrowed pointer; ownership
// stays with the hash. nullptr if `o` is not a hash, or `s` is absent,
// or the matching value is null. Accepts a json_object_ptr by reference
// as a convenience so callers don't have to write `.get()`.
// or the hash is still being parsed and the value slot for `s` is not
// yet filled. A JSON `null` value is *not* one of those cases: it comes
// back as a JSON_NULL node. Accepts a json_object_ptr by reference as a
// convenience so callers don't have to write `.get()`.
json_object *json_hash_get(const json_object_ptr &o, const char *s);
json_object *json_hash_get(json_object *o, const char *s);