<common> fix formatter buffer out-of-bounds

  The length of buf in Formatter is 1024. If the output was truncated due to size limit,
  then the return value is the number of characters (excluding the terminating null byte)
  which would have been written to the final string if enough space had been available.
  So XMLFormatter::dump_format_va will create a string_view that out of bounds of buf.

  use boost small_vector to allocate more buffer when content is too
  long to fit in the buffer pre-alloced

Signed-off-by: liubingrun <liubr1@chinatelecom.cn>
(cherry picked from commit 207003c512396b1381abbf1a4929186c21827060)

Conflicts:
	src/test/common/test_json_formatter.cc missing test dump_inf_or_nan

diff --git a/src/common/Formatter.cc b/src/common/Formatter.cc
index b43e04a8bd09f2fad9b0a1a478f4bed1dda5edec..a653a07929b00658b690464353af711ab9c9d304 100644 (file)
--- a/src/common/Formatter.cc
+++ b/src/common/Formatter.cc
@@ -22,6 +22,8 @@
 #include <algorithm>
 #include <set>
 #include <limits>
+#include <utility>
+#include <boost/container/small_vector.hpp>
 
 // -----------------------
 namespace ceph {
@@ -345,10 +347,21 @@ std::ostream& JSONFormatter::dump_stream(std::string_view name)
 
 void JSONFormatter::dump_format_va(std::string_view name, const char *ns, bool quoted, const char *fmt, va_list ap)
 {
-  char buf[LARGE_SIZE];
-  vsnprintf(buf, LARGE_SIZE, fmt, ap);
+  auto buf = boost::container::small_vector<char, LARGE_SIZE>{
+      LARGE_SIZE, boost::container::default_init};
 
-  add_value(name, buf, quoted);
+  va_list ap_copy;
+  va_copy(ap_copy, ap);
+  int len = vsnprintf(buf.data(), buf.size(), fmt, ap_copy);
+  va_end(ap_copy);
+
+  if (std::cmp_greater_equal(len, buf.size())) {
+    // output was truncated, allocate a buffer large enough
+    buf.resize(len + 1, boost::container::default_init);
+    vsnprintf(buf.data(), buf.size(), fmt, ap);
+  }
+
+  add_value(name, buf.data(), quoted);
 }
 
 int JSONFormatter::get_len() const
@@ -530,15 +543,27 @@ std::ostream& XMLFormatter::dump_stream(std::string_view name)
 
 void XMLFormatter::dump_format_va(std::string_view name, const char *ns, bool quoted, const char *fmt, va_list ap)
 {
-  char buf[LARGE_SIZE];
-  size_t len = vsnprintf(buf, LARGE_SIZE, fmt, ap);
+  auto buf = boost::container::small_vector<char, LARGE_SIZE>{
+      LARGE_SIZE, boost::container::default_init};
+
+  va_list ap_copy;
+  va_copy(ap_copy, ap);
+  int len = vsnprintf(buf.data(), buf.size(), fmt, ap_copy);
+  va_end(ap_copy);
+
+  if (std::cmp_greater_equal(len, buf.size())) {
+    // output was truncated, allocate a buffer large enough
+    buf.resize(len + 1, boost::container::default_init);
+    vsnprintf(buf.data(), buf.size(), fmt, ap);
+  }
+
   auto e = get_xml_name(name);
 
   print_spaces();
   if (ns) {
-    m_ss << "<" << e << " xmlns=\"" << ns << "\">" << xml_stream_escaper(std::string_view(buf, len)) << "</" << e << ">";
+    m_ss << "<" << e << " xmlns=\"" << ns << "\">" << xml_stream_escaper(std::string_view(buf.data(), len)) << "</" << e << ">";
   } else {
-    m_ss << "<" << e << ">" << xml_stream_escaper(std::string_view(buf, len)) << "</" << e << ">";
+    m_ss << "<" << e << ">" << xml_stream_escaper(std::string_view(buf.data(), len)) << "</" << e << ">";
   }
 
   if (m_pretty)
@@ -907,14 +932,26 @@ void TableFormatter::dump_format_va(std::string_view name,
                                    const char *fmt, va_list ap)
 {
   finish_pending_string();
-  char buf[LARGE_SIZE];
-  vsnprintf(buf, LARGE_SIZE, fmt, ap);
+  auto buf = boost::container::small_vector<char, LARGE_SIZE>{
+      LARGE_SIZE, boost::container::default_init};
+
+  va_list ap_copy;
+  va_copy(ap_copy, ap);
+  int len = vsnprintf(buf.data(), buf.size(), fmt, ap_copy);
+  va_end(ap_copy);
+
+  if (std::cmp_greater_equal(len, buf.size())) {
+    // output was truncated, allocate a buffer large enough
+    buf.resize(len + 1, boost::container::default_init);
+    vsnprintf(buf.data(), buf.size(), fmt, ap); 
+  }
 
   size_t i = m_vec_index(name);
   if (ns) {
-    m_ss << ns << "." << buf;
-  } else
-    m_ss << buf;
+    m_ss << ns << "." << buf.data();
+  } else {
+    m_ss << buf.data();
+  }
 
   m_vec[i].push_back(std::make_pair(get_section_name(name), m_ss.str()));
   m_ss.clear();

diff --git a/src/test/common/test_json_formatter.cc b/src/test/common/test_json_formatter.cc
index 8a0f547a9298a7be32bef141c43aa0438143c9a6..983f1021527895e0e58d7fd221bc813447ec201c 100644 (file)
--- a/src/test/common/test_json_formatter.cc
+++ b/src/test/common/test_json_formatter.cc
@@ -79,3 +79,27 @@ TEST(formatter, utime)
   EXPECT_EQ(input.sec(), output.sec());
   EXPECT_EQ(input.nsec(), output.nsec());
 }
+
+TEST(formatter, dump_large_item) {
+  JSONFormatter formatter;
+  formatter.open_object_section("large_item");
+
+  std::string base_url("http://example.com");
+  std::string bucket_name("bucket");
+  std::string object_key(1024, 'a');
+
+  std::string full_url = base_url + "/" + bucket_name + "/" + object_key;
+  formatter.dump_format("Location", "%s/%s/%s", base_url.c_str(), bucket_name.c_str(), object_key.c_str());
+
+  formatter.close_section();
+  bufferlist bl;
+  formatter.flush(bl);
+
+  // std::cout << std::string(bl.c_str(), bl.length()) << std::endl;
+
+  JSONParser parser;
+  parser.parse(bl.c_str(), bl.length());
+
+  EXPECT_TRUE(parser.parse(bl.c_str(), bl.length()));
+  EXPECT_EQ(parser.find_obj("Location")->get_data(), full_url);
+}

diff --git a/src/test/common/test_tableformatter.cc b/src/test/common/test_tableformatter.cc
index b152014a2b5e3d67ea5ad64fd6d9ddaa86c3920a..90de133d315550e00d2c2bf1fbe6934f99df11db 100644 (file)
--- a/src/test/common/test_tableformatter.cc
+++ b/src/test/common/test_tableformatter.cc
@@ -250,6 +250,23 @@ TEST(tableformatter, multiline_keyval)
   EXPECT_EQ(cmp, sout.str());
 }
 
+TEST(tableformatter, dump_large_item) {
+  std::stringstream sout;
+  TableFormatter* formatter = (TableFormatter*) Formatter::create("table-kv");
+
+  std::string base_url("http://example.com");
+  std::string bucket_name("bucket");
+  std::string object_key(1024, 'a');
+
+  std::string full_url = base_url + "/" + bucket_name + "/" + object_key;
+  formatter->dump_format("Location", "%s/%s/%s", base_url.c_str(), bucket_name.c_str(), object_key.c_str());
+  formatter->flush(sout);
+  delete formatter;
+
+  std::string cmp = "key::Location=\"" + full_url + "\" \n";
+  EXPECT_EQ(cmp, sout.str());
+}
+
 /*
  * Local Variables:
  * compile-command: "cd ../.. ; make -j4 &&

diff --git a/src/test/common/test_xmlformatter.cc b/src/test/common/test_xmlformatter.cc
index 9ac6dde456e831d2e20e5217579ed1b6d93a9065..abbe9e4e25e94be3173c05f8f95639e153b4b963 100644 (file)
--- a/src/test/common/test_xmlformatter.cc
+++ b/src/test/common/test_xmlformatter.cc
@@ -163,3 +163,25 @@ TEST(xmlformatter, pretty_lowercased_underscored)
     "<string_item>String</string_item>\n\n";
   EXPECT_EQ(cmp, sout.str());
 }
+
+TEST(xmlformatter, dump_format_large_item)
+{
+  std::stringstream sout;
+  XMLFormatter formatter(
+      true,  // pretty
+      false, // lowercased
+      false); // underscored
+
+  std::string base_url("http://example.com");
+  std::string bucket_name("bucket");
+  std::string object_key(1024, 'a');
+
+  formatter.dump_format("Location", "%s/%s/%s", base_url.c_str(), bucket_name.c_str(), object_key.c_str());
+
+  formatter.flush(sout);
+
+  std::string uri = base_url + "/" + bucket_name + "/" + object_key;
+  std::string expected_output = "<Location>" + uri + "</Location>\n\n";
+
+  EXPECT_EQ(expected_output, sout.str());
+}
\ No newline at end of file