MDEV-23766: Make Json_writer assert when one tries to author invalid JSON

- Add unit test.

sql/my_json_writer.cc

@@ -39,7 +39,7 @@ inline void Json_writer::on_start_object()
 #ifndef NDEBUG
   if(!fmt_helper.is_making_writer_calls())
   {
-    DBUG_ASSERT(got_name == named_item_expected());
+    VALIDITY_ASSERT(got_name == named_item_expected());
     named_items_expectation.push_back(true);
   }
 #endif
@@ -74,7 +74,7 @@ void Json_writer::start_array()
 #ifndef NDEBUG
   if(!fmt_helper.is_making_writer_calls())
   {
-    DBUG_ASSERT(got_name == named_item_expected());
+    VALIDITY_ASSERT(got_name == named_item_expected());
     named_items_expectation.push_back(false);
     got_name= false;
   }
@@ -98,7 +98,7 @@ void Json_writer::end_object()
 {
 #ifndef NDEBUG
   named_items_expectation.pop_back();
-  DBUG_ASSERT(!got_name);
+  VALIDITY_ASSERT(!got_name);
   got_name= false;
 #endif
   indent_level-=INDENT_SIZE;
@@ -246,8 +246,8 @@ void Json_writer::add_unquoted_str(const char* str)
 
 void Json_writer::add_unquoted_str(const char* str, size_t len)
 {
-  DBUG_ASSERT(fmt_helper.is_making_writer_calls() ||
-              got_name == named_item_expected());
+  VALIDITY_ASSERT(fmt_helper.is_making_writer_calls() ||
+                  got_name == named_item_expected());
   if (on_add_str(str, len))
     return;
 
@@ -279,8 +279,8 @@ void Json_writer::add_str(const char *str)
 
 void Json_writer::add_str(const char* str, size_t num_bytes)
 {
-  DBUG_ASSERT(fmt_helper.is_making_writer_calls() ||
-              got_name == named_item_expected());
+  VALIDITY_ASSERT(fmt_helper.is_making_writer_calls() ||
+                  got_name == named_item_expected());
   if (on_add_str(str, num_bytes))
     return;
 

sql/my_json_writer.h

@@ -16,7 +16,17 @@
 #ifndef JSON_WRITER_INCLUDED
 #define JSON_WRITER_INCLUDED
 #include "my_base.h"
+
+#ifdef JSON_WRITER_UNIT_TEST
+#include "sql_string.h"
+#include <vector>
+// Also, mock objects are defined in my_json_writer-t.cc
+#define VALIDITY_ASSERT(x) if ((!x)) this->invalid_json= true;
+#else
 #include "sql_select.h"
+#define VALIDITY_ASSERT(x) DBUG_ASSERT(x)
+#endif
+
 class Opt_trace_stmt;
 class Opt_trace_context;
 class Json_writer;
@@ -191,12 +201,22 @@ class String_with_limit
 class Json_writer
 {
 #ifndef NDEBUG
-
+  /*
+    In debug mode, Json_writer will fail and assertion if one attempts to
+    produce an invalid JSON document (e.g. JSON array having named elements).
+  */
   std::vector<bool> named_items_expectation;
 
   bool named_item_expected() const;
 
   bool got_name;
+
+#ifdef JSON_WRITER_UNIT_TEST
+public:
+  // When compiled for unit test, creating invalid JSON will set this to true
+  // instead of an assertion.
+  bool invalid_json= false;
+#endif
 #endif
 
 public:

unittest/sql/CMakeLists.txt

@@ -36,3 +36,8 @@ ADD_EXECUTABLE(mf_iocache-t mf_iocache-t.cc ../../sql/mf_iocache_encr.cc)
 TARGET_LINK_LIBRARIES(mf_iocache-t mysys mytap mysys_ssl)
 ADD_DEPENDENCIES(mf_iocache-t GenError)
 MY_ADD_TEST(mf_iocache)
+
+# Json writer needs String which needs sql library
+ADD_EXECUTABLE(my_json_writer-t my_json_writer-t.cc dummy_builtins.cc)
+TARGET_LINK_LIBRARIES(my_json_writer-t sql mytap)
+MY_ADD_TEST(my_json_writer)

unittest/sql/my_json_writer-t.cc

+/*
+   Copyright (c) 2021, MariaDB Corporation.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335 USA */
+
+#include <my_global.h>
+#include <my_pthread.h>
+#include <my_sys.h>
+#include <stdio.h>
+#include <tap.h>
+
+/*
+  Unit tests for class Json_writer. At the moment there are only tests for the
+  "Fail an assertion if one attempts to produce invalid JSON" feature.
+*/
+
+struct TABLE;
+struct JOIN_TAB;
+class Json_writer;
+
+
+/* Several fake objects */
+class Opt_trace 
+{
+public:
+  void enable_tracing_if_required() {}
+  void disable_tracing_if_required() {}
+  Json_writer *get_current_json() { return nullptr; }
+};
+
+class THD 
+{
+public:
+  Opt_trace opt_trace;
+};
+
+#define JSON_WRITER_UNIT_TEST
+#include "../sql/my_json_writer.h"
+#include "../sql/my_json_writer.cc"
+
+int main(int args, char **argv)
+{
+  plan(NO_PLAN);
+  diag("Testing Json_writer checks");
+
+  {
+    Json_writer w;
+    w.start_object();
+    w.add_member("foo"); 
+    w.end_object();
+    ok(w.invalid_json, "Started a name but didn't add a value");
+  }
+
+  {
+    Json_writer w;
+    w.start_object();
+    w.add_ull(123);
+    ok(w.invalid_json, "Unnamed value in an object");
+  }
+
+  {
+    Json_writer w;
+    w.start_array();
+    w.add_member("bebebe").add_ull(345);
+    ok(w.invalid_json, "Named member in array");
+  }
+
+  {
+    Json_writer w;
+    w.start_object();
+    w.start_array();
+    ok(w.invalid_json, "Unnamed array in an object");
+  }
+
+  {
+    Json_writer w;
+    w.start_object();
+    w.start_object();
+    ok(w.invalid_json, "Unnamed object in an object");
+  }
+
+  {
+    Json_writer w;
+    w.start_array();
+    w.add_member("zzz");
+    w.start_object();
+    ok(w.invalid_json, "Named object in an array");
+  }
+  {
+    Json_writer w;
+    w.start_array();
+    w.add_member("zzz");
+    w.start_array();
+    ok(w.invalid_json, "Named array in an array");
+  }
+
+  // BAD:
+  {
+    Json_writer w;
+    w.start_array();
+    w.end_object();
+    ok(!w.invalid_json, "BAD: not checked!");
+  }
+
+  // BAD:
+  {
+    Json_writer w;
+    w.start_object();
+    w.end_array();
+    ok(!w.invalid_json, "BAD: not checked!");
+  }
+
+
+
+  diag("Done");
+
+  return exit_status();
+}
+