common: admin socket fallback to json-pretty format

If the format argument to a command sent to the admin socket is not
among the supported formats ( json, json-pretty, xml, xml-pretty ) the
new_formatter function will return null and the AdminSocketHook::call
function must fall back to a sensible default.

The CephContextHook::call and HelpHook::call failed to do that and a
malformed format argument would cause the mon to crash. A check is added
to each of them and fallback to json-pretty if the format is not
recognized.

To further protect AdminSocketHook::call implementations from similar
problems the format argument is checked immediately after accepting the
command in AdminSocket::do_accept and replaced with json-pretty if it is
not known.

A test case is added for both CephContextHook::call and HelpHook::call
to demonstrate the problem exists and is fixed by the patch.

Three other instances of unsafe calls to new_formatter were found and
a fallback to json-pretty was added. All other calls have been audited
and appear to be safe.

http://tracker.ceph.com/issues/7378 fixes #7378

Backport: emperor, dumpling
Signed-off-by: Loic Dachary <loic@dachary.org>

1 files changed, 43 insertions, 0 deletions

diff --git a/src/test/admin_socket.cc b/src/test/admin_socket.cc
index 6f2a215b565..6484740674f 100644
--- a/src/test/admin_socket.cc
+++ b/src/test/admin_socket.cc
@@ -56,6 +56,38 @@ TEST(AdminSocket, TeardownSetup) {
   ASSERT_EQ(true, asoct.shutdown());
 }
 
+TEST(AdminSocket, SendHelp) {
+  std::auto_ptr<AdminSocket>
+      asokc(new AdminSocket(g_ceph_context));
+  AdminSocketTest asoct(asokc.get());
+  ASSERT_EQ(true, asoct.shutdown());
+  ASSERT_EQ(true, asoct.init(get_rand_socket_path()));
+  AdminSocketClient client(get_rand_socket_path());
+
+  {
+    string help;
+    ASSERT_EQ("", client.do_request("{\"prefix\":\"help\"}", &help));
+    ASSERT_NE(string::npos, help.find("\"list available commands\""));
+  }
+  {
+    string help;
+    ASSERT_EQ("", client.do_request("{"
+				    " \"prefix\":\"help\","
+				    " \"format\":\"xml\","
+				    "}", &help));
+    ASSERT_NE(string::npos, help.find(">list available commands<"));
+  }
+  {
+    string help;
+    ASSERT_EQ("", client.do_request("{"
+				    " \"prefix\":\"help\","
+				    " \"format\":\"UNSUPPORTED\","
+				    "}", &help));
+    ASSERT_NE(string::npos, help.find("\"list available commands\""));
+  }
+  ASSERT_EQ(true, asoct.shutdown());
+}
+
 TEST(AdminSocket, SendNoOp) {
   std::auto_ptr<AdminSocket>
       asokc(new AdminSocket(g_ceph_context));
@@ -146,3 +178,14 @@ TEST(AdminSocket, RegisterCommandPrefixes) {
   ASSERT_EQ("test| this thing", result);
   ASSERT_EQ(true, asoct.shutdown());
 }
+
+/*
+ * Local Variables:
+ * compile-command: "cd .. ;
+ *   make unittest_admin_socket &&
+ *    valgrind \
+ *    --max-stackframe=20000000 --tool=memcheck \
+ *   ./unittest_admin_socket # --gtest_filter=AdminSocket.*
+ * "
+ * End:
+ */