rgw: fix interface compliance of RGWCivetWeb::write_data().

This is a potential fix for partial writes and handling of error
codes that might come from mg_write() of the CivetWeb front-end.
None of the potential issues has been observed in testing.

The commit also documents the same aspects regarding the ASIO
and FastCGI front-ends.

Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>

diff --git a/src/rgw/rgw_asio_client.cc b/src/rgw/rgw_asio_client.cc
index aa920ecf96d455b396ae8f2dac46283a25645509..48381c310bbefaecb6c99d92fb331c8723cc44d1 100644 (file)
--- a/src/rgw/rgw_asio_client.cc
+++ b/src/rgw/rgw_asio_client.cc
@@ -86,8 +86,12 @@ size_t RGWAsioClientIO::write_data(const char* const buf,
   if (ec) {
     derr << "write_data failed: " << ec.message() << dendl;
     throw rgw::io::Exception(ec.value(), std::system_category());
+  } else {
+    /* According to the documentation of boost::asio::write if there is
+     * no error (signalised by ec), then bytes == len. We don't need to
+     * take care of partial writes in such situation. */
+    return bytes;
   }
-  return bytes;
 }
 
 size_t RGWAsioClientIO::read_data(char* const buf, const size_t max)

diff --git a/src/rgw/rgw_civetweb.cc b/src/rgw/rgw_civetweb.cc
index 1873a84a4eba1604e9bf5c008be94c514caeb684..eab43ab4f80c371ced1391fa18c8a78034b1dd43 100644 (file)
--- a/src/rgw/rgw_civetweb.cc
+++ b/src/rgw/rgw_civetweb.cc
@@ -12,16 +12,21 @@
 
 #define dout_subsys ceph_subsys_rgw
 
-size_t RGWCivetWeb::write_data(const char *buf, size_t len)
+size_t RGWCivetWeb::write_data(const char *buf, const size_t len)
 {
-  const int ret = mg_write(conn, buf, len);
-  if (ret == 0) {
-    /* didn't send anything, error out */
-    throw rgw::io::Exception(EIO, std::system_category());
-  } else if (ret < 0) {
-    throw rgw::io::Exception(-ret, std::system_category());
+  auto to_sent = len;
+  while (to_sent) {
+    const int ret = mg_write(conn, buf, len);
+    if (ret < 0 || ! ret) {
+      /* According to the documentation of mg_write() it always returns -1 on
+       * error. The details aren't available, so we will just throw EIO. Same
+       * goes to 0 that is associated with writing to a closed connection. */
+      throw rgw::io::Exception(EIO, std::system_category());
+    } else {
+      to_sent -= static_cast<size_t>(ret);
+    }
   }
-  return ret;
+  return len;
 }
 
 RGWCivetWeb::RGWCivetWeb(mg_connection* const conn, const int port)
@@ -36,7 +41,7 @@ size_t RGWCivetWeb::read_data(char *buf, size_t len)
 {
   const int ret = mg_read(conn, buf, len);
   if (ret < 0) {
-    throw rgw::io::Exception(-ret, std::system_category());
+    throw rgw::io::Exception(EIO, std::system_category());
   }
   return ret;
 }

diff --git a/src/rgw/rgw_fcgi.cc b/src/rgw/rgw_fcgi.cc
index 173f4539063145d7229a3332a0fb7476c4326916..90074b5c2a06a99ef6d70c43e745bd5a75f70f6b 100644 (file)
--- a/src/rgw/rgw_fcgi.cc
+++ b/src/rgw/rgw_fcgi.cc
@@ -6,7 +6,9 @@
 
 size_t RGWFCGX::write_data(const char* const buf, const size_t len)
 {
-  const auto ret = FCGX_PutStr(buf, len, fcgx->out);
+ /* According to the documentation of FCGX_PutStr if there is no error
+ * (signalised by negative return value), then always ret == len. */
+ const auto ret = FCGX_PutStr(buf, len, fcgx->out);
   if (ret < 0) {
     throw rgw::io::Exception(-ret, std::system_category());
   }