Develop (#324)

* 217: fix unused code.

* 217: unused code and warning

* disabled man pages for rmb

* Bugfix/283 virtual mailbox fetch metadata (#284)

* Develop (#282)

* 217: fix unused code.

* 217: unused code and warning

* disabled man pages for rmb

* #283: fetch metadata for mails in virtual mailbox

* #283: fix metadata date.saved. date.received virtual mailbox

* #283: version

* Feature/286 use guid from UUID string (#287)

* #256: use guid_128_from_uuid_string instead of guid_128_from_string, to support older uuid formats like RECORD or MICROSOFT, always use compact for printing

* version 0.0.25 preparations

* #286: build issue

* merge

* version

* #286: prefere cached mail guid.

* #286: determine if uuid has - hyphon, if true preseve it.

* fix non void return

* Feature/289 GitHub actions (#290)

* #289: build

* #289: build plugin.

* #289: submodules

* submodules

* upgrade git

* clean outdated repos

* Bugfix/UUID record format (#293)

* bugfix initialisaction rados_mail->deprecated_uid

* rados_mail creation and default value check

* #295: update index after full object has been moved (metadata) (#296)

* Bugfix/298 295 bugfixes (#300)

* #289: fix force-resync (always use INBOX)
#295: rbox_mail_set_expunge (play save)

* #298: move ceph objects the standard way.

* clean up log.

* #298: log warning if client connection times out.

* Feature/302 retry expunge connection timeout (#303)

* #302: retry in case of ceph connection timeout

* rpm

* #302: max_retry = 10

* max 10 retry, in case object not available, fail with error

* #304: use last know uid to restore email flags (#305)

* #306: re-assign unreferenced mail objects to inbox (#309)

* #310: read osd_max_object_size and fail if mail.size > osd_max_object_size (#311)

* version: 0.0.31

* #313: fix imap append crash (#314)

* version 0.0.32

* Bugfix/316 mailbox save (#317)

* #316: imap append (remove index entry twice in case of error)

* #316: changed storage for big attachments. (sync write)

* #316: operate api

* #316:  fix operate call

* #316: release preparations

* #316: fixed some legacy unit tests

* 316: review results

* #316: fix

* Bugfix/test output stream check (#323)

* init output stream and free mailbuffer

* build

* extracted write chunks

* mailsize -1;

* fix

* debug messages

* init stream any case

* added deprecation comment, writing chunks now default. new config param rbox_chunk_size default 10240 bytes

* missing mock class

* version

* disabled (temporarily storage tests)

* disabled storage tests( api change)

.github/workflows/build.yml

@@ -34,8 +34,5 @@ jobs:
     - name: configure
       run: ./autogen.sh && ./configure --with-dovecot=/usr/local/lib/dovecot --enable-maintainer-mode --enable-debug --with-integration-tests --enable-valgrind --enable-debug
     - name: build
-      run:  make clean install
-    - name: test-storage
-      run: src/tests/test_storage_mock_rbox
-    - name: test-storage-bugs
-      run: src/tests/test_storage_mock_rbox_bugs
+      run:  make clean install   
+

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Change Log
 
+
+## [0.0.34](https://github.com/ceph-dovecot/dovecot-ceph-plugin/tree/0.0.34) (2022-05-03)
+- #322: [BUGFIX] memory crash appending big attachments (bufferlist)
+- #322: [CONFIGURATION] new configuration setting rbox_chunk_size with default 10240 Bytes 
+
 ## [0.0.33](https://github.com/ceph-dovecot/dovecot-ceph-plugin/tree/0.0.33) (2022-04-27)
 - #316: wait synchronously for rados write operations
 

configure.ac

@@ -8,7 +8,9 @@
 
 AC_PREREQ([2.59])
 
-AC_INIT([dovecot-ceph-plugin], [0.0.33], [https://github.com/ceph-dovecot/dovecot-ceph-plugin/issues/new], ,[https://github.com/ceph-dovecot/dovecot-ceph-plugin])
+
+AC_INIT([dovecot-ceph-plugin], [0.0.34], [https://github.com/ceph-dovecot/dovecot-ceph-plugin/issues/new], ,[https://github.com/ceph-dovecot/dovecot-ceph-plugin])
+
 
 AC_CONFIG_AUX_DIR([.])
 AC_CONFIG_SRCDIR([src])

rpm/dovecot-ceph-plugin.spec

@@ -14,7 +14,7 @@
 Name:		dovecot-ceph-plugin
 Summary:	Dovecot Ceph RADOS plugins
 
-Version:	0.0.33
+Version:	0.0.34
 
 Release:	0%{?dist}
 URL:		https://github.com/ceph-dovecot/dovecot-ceph-plugin

src/librmb/rados-dovecot-ceph-cfg-impl.h

@@ -36,6 +36,7 @@ class RadosDovecotCephCfgImpl : public RadosDovecotCephCfg {
   const std::string &get_rados_save_log_file() override { return dovecot_cfg.get_rados_save_log_file(); }
   const std::string &get_pool_name_metadata_key() override { return dovecot_cfg.get_pool_name_metadata_key(); }
 
+  int get_chunk_size() override { return std::stoi(dovecot_cfg.get_chunk_size());}
   std::string &get_pool_name() override { return dovecot_cfg.get_pool_name(); }
 
   std::string &get_key_prefix_keywords() override { return dovecot_cfg.get_key_prefix_keywords(); }

src/librmb/rados-dovecot-ceph-cfg.h

@@ -41,6 +41,7 @@ class RadosDovecotCephCfg {
   virtual bool is_ceph_posix_bugfix_enabled() = 0;
   virtual bool is_ceph_aio_wait_for_safe_and_cb() = 0;
   virtual bool is_write_chunks() = 0;
+  virtual int get_chunk_size() = 0;
 
   virtual const std::string &get_pool_name_metadata_key() = 0;
   virtual const std::string &get_update_attributes_key() = 0;

src/librmb/rados-dovecot-config.cpp

@@ -27,7 +27,8 @@ RadosConfig::RadosConfig()
       save_log("rados_save_log"),
       rbox_check_empty_mailboxes("rados_check_empty_mailboxes"),
       rbox_ceph_aio_wait_for_safe_and_cb("rbox_ceph_aio_wait_for_safe_and_cb"),
-      rbox_ceph_write_chunks("rbox_ceph_write_chunks") {
+      rbox_ceph_write_chunks("rbox_ceph_write_chunks"),
+      rbox_chunk_size("rbox_chunk_size") {
   config[pool_name] = "mail_storage";
 
   config[rbox_cfg_object_name] = "rbox_cfg";
@@ -38,6 +39,7 @@ RadosConfig::RadosConfig()
   config[rbox_check_empty_mailboxes] = "false";
   config[rbox_ceph_aio_wait_for_safe_and_cb] = "false";
   config[rbox_ceph_write_chunks] = "false";
+  config[rbox_chunk_size] = "10240";
   is_valid = false;
 }
 
@@ -72,7 +74,8 @@ std::string RadosConfig::to_string() {
   ss << "  " << save_log << "=" << config[save_log] << std::endl;
   ss << "  " << rbox_check_empty_mailboxes << "=" << config[rbox_check_empty_mailboxes] << std::endl;
   ss << "  " << rbox_ceph_aio_wait_for_safe_and_cb << "=" << config[rbox_ceph_aio_wait_for_safe_and_cb] << std::endl;
-  ss << "  " << rbox_ceph_write_chunks << "=" << config[rbox_ceph_write_chunks]
+  ss << "  " << rbox_ceph_write_chunks << "=" << config[rbox_ceph_write_chunks] << std::endl;
+  ss << "  " << rbox_chunk_size << "=" << config[rbox_chunk_size]
      << std::endl;
   return ss.str();
 }

src/librmb/rados-dovecot-config.h

@@ -44,6 +44,8 @@ class RadosConfig {
 
   std::string &get_rbox_cfg_object_name() { return config[rbox_cfg_object_name]; }
 
+  const std::string &get_chunk_size() { return config[rbox_chunk_size]; }
+
   const std::string &get_rbox_cluster_name() { return config[rbox_cluster_name]; }
   const std::string &get_rados_username() { return config[rados_username]; }
   void update_metadata(const std::string &key, const char *value_);
@@ -82,6 +84,7 @@ class RadosConfig {
   std::string rbox_check_empty_mailboxes;
   std::string rbox_ceph_aio_wait_for_safe_and_cb;
   std::string rbox_ceph_write_chunks;
+  std::string rbox_chunk_size;
   bool is_valid;
 };
 

src/librmb/rados-storage-impl.cpp

@@ -42,6 +42,7 @@ RadosStorageImpl::RadosStorageImpl(RadosCluster *_cluster) {
 
 RadosStorageImpl::~RadosStorageImpl() {}
 
+//DEPRECATED!!!!! -> moved to rbox-save.cpp
 int RadosStorageImpl::split_buffer_and_exec_op(RadosMail *current_object,
                                                librados::ObjectWriteOperation *write_op_xattr,
                                                const uint64_t &max_write) {
@@ -51,7 +52,7 @@ int RadosStorageImpl::split_buffer_and_exec_op(RadosMail *current_object,
   }
 
   int ret_val = 0;
-  uint64_t write_buffer_size = current_object->get_mail_size();
+  uint64_t write_buffer_size = current_object->get_mail_size() -1;
 
   assert(max_write > 0);
 
@@ -95,6 +96,7 @@ int RadosStorageImpl::split_buffer_and_exec_op(RadosMail *current_object,
       tmp_buffer.substr_of(*current_object->get_mail_buffer(), offset, length);
       write_op.write(offset, tmp_buffer);
     }
+
     ret_val = get_io_ctx().operate(*current_object->get_oid(), &write_op);
     if(ret_val < 0){
       ret_val = -1;
@@ -106,6 +108,10 @@ int RadosStorageImpl::split_buffer_and_exec_op(RadosMail *current_object,
   current_object->set_completion(nullptr);
   current_object->set_active_op(0);
 
+  // free mail's buffer cause we don't need it anymore
+  librados::bufferlist *mail_buffer = current_object->get_mail_buffer();
+  delete mail_buffer;
+
   return ret_val;
 }
 
@@ -403,6 +409,7 @@ int RadosStorageImpl::copy(std::string &src_oid, const char *src_ns, std::string
   return ret;
 }
 
+// DEPRECATED => MOVED to rbox-save.cpp
 // if save_async = true, don't forget to call wait_for_rados_operations e.g. wait_for_write_operations_complete
 // to wait for completion and free resources.
 bool RadosStorageImpl::save_mail(librados::ObjectWriteOperation *write_op_xattr, 

src/storage-rbox/rbox-save.cpp

@@ -111,7 +111,6 @@ void rbox_index_append(struct mail_save_context *_ctx) {
 
   i_debug("added index for seq: %d",r_ctx->seq);
 
-
   mail_index_update_flags(r_ctx->trans, r_ctx->seq, MODIFY_REPLACE,
                           static_cast<enum mail_flags>(_ctx->data.flags & ~MAIL_RECENT));
 
@@ -196,6 +195,7 @@ void init_output_stream(mail_save_context *_ctx) {
   rbox_save_context *r_ctx = (struct rbox_save_context *)_ctx;
   struct rbox_mailbox *rbox = (struct rbox_mailbox *)_ctx->transaction->box;
   if (_ctx->data.output != NULL) {
+    i_debug("freing data output stream!");
     o_stream_unref(&_ctx->data.output);
   }
 
@@ -220,6 +220,7 @@ int rbox_save_begin(struct mail_save_context *_ctx, struct istream *input) {
     r_ctx->dest_mail_allocated = TRUE;
   }
   setup_mail_object(_ctx);
+
   // init stream in any case.
   init_output_stream(_ctx);
 
@@ -455,6 +456,84 @@ static void clean_up_write_finish(struct mail_save_context *_ctx) {
   FUNC_END();
 }
 
+int save_mail(RadosStorage *rados_storage,
+                             RadosMail *current_object,
+                             librados::ObjectWriteOperation *write_op_xattr,
+                             const uint64_t &max_write) {
+
+  int ret_val = 0;
+  uint64_t write_buffer_size = current_object->get_mail_size() -1; // write buffer size needs to be length -1
+
+  assert(max_write > 0);
+
+  if (write_buffer_size == 0 || max_write <= 0) {
+    ret_val = -1;
+    i_debug("write_buffer_size == 0 or max_write <=0 < -1" );
+    return ret_val;
+  }
+
+  ret_val = rados_storage->get_io_ctx().operate(*current_object->get_oid(), write_op_xattr);
+
+  if(ret_val< 0){
+    i_debug("write metadata did not work: %d",ret_val);
+    ret_val = -1;
+    return ret_val;
+  }
+
+  uint64_t rest = write_buffer_size % max_write;
+  int div = write_buffer_size / max_write + (rest > 0 ? 1 : 0);
+  for (int i = 0; i < div; ++i) {
+
+    // split the buffer.
+    librados::bufferlist tmp_buffer;
+
+    librados::ObjectWriteOperation write_op;
+
+    int offset = i * max_write;
+
+    uint64_t length = max_write;
+    if (write_buffer_size < ((i + 1) * length)) {
+      length = rest;
+    }
+#ifdef HAVE_ALLOC_HINT_2
+    write_op.set_alloc_hint2(write_buffer_size, length, librados::ALLOC_HINT_FLAG_COMPRESSIBLE);
+#else
+    write_op.set_alloc_hint(write_buffer_size, length);
+#endif
+    if (div == 1) {
+      i_debug("write full mail at once");
+      write_op.write(0, *current_object->get_mail_buffer());
+    } else {
+      i_debug("write chunk size %d, offset=%d,lenght=%d",write_buffer_size,offset,length);      
+      if(offset + length > write_buffer_size){
+        i_error("offset and length (%d) is bigger then write_buffer size (%d)", (offset+length), write_buffer_size);
+        return -1;
+      }else{
+        tmp_buffer.substr_of(*current_object->get_mail_buffer(), offset, length);
+      }
+      write_op.write(offset, tmp_buffer);
+    }
+
+    ret_val = rados_storage->get_io_ctx().operate(*current_object->get_oid(), &write_op);
+    i_debug("append mail (operate) return value: %d",ret_val);
+    if(ret_val < 0){
+      ret_val = -1;
+      break;
+    }
+  }
+  // deprecated unused
+  current_object->set_write_operation(nullptr);
+  current_object->set_completion(nullptr);
+  current_object->set_active_op(0);
+
+  i_debug("freeing mailbuffer");
+  // free mail's buffer cause we don't need it anymore
+  librados::bufferlist *mail_buffer = current_object->get_mail_buffer();
+  delete mail_buffer;
+
+  return ret_val;
+}
+
 int rbox_save_finish(struct mail_save_context *_ctx) {
   FUNC_START();
 
@@ -516,7 +595,8 @@ int rbox_save_finish(struct mail_save_context *_ctx) {
       // error mail size is null
       r_ctx->failed = true;
       i_error("ERROR, mailsize is <= 0 ");
-    } else {
+    } 
+    else {
 
       if (!zlib_plugin_active) {
         // write \0 to ceph (length()+1) if stream is not binary
@@ -543,17 +623,22 @@ int rbox_save_finish(struct mail_save_context *_ctx) {
         r_ctx->failed = true;  
       }else {
 
-        if (!r_storage->config->is_write_chunks()) {
-          i_debug("not write chunks enabled max write size: %d ", r_storage->s->get_max_write_size_bytes() );
-          r_ctx->failed = !r_storage->s->save_mail(&write_op, r_ctx->rados_mail);
-          i_debug("SAVE_MAIL result: %d", r_ctx->failed);
-        } else {
-          r_ctx->failed = r_storage->s->aio_operate(&r_storage->s->get_io_ctx(), *r_ctx->rados_mail->get_oid(),
-                                              r_ctx->rados_mail->get_completion(), &write_op) < 0;
-        }
+          time_t save_date = r_ctx->rados_mail->get_rados_save_date();
+          write_op.mtime(&save_date);  
+
+          uint32_t config_chunk_size = r_storage->config->get_chunk_size();
+          if(config_chunk_size > r_storage->s->get_max_write_size_bytes()){
+            config_chunk_size = r_storage->s->get_max_write_size_bytes();
+          }
+          i_debug("max chunk write size: %d ", config_chunk_size );
+
+          int ret = save_mail(r_storage->s,r_ctx->rados_mail, &write_op, config_chunk_size);          
+          r_ctx->failed = ret < 0;
+          i_debug("SAVE_MAIL result: %d", r_ctx->failed);        
+
       }
       if (r_ctx->failed) {
-        i_error("saved mail: %s failed metadata_count %ld, mail_size (%d)", r_ctx->rados_mail->get_oid()->c_str(),
+        i_error("saved mail: %s failed. Metadata_count %ld, mail_size (%d)", r_ctx->rados_mail->get_oid()->c_str(),
                 r_ctx->rados_mail->get_metadata()->size(), r_ctx->rados_mail->get_mail_size());
       }
       if (r_storage->save_log->is_open()) {
@@ -584,10 +669,10 @@ static int rbox_save_assign_uids(struct rbox_save_context *r_ctx, const ARRAY_TY
   FUNC_START();
 
   struct seq_range_iter iter;
-  unsigned int n = 0;
   uint32_t uid = -1;
 
   if (r_ctx->rados_mails.size() > 0) {
+    unsigned int n = 0;
     seq_range_array_iter_init(&iter, uids);
     struct rbox_storage *r_storage = (struct rbox_storage *)&r_ctx->mbox->storage->storage;
     RadosMetadata metadata;

src/tests/mocks/mock_test.h

@@ -152,6 +152,8 @@ class RadosDovecotCephCfgMock : public RadosDovecotCephCfg {
   MOCK_METHOD0(is_ceph_posix_bugfix_enabled, bool());
   MOCK_METHOD0(is_ceph_aio_wait_for_safe_and_cb, bool());
   MOCK_METHOD0(is_write_chunks, bool());
+  MOCK_METHOD0(get_chunk_size,int());
+
 
   MOCK_METHOD1(update_mail_attributes, void(const char *value));
   MOCK_METHOD1(update_updatable_attributes, void(const char *value));