MDEV-28598 Assertion 'got_name == named_item_expected()' failedbb-10.4-MDEV-28598

This assertion triggered on an attempt to write a piece of corrupt
JSON to the optimizer trace:
  "transformation": {
    "select_id": 2,
    "from": "IN (SELECT)",
    "to": "semijoin",
    {
       "join_optimization": {
       "select_id": 3,
       "steps": []
    }
  }

This happened because some parts of the query may be evaluated right
during the optimization stage. To handle such cases Json_writer will now
implicitly add named members with automatically assigned names
"implicitly_added_member_#1", "implicitly_added_member_#2", etc
to preserve JSON document correctness. Also the tail of the JSON document
will be printed to stderr before abort.
Those two improvements only apply for Debug builds, not for Release ones
as the Release builds skip JSON correctness checks

1 files changed, 92 insertions, 10 deletions

diff --git a/sql/my_json_writer.cc b/sql/my_json_writer.cc
index 8e35b25b822..c95e4c6d657 100644
--- a/sql/my_json_writer.cc
+++ b/sql/my_json_writer.cc
@@ -39,7 +39,7 @@ inline void Json_writer::on_start_object()
 #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
   if(!fmt_helper.is_making_writer_calls())
   {
-    VALIDITY_ASSERT(got_name == named_item_expected());
+    validate_named_item_expectation(true);
     named_items_expectation.push_back(true);
   }
 #endif
@@ -68,7 +68,7 @@ void Json_writer::start_array()
 #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
   if(!fmt_helper.is_making_writer_calls())
   {
-    VALIDITY_ASSERT(got_name == named_item_expected());
+    validate_named_item_expectation(true);
     named_items_expectation.push_back(false);
     got_name= false;
   }
@@ -91,9 +91,20 @@ void Json_writer::start_array()
 void Json_writer::end_object()
 {
 #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
-  VALIDITY_ASSERT(named_item_expected());
+  if (!named_item_expected())
+  {
+    print_json_tail_to_stderr(
+          "end_object failed since an unnamed item is expected");
+    VALIDITY_ASSERT(false);
+  }
   named_items_expectation.pop_back();
-  VALIDITY_ASSERT(!got_name);
+
+  if (got_name)
+  {
+    print_json_tail_to_stderr(
+          "end_object failed since got_name==true");
+    VALIDITY_ASSERT(false);
+  }
   got_name= false;
 #endif
   indent_level-=INDENT_SIZE;
@@ -107,7 +118,12 @@ void Json_writer::end_object()
 void Json_writer::end_array()
 {
 #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
-  VALIDITY_ASSERT(!named_item_expected());
+  if (named_item_expected())
+  {
+    print_json_tail_to_stderr(
+          "end_array failed since a named item is expected");
+    VALIDITY_ASSERT(false);
+  }
   named_items_expectation.pop_back();
   got_name= false;
 #endif
@@ -130,8 +146,18 @@ Json_writer& Json_writer::add_member(const char *name, size_t len)
 {
   if (!fmt_helper.on_add_member(name, len))
   {
+#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
     // assert that we are in an object
-    DBUG_ASSERT(!element_started);
+    if (element_started)
+    {
+      char err_descr[200];
+      snprintf(err_descr, sizeof(err_descr),
+               "failed to add_member(\"%s\", %zu) since element_started=false",
+               name, len);
+      print_json_tail_to_stderr(err_descr);
+      VALIDITY_ASSERT(false);
+    }
+#endif
     start_element();
 
     output.append('"');
@@ -242,8 +268,12 @@ void Json_writer::add_unquoted_str(const char* str)
 
 void Json_writer::add_unquoted_str(const char* str, size_t len)
 {
-  VALIDITY_ASSERT(fmt_helper.is_making_writer_calls() ||
-                  got_name == named_item_expected());
+#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
+  if (!fmt_helper.is_making_writer_calls())
+  {
+    validate_named_item_expectation();
+  }
+#endif
   if (on_add_str(str, len))
     return;
 
@@ -275,8 +305,12 @@ void Json_writer::add_str(const char *str)
 
 void Json_writer::add_str(const char* str, size_t num_bytes)
 {
-  VALIDITY_ASSERT(fmt_helper.is_making_writer_calls() ||
-                  got_name == named_item_expected());
+#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
+  if (!fmt_helper.is_making_writer_calls())
+  {
+    validate_named_item_expectation();
+  }
+#endif
   if (on_add_str(str, num_bytes))
     return;
 
@@ -294,6 +328,53 @@ void Json_writer::add_str(const String &str)
   add_str(str.ptr(), str.length());
 }
 
+#if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST)
+void Json_writer::print_json_tail_to_stderr(const char *err_descr)
+{
+  size_t pos_to_print_from=
+    output.get_string()->length() < MAX_JSON_LEN_FOR_STDERR ?
+      0 : output.get_string()->length() - MAX_JSON_LEN_FOR_STDERR;
+  auto str_to_print= output.get_string()->c_ptr() + pos_to_print_from;
+  fprintf(stderr, "JSON writer error: %s.\nJSON tail: %s\n",
+          err_descr, str_to_print);
+}
+
+void Json_writer::validate_named_item_expectation(bool fix_wth_implicit_member)
+{
+  if (got_name != named_item_expected())
+  {
+    if (!got_name && fix_wth_implicit_member)
+    {
+      /*
+        Context is expecting a named member but has an unnamed one.
+        Add a member with automatically assigned name to preserve
+        JSON document correctness
+      */
+      add_implicit_member();
+      return;
+    }
+    const char *expected= named_item_expected() ?
+          "named" : "unnamed";
+    const char *actual= named_item_expected() ?
+          "unnamed" : "named";
+    char err_descr[100];
+    snprintf(err_descr, sizeof(err_descr),
+             "expected a %s member but a %s one has been added",
+             expected, actual);
+    print_json_tail_to_stderr(err_descr);
+    VALIDITY_ASSERT(false);
+  }
+}
+
+void Json_writer::add_implicit_member()
+{
+  char name[50];
+  sprintf(name, "implicitly_added_member_#%u", implicit_member_num);
+  add_member(name);
+  implicit_member_num++;
+}
+#endif
+
 Json_writer_temp_disable::Json_writer_temp_disable(THD *thd_arg)
 {
   thd= thd_arg;
@@ -308,6 +389,7 @@ bool Single_line_formatting_helper::on_add_member(const char *name,
                                                   size_t len)
 {
   DBUG_ASSERT(state== INACTIVE || state == DISABLED);
+
   if (state != DISABLED)
   {
     // remove everything from the array