maria-developers team mailing list archive

[Sergey Petrunia <sergey@xxxxxxxxxxx>]

Hi Serg,

This is review input for commit 8fb54fa006367e3a9cbe32dc1734c980cc5d5096.

Item #1: I've tried adding some tests and neither of them fails. Can you check
what's going on? I'm attaching the patch with tests to this email.

Item #2: please improve the commit comment as was requested in my previous
e-mail:

On Thu, Oct 28, 2021 at 04:58:35PM +0300, Sergey Petrunia wrote:
> First, input on the commit comment. Please consider it as a request applying
> to ALL further commits to the MariaDB codebase.
> 
> 
> > MDEV-23766: implemented requested debug checks
> 
> Imagine somebody looking at this in a few years. Will they know what checks
> were requested? They might get a clue by looking at the Jira text, but the
> idea to make the commit comments concise and self-contained descriptions.
> 
> Something like this:
> 
> Line #1: one-line description of the patch:
> 
> >> MDEV-23766: Make attempts to write invalid JSON cause assertion failures
> 
> Subsequent lines:
> 
> >> Make JSON writing API (class Json_writer) check the produced JSON document
> >> is valid. The following checks are made:
> >> - JSON objects must contain named members
> >> - JSON arrays must contain unnamed members.
> >> - (TODO: add other restrictions we're enforcing).

BR
 Sergei
-- 
Sergei Petrunia, Software Developer
MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net


diff --git a/sql/item_func.cc b/sql/item_func.cc
index 60efc55d878..22ecb06a837 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -56,6 +56,8 @@
 #include "sql_base.h"
 #include "sql_cte.h"
 
+#include "my_json_writer.h"
+
 #ifdef NO_EMBEDDED_ACCESS_CHECKS
 #define sp_restore_security_context(A,B) while (0) {}
 #endif
@@ -2137,6 +2139,7 @@ double Item_func_cos::val_real()
 
 double Item_func_sin::val_real()
 {
+  json_validity_test();
   DBUG_ASSERT(fixed == 1);
   double value= args[0]->val_real();
   if ((null_value=args[0]->null_value))
diff --git a/sql/my_json_writer.cc b/sql/my_json_writer.cc
index dc1f960ae8b..afd3f0ba8e8 100644
--- a/sql/my_json_writer.cc
+++ b/sql/my_json_writer.cc
@@ -289,6 +289,50 @@ void Json_writer::add_str(const char *str)
   add_str(str, len);
 }
 
+void test1()
+{
+  Json_writer w;
+  w.start_object();
+  w.add_member("foo"); // just a name, without a value
+  w.end_object();
+  fprintf(stderr, "\n%s\n", ((String*)w.output.get_string())->c_ptr());
+}
+
+void test2()
+{
+  Json_writer w;
+  w.start_object();
+   w.add_ull(123); // unnamed value in an object
+  w.end_object();
+  fprintf(stderr, "\n%s\n", ((String*)w.output.get_string())->c_ptr());
+}
+
+void test3()
+{
+  Json_writer w;
+  w.start_object();
+  w.add_member("bebebe").add_ull(345); // named value in an object. this is actually valid.
+  w.end_object();
+  fprintf(stderr, "\n%s\n", ((String*)w.output.get_string())->c_ptr());
+}
+
+void test4()
+{
+  Json_writer w;
+  w.start_array();
+  w.add_member("bebebe").add_ull(345); // named member in array.
+  w.end_array();
+  fprintf(stderr, "\n%s\n", ((String*)w.output.get_string())->c_ptr());
+}
+
+void json_validity_test()
+{
+  test1();
+  test2();
+  test3();
+  test4();
+}
+
 /*
   This function is used to add only num_bytes of str to the output string
 */
diff --git a/sql/my_json_writer.h b/sql/my_json_writer.h
index 94cd438bbb0..e5295a63664 100644
--- a/sql/my_json_writer.h
+++ b/sql/my_json_writer.h
@@ -174,6 +174,9 @@ class String_with_limit
   size_t truncated_len;
 };
 
+
+void json_validity_test();
+
 /*
   A class to write well-formed JSON documents. The documents are also formatted
   for human readability.