Merge branch 'pu/json-hooks-21.04-bugfixes' into 'main'

Bug fixes for JSON hooks

See merge request apt-team/apt!165

3 files changed, 102 insertions, 4 deletions

diff --git a/apt-private/private-json-hooks.cc b/apt-private/private-json-hooks.cc
index 0b765a4ed..6bf70b1c6 100644
--- a/apt-private/private-json-hooks.cc
+++ b/apt-private/private-json-hooks.cc
@@ -11,7 +11,9 @@
 #include <apt-pkg/macros.h>
 #include <apt-pkg/strutl.h>
 #include <apt-private/private-json-hooks.h>
+#include <apt-private/private-output.h>
 
+#include <iomanip>
 #include <ostream>
 #include <sstream>
 #include <stack>
@@ -23,7 +25,7 @@
 /**
  * @brief Simple JSON writer
  *
- * This performs no error checking, or string escaping, be careful.
+ * This performs no error checking, so be careful.
  */
 class APT_HIDDEN JsonWriter
 {
@@ -78,6 +80,7 @@ class APT_HIDDEN JsonWriter
    void popState()
    {
       this->state = old_states.top();
+      old_states.pop();
    }
 
    public:
@@ -109,22 +112,40 @@ class APT_HIDDEN JsonWriter
       os << '}';
       return *this;
    }
+   std::ostream &encodeString(std::ostream &out, std::string const &str)
+   {
+      out << '"';
+
+      for (std::string::const_iterator c = str.begin(); c != str.end(); c++)
+      {
+	 if (*c <= 0x1F || *c == '"' || *c == '\\')
+	    ioprintf(out, "\\u%04X", *c);
+	 else
+	    out << *c;
+      }
+
+      out << '"';
+      return out;
+   }
    JsonWriter &name(std::string const &name)
    {
       maybeComma();
-      os << '"' << name << '"' << ':';
+      encodeString(os, name) << ':';
       return *this;
    }
    JsonWriter &value(std::string const &value)
    {
       maybeComma();
-      os << '"' << value << '"';
+      encodeString(os, value);
       return *this;
    }
    JsonWriter &value(const char *value)
    {
       maybeComma();
-      os << '"' << value << '"';
+      if (value == nullptr)
+	 os << "null";
+      else
+	 encodeString(os, value);
       return *this;
    }
    JsonWriter &value(int value)
@@ -315,6 +336,13 @@ bool RunJsonHook(std::string const &option, std::string const &method, const cha
       return true;
    Opts = Opts->Child;
 
+   // Flush output before calling hooks
+   std::clog.flush();
+   std::cerr.flush();
+   std::cout.flush();
+   c2out.flush();
+   c1out.flush();
+
    sighandler_t old_sigpipe = signal(SIGPIPE, SIG_IGN);
    sighandler_t old_sigint = signal(SIGINT, SIG_IGN);
    sighandler_t old_sigquit = signal(SIGQUIT, SIG_IGN);
diff --git a/test/integration/test-apt-cli-json-hooks b/test/integration/test-apt-cli-json-hooks
index 80922e01b..298fae072 100755
--- a/test/integration/test-apt-cli-json-hooks
+++ b/test/integration/test-apt-cli-json-hooks
@@ -21,6 +21,7 @@ APTARCHIVE="$(readlink -f ./aptarchive)"
 
 cat >> json-hook.sh << EOF
 #!/bin/bash
+set -e
 while true; do
 	read request <&\$APT_HOOK_SOCKET
 	read empty <&\$APT_HOOK_SOCKET
diff --git a/test/libapt/json_test.cc b/test/libapt/json_test.cc
new file mode 100644
index 000000000..ee8f3cebe
--- /dev/null
+++ b/test/libapt/json_test.cc
@@ -0,0 +1,69 @@
+#include <config.h>
+#include "../../apt-private/private-cachefile.cc"
+#include "../../apt-private/private-json-hooks.cc"
+#include <gtest/gtest.h>
+#include <string>
+
+TEST(JsonTest, JsonString)
+{
+   std::ostringstream os;
+
+   // Check for escaping backslash and quotation marks, and ensure that we do not change number formatting
+   JsonWriter(os).value("H al\"l\\o").value(17);
+
+   EXPECT_EQ("\"H al\\u0022l\\u005Co\"17", os.str());
+
+   for (int i = 0; i <= 0x1F; i++)
+   {
+      os.str("");
+
+      JsonWriter(os).encodeString(os, std::string("X") + char(i) + "Z");
+
+      std::string exp;
+      strprintf(exp, "\"X\\u%04XZ\"", i);
+
+      EXPECT_EQ(exp, os.str());
+   }
+}
+
+TEST(JsonTest, JsonObject)
+{
+   std::ostringstream os;
+
+   JsonWriter(os).beginObject().name("key").value("value").endObject();
+
+   EXPECT_EQ("{\"key\":\"value\"}", os.str());
+}
+
+TEST(JsonTest, JsonArrayAndValues)
+{
+   std::ostringstream os;
+
+   JsonWriter(os).beginArray().value(0).value("value").value(1).value(true).endArray();
+
+   EXPECT_EQ("[0,\"value\",1,true]", os.str());
+}
+TEST(JsonTest, JsonStackRegression)
+{
+   std::ostringstream os;
+
+   JsonWriter w(os);
+
+   // Nest those things deeply such that we transition states:
+   //    object -> array -> object; -> array -> object
+   // Older versions never popped back and got stuck on array state.
+   w.beginObject();
+   w.name("a").beginArray().beginObject().endObject().endArray();
+   w.name("b").beginArray().beginObject().endObject().endArray();
+   w.endObject();
+
+   EXPECT_EQ("{\"a\":[{}],\"b\":[{}]}", os.str());
+}
+TEST(JsonTest, JsonNull)
+{
+   std::ostringstream os;
+
+   JsonWriter(os).value(nullptr);
+
+   EXPECT_EQ("null", os.str());
+}