Copilot commented on code in PR #13609:
URL: https://github.com/apache/trafficserver/pull/13609#discussion_r3923485168
##########
include/tsutil/YamlCfg.h:
##########
@@ -39,6 +39,26 @@ namespace Yaml
constexpr std::string_view YAML_BOOL_TAG_URI{"tag:yaml.org,2002:bool"};
constexpr std::string_view YAML_NULL_TAG_URI{"tag:yaml.org,2002:null"};
+ // Put an emitter into JSON output mode.
+ //
+ // yaml-cpp has no JSON output mode. The nearest equivalent is flow style
with every scalar double quoted. For the
+ // node shapes the callers here emit -- maps, sequences, scalars and nulls,
carrying no tags, anchors or aliases --
+ // that is JSON-compatible except for null: yaml-cpp writes `~`, which JSON
parsers reject. LowerNull writes the
+ // literal `null` instead. YAML resolves `~` and `null` to the same value,
so the output still reads as YAML.
+ //
Review Comment:
The comment says the output is “JSON-compatible except for null”, but with
`YAML::DoubleQuoted` every scalar (including numbers and booleans) is emitted
as a JSON string (e.g. `"123"`, `"true"`). That is syntactically valid JSON,
but it’s not type-faithful JSON and can fail schema validation (as noted in the
PR description). Consider tightening the wording to explicitly say
“syntactically valid JSON” (or “JSON parseable”) and call out the scalar-typing
limitation so future callers don’t assume numeric/bool types are preserved.
##########
src/mgmt/rpc/handlers/hostdb/HostDB.cc:
##########
@@ -82,7 +82,7 @@ template <> struct convert<HostDBCache> {
static Node
encode(const HostDBCache *const hostDB, std::string_view hostname)
{
- Node partitions;
+ Node partitions{YAML::NodeType::Sequence};
Review Comment:
This change fixes a user-visible parsing failure (`partitions` becoming
`~`/null and breaking JSON parsers). There’s no test change in this PR to
prevent regressions. Add an autest (or CLI-level regression test) that runs
`traffic_ctl hostdb status -f json` against a freshly started server and
asserts the output is valid JSON (e.g., can be parsed by a strict JSON parser)
and that `partitions` is an array (empty `[]` in the empty-HostDB case), not
null.
##########
src/mgmt/rpc/handlers/plugins/Plugins.cc:
##########
@@ -100,7 +100,7 @@ get_plugin_list(std::string_view const & /* id ATS_UNUSED
*/, YAML::Node const &
data["source"] = summary.source;
- YAML::Node plugins;
+ YAML::Node plugins{YAML::NodeType::Sequence};
Review Comment:
Similar to `hostdb status`, `plugin list` previously produced invalid “JSON”
when the list was empty. Add a regression test case for an empty-plugins
scenario (e.g., empty `plugin.config`) that runs the relevant RPC/CLI path and
asserts the payload is strict-JSON parseable and `plugins` is `[]` (array)
rather than null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
