From: Loic Dachary Date: Mon, 10 Feb 2014 22:42:38 +0000 (+0100) Subject: common: admin socket fallback to json-pretty format X-Git-Tag: v0.78~203^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=165e76d4d03ffcc490fd3c2ba60fb37372990d0a;p=ceph.git 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 --- diff --git a/src/client/Client.cc b/src/client/Client.cc index d7d9a507d3ea..ba39b1158aab 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -111,6 +111,8 @@ bool Client::CommandHook::call(std::string command, cmdmap_t& cmdmap, std::string format, bufferlist& out) { Formatter *f = new_formatter(format); + if (!f) + f = new_formatter("json-pretty"); f->open_object_section("result"); m_client->client_lock.Lock(); if (command == "mds_requests") diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc index 12e5868b409e..dc208c30f643 100644 --- a/src/common/admin_socket.cc +++ b/src/common/admin_socket.cc @@ -321,6 +321,9 @@ bool AdminSocket::do_accept() return false; } cmd_getval(m_cct, cmdmap, "format", format); + if (format != "json" && format != "json-pretty" && + format != "xml" && format != "xml-pretty") + format = "json-pretty"; cmd_getval(m_cct, cmdmap, "prefix", c); string firstword; @@ -444,6 +447,8 @@ public: HelpHook(AdminSocket *as) : m_as(as) {} bool call(string command, cmdmap_t &cmdmap, string format, bufferlist& out) { Formatter *f = new_formatter(format); + if (!f) + f = new_formatter("json-pretty"); f->open_object_section("help"); for (map::iterator p = m_as->m_help.begin(); p != m_as->m_help.end(); diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc index f7c2df5537e7..4ebf79e82b96 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -172,6 +172,8 @@ void CephContext::do_command(std::string command, cmdmap_t& cmdmap, std::string format, bufferlist *out) { Formatter *f = new_formatter(format); + if (!f) + f = new_formatter("json-pretty"); stringstream ss; for (cmdmap_t::iterator it = cmdmap.begin(); it != cmdmap.end(); ++it) { if (it->first != "prefix") { diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 38bb17194a36..97f8f42aeda9 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -907,9 +907,9 @@ public: bool OSD::asok_command(string command, cmdmap_t& cmdmap, string format, ostream& ss) { - if (format == "") - format = "json-pretty"; Formatter *f = new_formatter(format); + if (!f) + f = new_formatter("json-pretty"); if (command == "dump_ops_in_flight") { op_tracker.dump_ops_in_flight(f); } else if (command == "dump_historic_ops") { diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc index 3da9be9c1c03..ce48fba05007 100644 --- a/src/osdc/Objecter.cc +++ b/src/osdc/Objecter.cc @@ -2507,6 +2507,8 @@ bool Objecter::RequestStateHook::call(std::string command, cmdmap_t& cmdmap, std::string format, bufferlist& out) { Formatter *f = new_formatter(format); + if (!f) + f = new_formatter("json-pretty"); m_objecter->client_lock.Lock(); m_objecter->dump_requests(f); m_objecter->client_lock.Unlock(); diff --git a/src/test/Makefile.am b/src/test/Makefile.am index 83b83f4676ef..84cc69e11c1c 100644 --- a/src/test/Makefile.am +++ b/src/test/Makefile.am @@ -548,6 +548,11 @@ unittest_config_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL) unittest_config_CXXFLAGS = $(UNITTEST_CXXFLAGS) check_PROGRAMS += unittest_config +unittest_context_SOURCES = test/common/test_context.cc +unittest_context_LDADD = $(UNITTEST_LDADD) $(CEPH_GLOBAL) +unittest_context_CXXFLAGS = $(UNITTEST_CXXFLAGS) +check_PROGRAMS += unittest_context + unittest_heartbeatmap_SOURCES = test/heartbeat_map.cc unittest_heartbeatmap_LDADD = $(LIBCOMMON) $(UNITTEST_LDADD) $(CEPH_GLOBAL) unittest_heartbeatmap_CXXFLAGS = $(UNITTEST_CXXFLAGS) diff --git a/src/test/admin_socket.cc b/src/test/admin_socket.cc index 6f2a215b565d..6484740674f6 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 + 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 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: + */ diff --git a/src/test/common/test_context.cc b/src/test/common/test_context.cc new file mode 100644 index 000000000000..ca745c9a89bd --- /dev/null +++ b/src/test/common/test_context.cc @@ -0,0 +1,64 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab +/* + * Ceph - scalable distributed file system + * + * Copyright (C) 2014 Cloudwatt + * + * Author: Loic Dachary + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Library Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * 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 Library Public License for more details. + * + * + */ +#include "gtest/gtest.h" +#include "include/types.h" +#include "include/msgr.h" +#include "common/ceph_context.h" +#include "common/config.h" + +TEST(CephContext, do_command) +{ + CephContext *cct = (new CephContext(CEPH_ENTITY_TYPE_CLIENT))->get(); + + string key("key"); + string value("value"); + cct->_conf->set_val(key.c_str(), value.c_str(), false); + cmdmap_t cmdmap; + cmdmap["var"] = key; + + { + bufferlist out; + cct->do_command("config get", cmdmap, "xml", &out); + string s(out.c_str(), out.length()); + EXPECT_EQ("" + value + "", s); + } + + { + bufferlist out; + cct->do_command("config get", cmdmap, "UNSUPPORTED", &out); + string s(out.c_str(), out.length()); + EXPECT_EQ("{ \"key\": \"value\"}", s); + } + + cct->put(); +} + +/* + * Local Variables: + * compile-command: "cd ../.. ; + * make unittest_context && + * valgrind \ + * --max-stackframe=20000000 --tool=memcheck \ + * ./unittest_context # --gtest_filter=CephContext.* + * " + * End: + */