traffic_ctl: emit JSON null instead of YAML tilde

doc/developer-guide/jsonrpc/jsonrpc-architecture.en.rst

@@ -74,6 +74,9 @@ Our JSONRPC  protocol implementation uses lib yamlcpp for parsing incoming and o
 this allows the server to accept either JSON or YAML format messages which then will be parsed by the protocol implementation. This seems handy
 for user that want to feed |TS| with existing yaml configuration without the need to translate yaml into json.
 
+The server emits null values as the literal ``null``, not as YAML's ``~``. JSON parsers reject ``~``. YAML resolves ``~`` and
+``null`` to the same value. Clients that read the response as YAML see no change, and the server still accepts YAML input.
+
 .. note::
 
    :program:`traffic_ctl` have an option to read files from disc and push them into |TS| through the RPC server. Files should be a

include/mgmt/rpc/jsonrpc/json/YAMLCodec.h

@@ -21,16 +21,18 @@
 #pragma once
 
 #include <yaml-cpp/yaml.h>
+
+#include "tsutil/YamlCfg.h"
 #include "mgmt/rpc/jsonrpc/error/RPCError.h"
 #include "mgmt/rpc/jsonrpc/Defs.h"
 
 namespace rpc::json_codecs
 {
 ///
-/// @note The overall design is to make this classes @c yamlcpp_json_decoder and @c yamlcpp_json_encoder plugables into the Json Rpc
-/// encode/decode logic. yamlcpp does not give us all the behavior we need, such as the way it handles the null values. Json needs
-/// to use literal null and yamlcpp uses ~. If this becomes a problem, then we may need to change the codec implementation, we just
-/// follow the api and it should work with minimum changes.
+/// @note The design keeps @c yamlcpp_json_decoder and @c yamlcpp_json_encoder replaceable in the JSONRPC encode/decode logic.
+/// yaml-cpp emits null as @c ~ by default, which JSON parsers reject. Every emitter here calls
+/// @c ts::Yaml::configure_json_emitter, which writes the literal @c null instead. YAML resolves @c ~ and @c null to the same
+/// value, so the server still accepts YAML input.
 ///
 
 ///
@@ -251,8 +253,8 @@ class yamlcpp_json_encoder
     if (!resp.id.empty()) {
       json << YAML::Key << "id" << YAML::Value << resp.id;
     }
-    // else: We do not insert null as it will break the json, we need literal null and not ~ (as per yaml)
-    // json << YAML::Null;
+    // else: the field is omitted rather than set to null. Emitting it would be valid JSON now that LowerNull is set, but the
+    // omission is deliberate, see the id note in mgmt/rpc/schema/jsonrpc_response_schema.json.
 
     json << YAML::EndMap;
   }
@@ -268,7 +270,7 @@ class yamlcpp_json_encoder
   encode(const specs::RPCResponseInfo &resp)
   {
     YAML::Emitter json;
-    json << YAML::DoubleQuoted << YAML::Flow;
+    ts::Yaml::configure_json_emitter(json);
     encode(resp, json);
 
     return json.c_str();
@@ -284,7 +286,7 @@ class yamlcpp_json_encoder
   encode(const specs::RPCResponse &response)
   {
     YAML::Emitter json;
-    json << YAML::DoubleQuoted << YAML::Flow;
+    ts::Yaml::configure_json_emitter(json);
     {
       if (response.is_batch()) {
         json << YAML::BeginSeq;

include/shared/rpc/yaml_codecs.h

@@ -22,6 +22,8 @@
 #include <string_view>
 #include <yaml-cpp/yaml.h>
 
+#include "tsutil/YamlCfg.h"
+
 #include "shared/rpc/RPCRequests.h"
 
 /// JSONRPC 2.0 Client API request/response codecs only. If you need to define your own specific codecs they should then be defined
@@ -202,7 +204,7 @@ class yamlcpp_json_emitter
   encode(shared::rpc::JSONRPCRequest const &req)
   {
     YAML::Emitter json;
-    json << YAML::DoubleQuoted << YAML::Flow;
+    ts::Yaml::configure_json_emitter(json);
     json << YAML::BeginMap;
 
     if (!req.id.empty()) {

include/tsutil/YamlCfg.h

@@ -39,6 +39,31 @@ 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 parses as JSON 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.
+  //
+  // Parses as JSON is the whole guarantee. It is not type-faithful JSON: `DoubleQuoted` quotes every scalar, so
+  // numbers and booleans arrive as strings -- `"12"` rather than `12`, `"true"` rather than `true`. A consumer
+  // validating against a schema that declares `integer` or `boolean` will reject that, and no manipulator here
+  // changes it. Preserving scalar types needs a real JSON serializer, not a yaml-cpp emitter.
+  //
+  // This is not a general YAML to JSON converter. A node that carries a tag, an anchor or an alias still emits YAML
+  // syntax that JSON does not accept.
+  //
+  // Every emitter whose output reaches a JSON consumer must go through here. Setting only some of the manipulators
+  // gives output that looks like JSON and parses correctly until some node is null.
+  //
+  inline void
+  configure_json_emitter(YAML::Emitter &emitter)
+  {
+    emitter.SetNullFormat(YAML::LowerNull);
+    emitter << YAML::DoubleQuoted << YAML::Flow;
+  }
+
   // A class that is a wrapper for a YAML::Node that corresponds to a map in a YAML input file.
   // It's purpose is to make sure all keys in the map are processed.
   //

src/config/ssl_multicert.cc

@@ -34,6 +34,7 @@
 #include "swoc/swoc_file.h"
 #include "swoc/TextView.h"
 #include "tsutil/ts_diag_levels.h"
+#include "tsutil/YamlCfg.h"
 
 namespace
 {
@@ -360,7 +361,7 @@ std::string
 SSLMultiCertMarshaller::to_json(SSLMultiCertConfig const &config)
 {
   YAML::Emitter json;
-  json << YAML::DoubleQuoted << YAML::Flow;
+  ts::Yaml::configure_json_emitter(json);
   json << YAML::BeginMap;
   json << YAML::Key << KEY_SSL_MULTICERT << YAML::Value << YAML::BeginSeq;
 

src/config/storage.cc

@@ -37,6 +37,7 @@
 #include "swoc/swoc_file.h"
 #include "tscore/ParseRules.h"
 #include "tsutil/ts_diag_levels.h"
+#include "tsutil/YamlCfg.h"
 
 namespace
 {
@@ -828,7 +829,7 @@ std::string
 StorageMarshaller::to_json(StorageConfig const &config)
 {
   YAML::Emitter out;
-  out << YAML::DoubleQuoted << YAML::Flow;
+  ts::Yaml::configure_json_emitter(out);
   out << YAML::BeginMap;
   out << YAML::Key << KEY_CACHE << YAML::Value << YAML::BeginMap;
 

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};
     for (size_t i = 0; i < hostDB->refcountcache->partition_count(); i++) {
       auto                                 &partition = hostDB->refcountcache->get_partition(i);
       std::vector<RefCountCacheHashEntry *> partition_entries;

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};
     for (const auto &e : summary.entries) {
       YAML::Node plugin;
 

src/traffic_ctl/CtrlPrinters.cc

@@ -24,6 +24,7 @@
 
 #include <swoc/swoc_meta.h>
 #include "tsutil/ts_bw_format.h"
+#include "tsutil/YamlCfg.h"
 
 #include "CtrlPrinters.h"
 #include "jsonrpc/ctrl_yaml_codecs.h"
@@ -102,7 +103,7 @@ void
 BasePrinter::write_output_json(YAML::Node const &node) const
 {
   YAML::Emitter out;
-  out << YAML::DoubleQuoted << YAML::Flow;
+  ts::Yaml::configure_json_emitter(out);
   out << node;
   std::cout << out.c_str() << '\n';
 }

tests/gold_tests/traffic_ctl/traffic_ctl_json_null.test.py

@@ -0,0 +1,94 @@
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+
+import sys
+
+# To include util classes
+sys.path.insert(0, f'{Test.TestDirectory}')
+
+from traffic_ctl_test_utils import Make_traffic_ctl
+
+Test.Summary = '''
+traffic_ctl JSON output must be parseable JSON, including when a node is null.
+
+yaml-cpp emits null as `~`, which is valid YAML but rejected by every JSON
+parser. Emitters that produce JSON must set YAML::LowerNull, and container
+nodes that may stay empty must be constructed as sequences so they emit `[]`
+rather than null.
+
+The trigger for both regressions is the *empty* case, so this test
+deliberately runs against a freshly started server with an empty HostDB and no
+plugins loaded. A test that populates either one first would pass against the
+bug.
+'''
+
+Test.ContinueOnFail = True
+
+records_yaml = '''
+  exec_thread:
+    autoconfig:
+      enabled: 0
+    limit: 4
+    '''
+
+traffic_ctl = Make_traffic_ctl(Test, records_yaml)
+
+######
+# hostdb status -- `partitions` is empty on a fresh server.
+#
+# Flagless output goes through BasePrinter::write_output_json (the client
+# printer). Before the fix this emitted `"partitions": ~`.
+traffic_ctl.hostdb().status().validate_is_valid_json()
+
+# ... and it must be an empty array, not null. hostdb_status_schema.json
+# declares partitions as "type": "array".
+traffic_ctl.hostdb().status().validate_json_contains(partitions=[])
+
+# -f json goes through the full envelope. Same emitter, different entry point.
+traffic_ctl.hostdb().status().as_json().validate_is_valid_json()
+
+# The server-side encoder (yamlcpp_json_encoder) is a third, independent
+# emitter. rpc invoke exercises it directly.
+#
+# The params are required: get_hostdb_status without them fails with "invalid
+# node; this may result from using a map iterator as a sequence iterator", and
+# an error envelope is valid JSON no matter what the emitter does -- the
+# assertion would pass against the bug.
+traffic_ctl.rpc().invoke(handler="get_hostdb_status", params='"hostname: \\"\\""').validate_is_valid_json()
+
+######
+# plugin list -- `plugins` is empty when plugin.config loads nothing.
+#
+# plugin list ignores the format flag today and prints a human table, so only
+# the RPC path is assertable. Once plugin list honours -f json, add:
+#   traffic_ctl.plugin().list().as_json().validate_is_valid_json()
+#
+# Assert the shape rather than mere parseability: `plugins` has to be `[]`,
+# matching what the hostdb case above asserts for `partitions`. The field sits
+# at result.data.plugins, which validate_json_contains cannot reach, so this
+# compares the whole result, as the connection tracker cases in
+# traffic_ctl_server_output.test.py do. Before the fix the field emitted `~`,
+# which fails this comparison as surely as it fails a JSON parser. Nothing
+# forces plugin.config to be empty here, and nothing needs to: should a
+# default ever load a plugin, this assertion fails rather than going quiet.
+traffic_ctl.rpc().invoke(
+    handler="admin_plugin_get_list").validate_result_with_text('{"data": {"source": "plugin.config", "plugins": []}}')
+
+######
+# Commands that were already valid JSON -- guard against the shared emitter
+# change regressing them.
+traffic_ctl.server().status().validate_is_valid_json()
+traffic_ctl.rpc().invoke(handler="show_registered_handlers").validate_is_valid_json()