Fix row::move_as behavior breaking reusability

Fix blob fetching semantics in Oracle.

Previously, the into-type would take the blob object's LOB locator
object and bind that in the query. That works as long as there is only a
single Blob object being fetched from the database. However, if multiple
values are fetched consecutively, there would only be a single locator
object involved. In the best case, this leads to every new fetch
overriding the previous result and in the worst case, that previous
result has been turned into an independent blob object that has gone out
of scope, which leads to freeing of this shared locator.

With this commit, the into-type object will create its own locator
object and use that. When assigning the fetched result to a blob object,
the locator is copied such that the into-type and the blob keep
independent locator objects, i.e. the locator is no longer shared.

Fixes #1144.

Closes #1145.

include/private/soci-trivial-blob-backend.h

@@ -2,6 +2,7 @@
 #define SOCI_PRIVATE_SOCI_TRIVIAL_BLOB_BACKEND_H_INCLUDED
 
 #include "soci/soci-backend.h"
+#include "soci/session.h"
 
 #include <vector>
 #include <cstring>
@@ -22,6 +23,8 @@ namespace details
 class trivial_blob_backend : public details::blob_backend
 {
 public:
+    trivial_blob_backend(details::session_backend &backend) : session_(backend) {}
+
     std::size_t get_len() override { return buffer_.size(); }
 
     std::size_t read_from_start(void* buf, std::size_t toRead,
@@ -71,7 +74,10 @@ class trivial_blob_backend : public details::blob_backend
 
     const std::uint8_t *get_buffer() const { return buffer_.data(); }
 
+    details::session_backend &get_session_backend() override { return session_; }
+
 protected:
+    details::session_backend &session_;
     std::vector< std::uint8_t > buffer_;
 };
 

include/soci/blob.h

@@ -40,6 +40,7 @@ class SOCI_DECL blob
 
     // (Re)initializes this blob
     void initialize(session &s);
+    void initialize(details::blob_backend *backend);
 
     std::size_t get_len();
 

include/soci/db2/soci-db2.h

@@ -233,6 +233,7 @@ struct db2_blob_backend : details::blob_backend
     std::size_t write_from_start(const void* buf, std::size_t toWrite, std::size_t offset = 0) override;
     std::size_t append(const void* buf, std::size_t toWrite) override;
     void trim(std::size_t newLen) override;
+    details::session_backend &get_session_backend() override;
 
     db2_session_backend& session_;
 };

include/soci/empty/soci-empty.h

@@ -146,6 +146,8 @@ struct empty_blob_backend : details::blob_backend
     std::size_t append(const void* buf, std::size_t toWrite) override;
     void trim(std::size_t newLen) override;
 
+    details::session_backend &get_session_backend() override;
+
     empty_session_backend& session_;
 };
 

include/soci/firebird/soci-firebird.h

@@ -266,6 +266,8 @@ struct firebird_blob_backend : details::blob_backend
     std::size_t append(const void *buf, std::size_t toWrite) override;
     void trim(std::size_t newLen) override;
 
+    details::session_backend &get_session_backend() override;
+
     // Writes the current data into the database by allocating a new BLOB
     // object for it.
     //

include/soci/odbc/soci-odbc.h

@@ -306,6 +306,7 @@ struct odbc_blob_backend : details::blob_backend
     std::size_t write_from_start(const void *buf, std::size_t toWrite, std::size_t offset = 0) override;
     std::size_t append(const void *buf, std::size_t toWrite) override;
     void trim(std::size_t newLen) override;
+    details::session_backend &get_session_backend() override;
 
     odbc_session_backend &session_;
 };

include/soci/oracle/soci-oracle.h

@@ -279,12 +279,14 @@ struct oracle_blob_backend : details::blob_backend
 
     locator_t get_lob_locator() const;
 
-    void set_lob_locator(locator_t locator, bool initialized = true);
+    void set_lob_locator(const locator_t locator, bool initialized = true);
 
     void reset();
 
     void ensure_initialized();
 
+    details::session_backend &get_session_backend() override;
+
 private:
     std::size_t do_deprecated_read(std::size_t offset, void *buf, std::size_t toRead) override
     {

include/soci/postgresql/soci-postgresql.h

@@ -370,6 +370,8 @@ class postgresql_blob_backend : public details::blob_backend
 
     void reset();
 
+    details::session_backend &get_session_backend() override;
+
 private:
     postgresql_session_backend & session_;
     blob_details details_;

include/soci/row.h

@@ -97,6 +97,11 @@ class SOCI_DECL row
 
         T ret;
         type_conversion<T>::move_from_base(baseVal, *indicators_.at(pos), ret);
+
+        // Re-initialize the holder in order to be able to use this row object
+        // for binding to another data set
+        baseVal = T{};
+
         return ret;
     }
 

include/soci/soci-backend.h

@@ -294,6 +294,8 @@ class rowid_backend
     virtual ~rowid_backend() {}
 };
 
+class session_backend;
+
 // polymorphic blob backend
 
 class blob_backend
@@ -312,6 +314,8 @@ class blob_backend
 
     virtual void trim(std::size_t newLen) = 0;
 
+    virtual session_backend &get_session_backend() = 0;
+
     // Deprecated functions with backend-specific semantics preserved only for
     // compatibility.
     [[deprecated("Use read_from_start instead")]]

src/backends/db2/blob.cpp

@@ -53,6 +53,11 @@ void db2_blob_backend::trim(std::size_t /* newLen */)
     throw soci_error("BLOBs are not supported.");
 }
 
+details::session_backend &db2_blob_backend::get_session_backend()
+{
+    return session_;
+}
+
 #ifdef _MSC_VER
 # pragma warning(pop)
 #endif

src/backends/empty/blob.cpp

@@ -53,6 +53,11 @@ void empty_blob_backend::trim(std::size_t /* newLen */)
     throw soci_error("BLOBs are not supported.");
 }
 
+details::session_backend &empty_blob_backend::get_session_backend()
+{
+    return session_;
+}
+
 #ifdef _MSC_VER
 # pragma warning(pop)
 #endif

src/backends/firebird/blob.cpp

@@ -325,3 +325,8 @@ long firebird_blob_backend::getBLOBInfo()
 
     return total_length;
 }
+
+details::session_backend &firebird_blob_backend::get_session_backend()
+{
+    return session_;
+}

src/backends/mysql/blob.cpp

@@ -20,8 +20,8 @@
 using namespace soci;
 using namespace soci::details;
 
-mysql_blob_backend::mysql_blob_backend(mysql_session_backend &)
-    : details::trivial_blob_backend()
+mysql_blob_backend::mysql_blob_backend(mysql_session_backend &backend)
+    : details::trivial_blob_backend(backend)
 {
 }
 

src/backends/odbc/blob.cpp

@@ -53,6 +53,11 @@ void odbc_blob_backend::trim(std::size_t /* newLen */)
     throw soci_error("BLOBs are not supported.");
 }
 
+details::session_backend &odbc_blob_backend::get_session_backend()
+{
+    return session_;
+}
+
 #ifdef _MSC_VER
 # pragma warning(pop)
 #endif

src/backends/oracle/blob.cpp

@@ -146,7 +146,7 @@ oracle_blob_backend::locator_t oracle_blob_backend::get_lob_locator() const
     return lobp_;
 }
 
-void oracle_blob_backend::set_lob_locator(oracle_blob_backend::locator_t locator, bool initialized)
+void oracle_blob_backend::set_lob_locator(const oracle_blob_backend::locator_t locator, bool initialized)
 {
     // If we select a BLOB value into a BLOB object, then the post_fetch code in
     // the standard_into_type_backend will set this object's locator to the one it is
@@ -157,7 +157,11 @@ void oracle_blob_backend::set_lob_locator(oracle_blob_backend::locator_t locator
     {
         reset();
 
-        lobp_ = locator;
+        sword res = OCILobLocatorAssign(session_.svchp_, session_.errhp_, locator, &lobp_);
+        if (res != OCI_SUCCESS)
+        {
+            throw_oracle_soci_error(res, session_.errhp_);
+        }
     }
 
     initialized_ = initialized;
@@ -239,3 +243,8 @@ void oracle_blob_backend::ensure_initialized()
         initialized_ = true;
     }
 }
+
+details::session_backend &oracle_blob_backend::get_session_backend()
+{
+    return session_;
+}

src/backends/oracle/standard-into-type.cpp

@@ -23,6 +23,8 @@
 #include <ctime>
 #include <sstream>
 
+#include <oci.h>
+
 #ifdef _MSC_VER
 #pragma warning(disable:4355)
 #endif
@@ -157,11 +159,24 @@ void oracle_standard_into_type_backend::define_by_pos(
             oracle_blob_backend *bbe
                 = static_cast<oracle_blob_backend *>(b->get_backend());
 
-            // Reset the blob to ensure that a potentially open temporary BLOB gets
-            // freed before the locator is changed to point to a different BLOB by the
-            // to-be-executed statement (which would leave us with a dangling temporary BLOB)
+            // Reset the blob to ensure that even when the query fails,
+            // the blob does not contain leftover data from whatever it
+            // contained before.
             bbe->reset();
-            ociData_ = bbe->get_lob_locator();
+
+            // Allocate our very own LOB locator. We have to do this instead of
+            // reusing the locator from the existing Blob object in case the
+            // statement for which we are binding is going to be executed multiple
+            // times. If we borrowed the locator from the Blob object, consecutive
+            // queries would override a previous locator (which might belong
+            // to an independent Blob object by now).
+            sword res = OCIDescriptorAlloc(statement_.session_.envhp_,
+                reinterpret_cast<dvoid**>(&ociData_), OCI_DTYPE_LOB, 0, 0);
+            if (res != OCI_SUCCESS)
+            {
+                throw soci_error("Cannot allocate the LOB locator");
+            }
+
 
             size = sizeof(ociData_);
             data = &ociData_;
@@ -384,6 +399,7 @@ void oracle_standard_into_type_backend::clean_up()
 
     if (type_ == x_blob)
     {
+        OCIDescriptorFree(ociData_, OCI_DTYPE_LOB);
         ociData_ = NULL;
     }
 

src/backends/postgresql/blob.cpp

@@ -321,3 +321,8 @@ void postgresql_blob_backend::clone()
         lo_close(session_.conn_, old_details.fd);
     }
 }
+
+details::session_backend &postgresql_blob_backend::get_session_backend()
+{
+    return session_;
+}

src/backends/sqlite3/blob.cpp

@@ -13,8 +13,8 @@
 
 using namespace soci;
 
-sqlite3_blob_backend::sqlite3_blob_backend(sqlite3_session_backend &)
-    : details::trivial_blob_backend()
+sqlite3_blob_backend::sqlite3_blob_backend(sqlite3_session_backend &backend)
+    : details::trivial_blob_backend(backend)
 {
 }
 

src/core/blob.cpp

@@ -28,7 +28,12 @@ bool blob::is_valid() const
 
 void blob::initialize(session &session)
 {
-    backEnd_.reset(session.make_blob_backend());
+    initialize(session.make_blob_backend());
+}
+
+void blob::initialize(details::blob_backend *backend)
+{
+    backEnd_.reset(backend);
 }
 
 std::size_t blob::get_len()

src/core/row.cpp

@@ -119,5 +119,9 @@ blob row::move_as<blob>(std::size_t pos) const
 
     blob ret;
     type_conversion<blob>::move_from_base(baseVal, *indicators_.at(pos), ret);
+
+    // Re-initialize blob object so it can be used in further queries
+    baseVal.initialize(ret.get_backend()->get_session_backend().make_blob_backend());
+
     return ret;
 }

tests/common-tests.h

@@ -6853,17 +6853,21 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]")
     SECTION("Blob binding")
     {
         // Add data
-        soci::blob blob(sql);
+        soci::blob blob1(sql);
+        soci::blob blob2(sql);
         static_assert(10 <= sizeof(dummy_data), "Underlying assumption violated");
-        blob.write_from_start(dummy_data, 10);
-        const int id = 42;
-        sql << "insert into soci_test (id, b) values(:id, :b)", soci::use(id), soci::use(blob);
+        blob1.write_from_start(dummy_data, 10);
+        blob2.write_from_start(dummy_data, 10);
+        const int id1 = 42;
+        const int id2 = 42;
+        sql << "insert into soci_test (id, b) values(:id, :b)", soci::use(id1), soci::use(blob1);
+        sql << "insert into soci_test (id, b) values(:id, :b)", soci::use(id2), soci::use(blob2);
 
         SECTION("into")
         {
             soci::blob intoBlob(sql);
 
-            sql << "select b from soci_test where id=:id", soci::use(id), soci::into(intoBlob);
+            sql << "select b from soci_test where id=:id", soci::use(id1), soci::into(intoBlob);
 
             char buffer[20];
             std::size_t written = intoBlob.read_from_start(buffer, sizeof(buffer));
@@ -6875,7 +6879,7 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]")
         }
         SECTION("move_as")
         {
-            soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id", soci::use(id));
+            soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id", soci::use(id1));
             bool containedData = false;
             for (auto it = rowSet.begin(); it != rowSet.end(); ++it)
             {
@@ -6897,8 +6901,8 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]")
         }
         SECTION("reusing bound blob")
         {
-            int secondID = id + 1;
-            sql << "insert into soci_test(id, b) values(:id, :b)", soci::use(secondID), soci::use(blob);
+            int secondID = id2 + 1;
+            sql << "insert into soci_test(id, b) values(:id, :b)", soci::use(secondID), soci::use(blob2);
 
             // Selecting the blob associated with secondID should yield the same result as selecting the one for id
             soci::blob intoBlob(sql);