SSPCPP-656 - NameID insert logic appears wrong for ODBC Session store
[shibboleth/cpp-sp.git] / odbc-store / odbc-store.cpp
index 95d4945..9fea872 100644 (file)
@@ -1,23 +1,27 @@
-/*
- *  Copyright 2001-2007 Internet2
- * 
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
+/**
+ * Licensed to the University Corporation for Advanced Internet
+ * Development, Inc. (UCAID) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ *
+ * UCAID licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the
+ * License at
  *
- *     http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
+ * either express or implied. See the License for the specific
+ * language governing permissions and limitations under the License.
  */
 
 /**
  * odbc-store.cpp
  *
- * Storage Service using ODBC
+ * Storage Service using ODBC.
  */
 
 #if defined (_MSC_VER) || defined(__BORLANDC__)
 # define ODBCSTORE_EXPORTS
 #endif
 
-#include <xercesc/util/XMLUniDefs.hpp>
 #include <xmltooling/logging.h>
+#include <xmltooling/unicode.h>
 #include <xmltooling/XMLToolingConfig.h>
 #include <xmltooling/util/NDC.h>
 #include <xmltooling/util/StorageService.h>
 #include <xmltooling/util/Threads.h>
 #include <xmltooling/util/XMLHelper.h>
+#include <xercesc/util/XMLUniDefs.hpp>
 
 #include <sql.h>
 #include <sqlext.h>
 
+#include <boost/lexical_cast.hpp>
+#include <boost/algorithm/string.hpp>
+
 using namespace xmltooling::logging;
 using namespace xmltooling;
 using namespace xercesc;
+using namespace boost;
 using namespace std;
 
 #define PLUGIN_VER_MAJOR 1
-#define PLUGIN_VER_MINOR 0
+#define PLUGIN_VER_MINOR 1
 
 #define LONGDATA_BUFLEN 16384
 
@@ -65,17 +74,17 @@ using namespace std;
 #define STRING_TABLE "strings"
 #define TEXT_TABLE "texts"
 
-/* tables definitions
+/* table definitions
 CREATE TABLE version (
-    major tinyint NOT NULL,
-    minor tinyint NOT NULL
+    major int NOT nullptr,
+    minor int NOT nullptr
     )
 
 CREATE TABLE strings (
     context varchar(255) not null,
     id varchar(255) not null,
     expires datetime not null,
-    version smallint not null,
+    version int not null,
     value varchar(255) not null,
     PRIMARY KEY (context, id)
     )
@@ -84,7 +93,7 @@ CREATE TABLE texts (
     context varchar(255) not null,
     id varchar(255) not null,
     expires datetime not null,
-    version smallint not null,
+    version int not null,
     value text not null,
     PRIMARY KEY (context, id)
     )
@@ -92,27 +101,30 @@ CREATE TABLE texts (
 
 namespace {
     static const XMLCh cleanupInterval[] =  UNICODE_LITERAL_15(c,l,e,a,n,u,p,I,n,t,e,r,v,a,l);
+    static const XMLCh isolationLevel[] =   UNICODE_LITERAL_14(i,s,o,l,a,t,i,o,n,L,e,v,e,l);
     static const XMLCh ConnectionString[] = UNICODE_LITERAL_16(C,o,n,n,e,c,t,i,o,n,S,t,r,i,n,g);
+    static const XMLCh RetryOnError[] =     UNICODE_LITERAL_12(R,e,t,r,y,O,n,E,r,r,o,r);
+    static const XMLCh contextSize[] =      UNICODE_LITERAL_11(c,o,n,t,e,x,t,S,i,z,e);
+    static const XMLCh keySize[] =          UNICODE_LITERAL_7(k,e,y,S,i,z,e);
+    static const XMLCh stringSize[] =       UNICODE_LITERAL_10(s,t,r,i,n,g,S,i,z,e);
 
     // RAII for ODBC handles
     struct ODBCConn {
-        ODBCConn(SQLHDBC conn) : handle(conn) {}
+        ODBCConn(SQLHDBC conn) : handle(conn), autoCommit(true) {}
         ~ODBCConn() {
-            SQLRETURN sr = SQLEndTran(SQL_HANDLE_DBC, handle, SQL_COMMIT);
-            SQLDisconnect(handle);
-            SQLFreeHandle(SQL_HANDLE_DBC,handle);
-            if (!SQL_SUCCEEDED(sr))
-                throw IOException("Failed to commit connection.");
+            if (handle != SQL_NULL_HDBC) {
+                SQLRETURN sr = SQL_SUCCESS;
+                if (!autoCommit)
+                    sr = SQLSetConnectAttr(handle, SQL_ATTR_AUTOCOMMIT, (SQLPOINTER)SQL_AUTOCOMMIT_ON, 0);
+                SQLDisconnect(handle);
+                SQLFreeHandle(SQL_HANDLE_DBC, handle);
+                if (!SQL_SUCCEEDED(sr))
+                    throw IOException("Failed to commit connection and return to auto-commit mode.");
+            }
         }
         operator SQLHDBC() {return handle;}
         SQLHDBC handle;
-    };
-
-    struct ODBCStatement {
-        ODBCStatement(SQLHSTMT statement) : handle(statement) {}
-        ~ODBCStatement() {SQLFreeHandle(SQL_HANDLE_STMT,handle);}
-        operator SQLHSTMT() {return handle;}
-        SQLHSTMT handle;
+        bool autoCommit;
     };
 
     class ODBCStorageService : public StorageService
@@ -121,13 +133,17 @@ namespace {
         ODBCStorageService(const DOMElement* e);
         virtual ~ODBCStorageService();
 
+        const Capabilities& getCapabilities() const {
+            return m_caps;
+        }
+
         bool createString(const char* context, const char* key, const char* value, time_t expiration) {
             return createRow(STRING_TABLE, context, key, value, expiration);
         }
-        int readString(const char* context, const char* key, string* pvalue=NULL, time_t* pexpiration=NULL, int version=0) {
-            return readRow(STRING_TABLE, context, key, pvalue, pexpiration, version, false);
+        int readString(const char* context, const char* key, string* pvalue=nullptr, time_t* pexpiration=nullptr, int version=0) {
+            return readRow(STRING_TABLE, context, key, pvalue, pexpiration, version);
         }
-        int updateString(const char* context, const char* key, const char* value=NULL, time_t expiration=0, int version=0) {
+        int updateString(const char* context, const char* key, const char* value=nullptr, time_t expiration=0, int version=0) {
             return updateRow(STRING_TABLE, context, key, value, expiration, version);
         }
         bool deleteString(const char* context, const char* key) {
@@ -137,10 +153,10 @@ namespace {
         bool createText(const char* context, const char* key, const char* value, time_t expiration) {
             return createRow(TEXT_TABLE, context, key, value, expiration);
         }
-        int readText(const char* context, const char* key, string* pvalue=NULL, time_t* pexpiration=NULL, int version=0) {
-            return readRow(TEXT_TABLE, context, key, pvalue, pexpiration, version, true);
+        int readText(const char* context, const char* key, string* pvalue=nullptr, time_t* pexpiration=nullptr, int version=0) {
+            return readRow(TEXT_TABLE, context, key, pvalue, pexpiration, version);
         }
-        int updateText(const char* context, const char* key, const char* value=NULL, time_t expiration=0, int version=0) {
+        int updateText(const char* context, const char* key, const char* value=nullptr, time_t expiration=0, int version=0) {
             return updateRow(TEXT_TABLE, context, key, value, expiration, version);
         }
         bool deleteText(const char* context, const char* key) {
@@ -165,7 +181,7 @@ namespace {
 
     private:
         bool createRow(const char *table, const char* context, const char* key, const char* value, time_t expiration);
-        int readRow(const char *table, const char* context, const char* key, string* pvalue, time_t* pexpiration, int version, bool text);
+        int readRow(const char *table, const char* context, const char* key, string* pvalue, time_t* pexpiration, int version);
         int updateRow(const char *table, const char* context, const char* key, const char* value, time_t expiration, int version);
         bool deleteRow(const char *table, const char* context, const char* key);
 
@@ -175,20 +191,24 @@ namespace {
 
         SQLHDBC getHDBC();
         SQLHSTMT getHSTMT(SQLHDBC);
-        pair<int,int> getVersion(SQLHDBC);
-        bool log_error(SQLHANDLE handle, SQLSMALLINT htype, const char* checkfor=NULL);
+        pair<SQLINTEGER,SQLINTEGER> getVersion(SQLHDBC);
+        pair<bool,bool> log_error(SQLHANDLE handle, SQLSMALLINT htype, const char* checkfor=nullptr);
 
         static void* cleanup_fn(void*); 
         void cleanup();
 
         Category& m_log;
+        Capabilities m_caps;
         int m_cleanupInterval;
-        CondWait* shutdown_wait;
+        scoped_ptr<CondWait> shutdown_wait;
         Thread* cleanup_thread;
         bool shutdown;
 
         SQLHENV m_henv;
         string m_connstring;
+        long m_isolation;
+        bool m_wideVersion;
+        vector<SQLINTEGER> m_retries;
     };
 
     StorageService* ODBCStorageServiceFactory(const DOMElement* const & e)
@@ -228,48 +248,48 @@ namespace {
         strftime(ret,32,"{ts '%Y-%m-%d %H:%M:%S'}",ptime);
     }
 
-    // make a string safe for SQL command
-    // result to be free'd only if it isn't the input
-    static char *makeSafeSQL(const char *src)
-    {
-       int ns = 0;
-       int nc = 0;
-       char *s;
-    
-       // see if any conversion needed
-       for (s=(char*)src; *s; nc++,s++) if (*s=='\'') ns++;
-       if (ns==0) return ((char*)src);
-    
-       char *safe = new char[(nc+2*ns+1)];
-       for (s=safe; *src; src++) {
-           if (*src=='\'') *s++ = '\'';
-           *s++ = (char)*src;
-       }
-       *s = '\0';
-       return (safe);
-    }
+    class SQLString {
+        const char* m_src;
+        string m_copy;
+    public:
+        SQLString(const char* src) : m_src(src) {
+            if (strchr(src, '\'')) {
+                m_copy = src;
+                replace_all(m_copy, "'", "''");
+            }
+        }
 
-    void freeSafeSQL(char *safe, const char *src)
-    {
-        if (safe!=src)
-            delete[](safe);
-    }
+        operator const char*() const {
+            return tostr();
+        }
+
+        const char* tostr() const {
+            return m_copy.empty() ? m_src : m_copy.c_str();
+        }
+    };
 };
 
 ODBCStorageService::ODBCStorageService(const DOMElement* e) : m_log(Category::getInstance("XMLTooling.StorageService")),
-   m_cleanupInterval(900), shutdown_wait(NULL), cleanup_thread(NULL), shutdown(false), m_henv(SQL_NULL_HANDLE)
+    m_caps(XMLHelper::getAttrInt(e, 255, contextSize), XMLHelper::getAttrInt(e, 255, keySize), XMLHelper::getAttrInt(e, 255, stringSize)),
+    m_cleanupInterval(XMLHelper::getAttrInt(e, 900, cleanupInterval)),
+    cleanup_thread(nullptr), shutdown(false), m_henv(SQL_NULL_HENV), m_isolation(SQL_TXN_SERIALIZABLE), m_wideVersion(false)
 {
 #ifdef _DEBUG
     xmltooling::NDC ndc("ODBCStorageService");
 #endif
-
-    const XMLCh* tag=e ? e->getAttributeNS(NULL,cleanupInterval) : NULL;
-    if (tag && *tag)
-        m_cleanupInterval = XMLString::parseInt(tag);
-    if (!m_cleanupInterval)
-        m_cleanupInterval = 900;
-
-    if (m_henv == SQL_NULL_HANDLE) {
+    string iso(XMLHelper::getAttrString(e, "SERIALIZABLE", isolationLevel));
+    if (iso == "SERIALIZABLE")
+        m_isolation = SQL_TXN_SERIALIZABLE;
+    else if (iso == "REPEATABLE_READ")
+        m_isolation = SQL_TXN_REPEATABLE_READ;
+    else if (iso == "READ_COMMITTED")
+        m_isolation = SQL_TXN_READ_COMMITTED;
+    else if (iso == "READ_UNCOMMITTED")
+        m_isolation = SQL_TXN_READ_UNCOMMITTED;
+    else
+        throw XMLToolingException("Unknown transaction isolationLevel property.");
+
+    if (m_henv == SQL_NULL_HENV) {
         // Enable connection pooling.
         SQLSetEnvAttr(SQL_NULL_HANDLE, SQL_ATTR_CONNECTION_POOLING, (void*)SQL_CP_ONE_PER_HENV, 0);
 
@@ -284,17 +304,17 @@ ODBCStorageService::ODBCStorageService(const DOMElement* e) : m_log(Category::ge
     }
 
     // Grab connection string from the configuration.
-    e = e ? XMLHelper::getFirstChildElement(e,ConnectionString) : NULL;
-    if (!e || !e->hasChildNodes()) {
+    e = e ? XMLHelper::getFirstChildElement(e, ConnectionString) : nullptr;
+    auto_ptr_char arg(e ? e->getTextContent() : nullptr);
+    if (!arg.get() || !*arg.get()) {
         SQLFreeHandle(SQL_HANDLE_ENV, m_henv);
         throw XMLToolingException("ODBC StorageService requires ConnectionString element in configuration.");
     }
-    auto_ptr_char arg(e->getFirstChild()->getNodeValue());
-    m_connstring=arg.get();
+    m_connstring = arg.get();
 
     // Connect and check version.
     ODBCConn conn(getHDBC());
-    pair<int,int> v=getVersion(conn);
+    pair<SQLINTEGER,SQLINTEGER> v = getVersion(conn);
 
     // Make sure we've got the right version.
     if (v.first != PLUGIN_VER_MAJOR) {
@@ -302,9 +322,30 @@ ODBCStorageService::ODBCStorageService(const DOMElement* e) : m_log(Category::ge
         m_log.crit("unknown database version: %d.%d", v.first, v.second);
         throw XMLToolingException("Unknown database version for ODBC StorageService.");
     }
+    
+    if (v.first > 1 || v.second > 0) {
+        m_log.info("using 32-bit int type for version fields in tables");
+        m_wideVersion = true;
+    }
+
+    // Load any retry errors to check.
+    e = XMLHelper::getNextSiblingElement(e, RetryOnError);
+    while (e) {
+        if (e->hasChildNodes()) {
+            try {
+                int code = XMLString::parseInt(e->getTextContent());
+                m_retries.push_back(code);
+                m_log.info("will retry operations when native ODBC error (%d) is returned", code);
+            }
+            catch (XMLException&) {
+                m_log.error("skipping non-numeric ODBC retry code");
+            }
+        }
+        e = XMLHelper::getNextSiblingElement(e, RetryOnError);
+    }
 
     // Initialize the cleanup thread
-    shutdown_wait = CondWait::create();
+    shutdown_wait.reset(CondWait::create());
     cleanup_thread = Thread::create(&cleanup_fn, (void*)this);
 }
 
@@ -312,12 +353,12 @@ ODBCStorageService::~ODBCStorageService()
 {
     shutdown = true;
     shutdown_wait->signal();
-    cleanup_thread->join(NULL);
-    delete shutdown_wait;
-    SQLFreeHandle(SQL_HANDLE_ENV, m_henv);
+    cleanup_thread->join(nullptr);
+    if (m_henv != SQL_NULL_HANDLE)
+        SQLFreeHandle(SQL_HANDLE_ENV, m_henv);
 }
 
-bool ODBCStorageService::log_error(SQLHANDLE handle, SQLSMALLINT htype, const char* checkfor)
+pair<bool,bool> ODBCStorageService::log_error(SQLHANDLE handle, SQLSMALLINT htype, const char* checkfor)
 {
     SQLSMALLINT         i = 0;
     SQLINTEGER  native;
@@ -326,13 +367,15 @@ bool ODBCStorageService::log_error(SQLHANDLE handle, SQLSMALLINT htype, const ch
     SQLSMALLINT         len;
     SQLRETURN   ret;
 
-    bool res = false;
+    pair<bool,bool> res = make_pair(false,false);
     do {
         ret = SQLGetDiagRec(htype, handle, ++i, state, &native, text, sizeof(text), &len);
         if (SQL_SUCCEEDED(ret)) {
             m_log.error("ODBC Error: %s:%ld:%ld:%s", state, i, native, text);
+            for (vector<SQLINTEGER>::const_iterator n = m_retries.begin(); !res.first && n != m_retries.end(); ++n)
+                res.first = (*n == native);
             if (checkfor && !strcmp(checkfor, (const char*)state))
-                res = true;
+                res.second = true;
         }
     } while(SQL_SUCCEEDED(ret));
     return res;
@@ -345,36 +388,37 @@ SQLHDBC ODBCStorageService::getHDBC()
 #endif
 
     // Get a handle.
-    SQLHDBC handle;
-    SQLRETURN sr=SQLAllocHandle(SQL_HANDLE_DBC, m_henv, &handle);
-    if (!SQL_SUCCEEDED(sr)) {
+    SQLHDBC handle = SQL_NULL_HDBC;
+    SQLRETURN sr = SQLAllocHandle(SQL_HANDLE_DBC, m_henv, &handle);
+    if (!SQL_SUCCEEDED(sr) || handle == SQL_NULL_HDBC) {
         m_log.error("failed to allocate connection handle");
         log_error(m_henv, SQL_HANDLE_ENV);
         throw IOException("ODBC StorageService failed to allocate a connection handle.");
     }
 
-    sr=SQLDriverConnect(handle,NULL,(SQLCHAR*)m_connstring.c_str(),m_connstring.length(),NULL,0,NULL,SQL_DRIVER_NOPROMPT);
+    sr = SQLDriverConnect(handle,nullptr,(SQLCHAR*)m_connstring.c_str(),m_connstring.length(),nullptr,0,nullptr,SQL_DRIVER_NOPROMPT);
     if (!SQL_SUCCEEDED(sr)) {
         m_log.error("failed to connect to database");
         log_error(handle, SQL_HANDLE_DBC);
+        SQLFreeHandle(SQL_HANDLE_DBC, handle);
         throw IOException("ODBC StorageService failed to connect to database.");
     }
 
-    sr = SQLSetConnectAttr(handle, SQL_ATTR_AUTOCOMMIT, SQL_AUTOCOMMIT_OFF, NULL);
-    if (!SQL_SUCCEEDED(sr))
-        throw IOException("ODBC StorageService failed to disable auto-commit mode.");
-    sr = SQLSetConnectAttr(handle, SQL_ATTR_TXN_ISOLATION, (SQLPOINTER)SQL_TXN_SERIALIZABLE, NULL);
-    if (!SQL_SUCCEEDED(sr))
-        throw IOException("ODBC StorageService failed to enable transaction isolation.");
+    sr = SQLSetConnectAttr(handle, SQL_ATTR_TXN_ISOLATION, (SQLPOINTER)m_isolation, 0);
+    if (!SQL_SUCCEEDED(sr)) {
+        SQLDisconnect(handle);
+        SQLFreeHandle(SQL_HANDLE_DBC, handle);
+        throw IOException("ODBC StorageService failed to set transaction isolation level.");
+    }
 
     return handle;
 }
 
 SQLHSTMT ODBCStorageService::getHSTMT(SQLHDBC conn)
 {
-    SQLHSTMT hstmt;
-    SQLRETURN sr=SQLAllocHandle(SQL_HANDLE_STMT,conn,&hstmt);
-    if (!SQL_SUCCEEDED(sr)) {
+    SQLHSTMT hstmt = SQL_NULL_HSTMT;
+    SQLRETURN sr = SQLAllocHandle(SQL_HANDLE_STMT, conn, &hstmt);
+    if (!SQL_SUCCEEDED(sr) || hstmt == SQL_NULL_HSTMT) {
         m_log.error("failed to allocate statement handle");
         log_error(conn, SQL_HANDLE_DBC);
         throw IOException("ODBC StorageService failed to allocate a statement handle.");
@@ -382,12 +426,12 @@ SQLHSTMT ODBCStorageService::getHSTMT(SQLHDBC conn)
     return hstmt;
 }
 
-pair<int,int> ODBCStorageService::getVersion(SQLHDBC conn)
+pair<SQLINTEGER,SQLINTEGER> ODBCStorageService::getVersion(SQLHDBC conn)
 {
     // Grab the version number from the database.
-    ODBCStatement stmt(getHSTMT(conn));
+    SQLHSTMT stmt = getHSTMT(conn);
     
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)"SELECT major,minor FROM version", SQL_NTS);
+    SQLRETURN sr = SQLExecDirect(stmt, (SQLCHAR*)"SELECT major,minor FROM version", SQL_NTS);
     if (!SQL_SUCCEEDED(sr)) {
         m_log.error("failed to read version from database");
         log_error(stmt, SQL_HANDLE_STMT);
@@ -396,17 +440,17 @@ pair<int,int> ODBCStorageService::getVersion(SQLHDBC conn)
 
     SQLINTEGER major;
     SQLINTEGER minor;
-    SQLBindCol(stmt,1,SQL_C_SLONG,&major,0,NULL);
-    SQLBindCol(stmt,2,SQL_C_SLONG,&minor,0,NULL);
+    SQLBindCol(stmt, 1, SQL_C_SLONG, &major, 0, nullptr);
+    SQLBindCol(stmt, 2, SQL_C_SLONG, &minor, 0, nullptr);
 
-    if ((sr=SQLFetch(stmt)) != SQL_NO_DATA)
-        return pair<int,int>(major,minor);
+    if ((sr = SQLFetch(stmt)) != SQL_NO_DATA)
+        return make_pair(major,minor);
 
     m_log.error("no rows returned in version query");
     throw IOException("ODBC StorageService failed to read version from database.");
 }
 
-bool ODBCStorageService::createRow(const char *table, const char* context, const char* key, const char* value, time_t expiration)
+bool ODBCStorageService::createRow(const chartable, const char* context, const char* key, const char* value, time_t expiration)
 {
 #ifdef _DEBUG
     xmltooling::NDC ndc("createRow");
@@ -417,31 +461,73 @@ bool ODBCStorageService::createRow(const char *table, const char* context, const
 
     // Get statement handle.
     ODBCConn conn(getHDBC());
-    ODBCStatement stmt(getHSTMT(conn));
-
-    // Prepare and exectute insert statement.
-    char *scontext = makeSafeSQL(context);
-    char *skey = makeSafeSQL(key);
-    char *svalue = makeSafeSQL(value);
-    string q  = string("INSERT ") + table + " VALUES ('" + scontext + "','" + skey + "'," + timebuf + ",1,'" + svalue + "')";
-    freeSafeSQL(scontext, context);
-    freeSafeSQL(skey, key);
-    freeSafeSQL(svalue, value);
-    m_log.debug("SQL: %s", q.c_str());
+    SQLHSTMT stmt = getHSTMT(conn);
 
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
+    string q  = string("INSERT INTO ") + table + " VALUES (?,?," + timebuf + ",1,?)";
+
+    SQLRETURN sr = SQLPrepare(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
     if (!SQL_SUCCEEDED(sr)) {
-        m_log.error("insert record failed (t=%s, c=%s, k=%s)", table, context, key);
-        if (log_error(stmt, SQL_HANDLE_STMT, "23000"))
-            return false;   // supposedly integrity violation?
+        m_log.error("SQLPrepare failed (t=%s, c=%s, k=%s)", table, context, key);
+        log_error(stmt, SQL_HANDLE_STMT);
         throw IOException("ODBC StorageService failed to insert record.");
     }
-    return true;
+    m_log.debug("SQLPrepare succeeded. SQL: %s", q.c_str());
+
+    SQLLEN b_ind = SQL_NTS;
+    sr = SQLBindParam(stmt, 1, SQL_C_CHAR, SQL_VARCHAR, 255, 0, const_cast<char*>(context), &b_ind);
+    if (!SQL_SUCCEEDED(sr)) {
+        m_log.error("SQLBindParam failed (context = %s)", context);
+        log_error(stmt, SQL_HANDLE_STMT);
+        throw IOException("ODBC StorageService failed to insert record.");
+    }
+    m_log.debug("SQLBindParam succeeded (context = %s)", context);
+
+    sr = SQLBindParam(stmt, 2, SQL_C_CHAR, SQL_VARCHAR, 255, 0, const_cast<char*>(key), &b_ind);
+    if (!SQL_SUCCEEDED(sr)) {
+        m_log.error("SQLBindParam failed (key = %s)", key);
+        log_error(stmt, SQL_HANDLE_STMT);
+        throw IOException("ODBC StorageService failed to insert record.");
+    }
+    m_log.debug("SQLBindParam succeeded (key = %s)", key);
+
+    if (strcmp(table, TEXT_TABLE)==0)
+        sr = SQLBindParam(stmt, 3, SQL_C_CHAR, SQL_LONGVARCHAR, strlen(value), 0, const_cast<char*>(value), &b_ind);
+    else
+        sr = SQLBindParam(stmt, 3, SQL_C_CHAR, SQL_VARCHAR, 255, 0, const_cast<char*>(value), &b_ind);
+    if (!SQL_SUCCEEDED(sr)) {
+        m_log.error("SQLBindParam failed (value = %s)", value);
+        log_error(stmt, SQL_HANDLE_STMT);
+        throw IOException("ODBC StorageService failed to insert record.");
+    }
+    m_log.debug("SQLBindParam succeeded (value = %s)", value);
+    
+    int attempts = 3;
+    pair<bool,bool> logres;
+    do {
+        logres = make_pair(false,false);
+        attempts--;
+        sr = SQLExecute(stmt);
+        if (SQL_SUCCEEDED(sr)) {
+            m_log.debug("SQLExecute of insert succeeded");
+            return true;
+        }
+        m_log.error("insert record failed (t=%s, c=%s, k=%s)", table, context, key);
+        logres = log_error(stmt, SQL_HANDLE_STMT, "23000");
+        if (logres.second) {
+            // Supposedly integrity violation.
+            // Try and delete any expired record still hanging around until the final attempt.
+            if (attempts > 0) {
+                reap(table, context);
+                continue;
+            }
+            return false;
+        }
+    } while (attempts && logres.first);
+
+    throw IOException("ODBC StorageService failed to insert record.");
 }
 
-int ODBCStorageService::readRow(
-    const char *table, const char* context, const char* key, string* pvalue, time_t* pexpiration, int version, bool text
-    )
+int ODBCStorageService::readRow(const char *table, const char* context, const char* key, string* pvalue, time_t* pexpiration, int version)
 {
 #ifdef _DEBUG
     xmltooling::NDC ndc("readRow");
@@ -449,26 +535,25 @@ int ODBCStorageService::readRow(
 
     // Get statement handle.
     ODBCConn conn(getHDBC());
-    ODBCStatement stmt(getHSTMT(conn));
+    SQLHSTMT stmt = getHSTMT(conn);
 
     // Prepare and exectute select statement.
     char timebuf[32];
-    timestampFromTime(time(NULL), timebuf);
-    char *scontext = makeSafeSQL(context);
-    char *skey = makeSafeSQL(key);
-    ostringstream q;
-    q << "SELECT version";
+    timestampFromTime(time(nullptr), timebuf);
+    SQLString scontext(context);
+    SQLString skey(key);
+    string q("SELECT version");
     if (pexpiration)
-        q << ",expires";
-    if (pvalue)
-        q << ",CASE version WHEN " << version << " THEN NULL ELSE value END";
-    q << " FROM " << table << " WHERE context='" << scontext << "' AND id='" << skey << "' AND expires > " << timebuf;
-    freeSafeSQL(scontext, context);
-    freeSafeSQL(skey, key);
+        q += ",expires";
+    if (pvalue) {
+        pvalue->erase();
+        q = q + ",CASE version WHEN " + lexical_cast<string>(version) + " THEN null ELSE value END";
+    }
+    q = q + " FROM " + table + " WHERE context='" + scontext.tostr() + "' AND id='" + skey.tostr() + "' AND expires > " + timebuf;
     if (m_log.isDebugEnabled())
-        m_log.debug("SQL: %s", q.str().c_str());
+        m_log.debug("SQL: %s", q.c_str());
 
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.str().c_str(), SQL_NTS);
+    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
     if (!SQL_SUCCEEDED(sr)) {
         m_log.error("error searching for (t=%s, c=%s, k=%s)", table, context, key);
         log_error(stmt, SQL_HANDLE_STMT);
@@ -476,25 +561,35 @@ int ODBCStorageService::readRow(
     }
 
     SQLSMALLINT ver;
+    SQLINTEGER widever;
     SQL_TIMESTAMP_STRUCT expiration;
 
-    SQLBindCol(stmt,1,SQL_C_SSHORT,&ver,0,NULL);
+    if (m_wideVersion)
+        SQLBindCol(stmt, 1, SQL_C_SLONG, &widever, 0, nullptr);
+    else
+        SQLBindCol(stmt, 1, SQL_C_SSHORT, &ver, 0, nullptr);
     if (pexpiration)
-        SQLBindCol(stmt,2,SQL_C_TYPE_TIMESTAMP,&expiration,0,NULL);
+        SQLBindCol(stmt, 2, SQL_C_TYPE_TIMESTAMP, &expiration, 0, nullptr);
 
-    if ((sr=SQLFetch(stmt)) == SQL_NO_DATA)
+    if ((sr = SQLFetch(stmt)) == SQL_NO_DATA) {
+        if (m_log.isDebugEnabled())
+            m_log.debug("search returned no data (t=%s, c=%s, k=%s)", table, context, key);
         return 0;
+    }
 
     if (pexpiration)
         *pexpiration = timeFromTimestamp(expiration);
 
-    if (version == ver)
+    if (version == (m_wideVersion ? widever : ver)) {
+        if (m_log.isDebugEnabled())
+            m_log.debug("versioned search detected no change (t=%s, c=%s, k=%s)", table, context, key);
         return version; // nothing's changed, so just echo back the version
+    }
 
     if (pvalue) {
-        SQLINTEGER len;
+        SQLLEN len;
         SQLCHAR buf[LONGDATA_BUFLEN];
-        while ((sr=SQLGetData(stmt,pexpiration ? 3 : 2,SQL_C_CHAR,buf,sizeof(buf),&len)) != SQL_NO_DATA) {
+        while ((sr = SQLGetData(stmt, (pexpiration ? 3 : 2), SQL_C_CHAR, buf, sizeof(buf), &len)) != SQL_NO_DATA) {
             if (!SQL_SUCCEEDED(sr)) {
                 m_log.error("error while reading text field from result set");
                 log_error(stmt, SQL_HANDLE_STMT);
@@ -504,7 +599,7 @@ int ODBCStorageService::readRow(
         }
     }
     
-    return ver;
+    return (m_wideVersion ? widever : ver);
 }
 
 int ODBCStorageService::updateRow(const char *table, const char* context, const char* key, const char* value, time_t expiration, int version)
@@ -516,52 +611,58 @@ int ODBCStorageService::updateRow(const char *table, const char* context, const
     if (!value && !expiration)
         throw IOException("ODBC StorageService given invalid update instructions.");
 
-    // Get statement handle.
+    // Get statement handle. Disable auto-commit mode to wrap select + update.
     ODBCConn conn(getHDBC());
-    ODBCStatement stmt(getHSTMT(conn));
+    SQLRETURN sr = SQLSetConnectAttr(conn, SQL_ATTR_AUTOCOMMIT, SQL_AUTOCOMMIT_OFF, 0);
+    if (!SQL_SUCCEEDED(sr))
+        throw IOException("ODBC StorageService failed to disable auto-commit mode.");
+    conn.autoCommit = false;
+    SQLHSTMT stmt = getHSTMT(conn);
 
     // First, fetch the current version for later, which also ensures the record still exists.
     char timebuf[32];
-    timestampFromTime(time(NULL), timebuf);
-    char *scontext = makeSafeSQL(context);
-    char *skey = makeSafeSQL(key);
+    timestampFromTime(time(nullptr), timebuf);
+    SQLString scontext(context);
+    SQLString skey(key);
     string q("SELECT version FROM ");
-    q = q + table + " WHERE context='" + scontext + "' AND id='" + key + "' AND expires > " + timebuf;
+    q = q + table + " WHERE context='" + scontext.tostr() + "' AND id='" + skey.tostr() + "' AND expires > " + timebuf;
 
     m_log.debug("SQL: %s", q.c_str());
 
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
+    sr = SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
     if (!SQL_SUCCEEDED(sr)) {
-        freeSafeSQL(scontext, context);
-        freeSafeSQL(skey, key);
         m_log.error("error searching for (t=%s, c=%s, k=%s)", table, context, key);
         log_error(stmt, SQL_HANDLE_STMT);
         throw IOException("ODBC StorageService search failed.");
     }
 
     SQLSMALLINT ver;
-    SQLBindCol(stmt,1,SQL_C_SSHORT,&ver,0,NULL);
-    if ((sr=SQLFetch(stmt)) == SQL_NO_DATA) {
-        freeSafeSQL(scontext, context);
-        freeSafeSQL(skey, key);
+    SQLINTEGER widever;
+    if (m_wideVersion)
+        SQLBindCol(stmt, 1, SQL_C_SLONG, &widever, 0, nullptr);
+    else
+        SQLBindCol(stmt, 1, SQL_C_SSHORT, &ver, 0, nullptr);
+    if ((sr = SQLFetch(stmt)) == SQL_NO_DATA) {
         return 0;
     }
 
     // Check version?
-    if (version > 0 && version != ver) {
-        freeSafeSQL(scontext, context);
-        freeSafeSQL(skey, key);
+    if (version > 0 && version != (m_wideVersion ? widever : ver)) {
         return -1;
     }
+    else if ((m_wideVersion && widever == INT_MAX) || (!m_wideVersion && ver == 32767)) {
+        m_log.error("record version overflow (t=%s, c=%s, k=%s)", table, context, key);
+        throw IOException("Version overflow, record in ODBC StorageService could not be updated.");
+    }
+
+    SQLFreeHandle(SQL_HANDLE_STMT, stmt);
+    stmt = getHSTMT(conn);
 
     // Prepare and exectute update statement.
     q = string("UPDATE ") + table + " SET ";
 
-    if (value) {
-        char *svalue = makeSafeSQL(value);
-        q = q + "value='" + svalue + "'" + ",version=version+1";
-        freeSafeSQL(svalue, value);
-    }
+    if (value)
+        q = q + "value=?, version=version+1";
 
     if (expiration) {
         timestampFromTime(expiration, timebuf);
@@ -570,21 +671,48 @@ int ODBCStorageService::updateRow(const char *table, const char* context, const
         q = q + "expires = " + timebuf;
     }
 
-    q = q + " WHERE context='" + scontext + "' AND id='" + key + "'";
-    freeSafeSQL(scontext, context);
-    freeSafeSQL(skey, key);
+    q = q + " WHERE context='" + scontext.tostr() + "' AND id='" + skey.tostr() + "'";
 
-    m_log.debug("SQL: %s", q.c_str());
-    sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
-    if (sr==SQL_NO_DATA)
-        return 0;   // went missing?
-    else if (!SQL_SUCCEEDED(sr)) {
+    sr = SQLPrepare(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
+    if (!SQL_SUCCEEDED(sr)) {
         m_log.error("update of record failed (t=%s, c=%s, k=%s", table, context, key);
         log_error(stmt, SQL_HANDLE_STMT);
         throw IOException("ODBC StorageService failed to update record.");
     }
+    m_log.debug("SQLPrepare succeeded. SQL: %s", q.c_str());
 
-    return ver + 1;
+    SQLLEN b_ind = SQL_NTS;
+    if (value) {
+        if (strcmp(table, TEXT_TABLE)==0)
+            sr = SQLBindParam(stmt, 1, SQL_C_CHAR, SQL_LONGVARCHAR, strlen(value), 0, const_cast<char*>(value), &b_ind);
+        else
+            sr = SQLBindParam(stmt, 1, SQL_C_CHAR, SQL_VARCHAR, 255, 0, const_cast<char*>(value), &b_ind);
+        if (!SQL_SUCCEEDED(sr)) {
+            m_log.error("SQLBindParam failed (value = %s)", value);
+            log_error(stmt, SQL_HANDLE_STMT);
+            throw IOException("ODBC StorageService failed to update record.");
+        }
+        m_log.debug("SQLBindParam succeeded (value = %s)", value);
+    }
+
+    int attempts = 3;
+    pair<bool,bool> logres;
+    do {
+        logres = make_pair(false,false);
+        attempts--;
+        sr = SQLExecute(stmt);
+        if (sr == SQL_NO_DATA)
+            return 0;   // went missing?
+        else if (SQL_SUCCEEDED(sr)) {
+            m_log.debug("SQLExecute of update succeeded");
+            return (m_wideVersion ? widever : ver) + 1;
+        }
+
+        m_log.error("update of record failed (t=%s, c=%s, k=%s)", table, context, key);
+        logres = log_error(stmt, SQL_HANDLE_STMT);
+    } while (attempts && logres.first);
+
+    throw IOException("ODBC StorageService failed to update record.");
 }
 
 bool ODBCStorageService::deleteRow(const char *table, const char *context, const char* key)
@@ -595,18 +723,16 @@ bool ODBCStorageService::deleteRow(const char *table, const char *context, const
 
     // Get statement handle.
     ODBCConn conn(getHDBC());
-    ODBCStatement stmt(getHSTMT(conn));
+    SQLHSTMT stmt = getHSTMT(conn);
 
     // Prepare and execute delete statement.
-    char *scontext = makeSafeSQL(context);
-    char *skey = makeSafeSQL(key);
-    string q = string("DELETE FROM ") + table + " WHERE context='" + scontext + "' AND id='" + skey + "'";
-    freeSafeSQL(scontext, context);
-    freeSafeSQL(skey, key);
+    SQLString scontext(context);
+    SQLString skey(key);
+    string q = string("DELETE FROM ") + table + " WHERE context='" + scontext.tostr() + "' AND id='" + skey.tostr() + "'";
     m_log.debug("SQL: %s", q.c_str());
 
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
-     if (sr==SQL_NO_DATA)
+    SQLRETURN sr = SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
+     if (sr == SQL_NO_DATA)
         return false;
     else if (!SQL_SUCCEEDED(sr)) {
         m_log.error("error deleting record (t=%s, c=%s, k=%s)", table, context, key);
@@ -624,20 +750,20 @@ void ODBCStorageService::cleanup()
     xmltooling::NDC ndc("cleanup");
 #endif
 
-    Mutex* mutex = Mutex::create();
+    scoped_ptr<Mutex> mutex(Mutex::create());
 
     mutex->lock();
 
     m_log.info("cleanup thread started... running every %d secs", m_cleanupInterval);
 
     while (!shutdown) {
-        shutdown_wait->timedwait(mutex, m_cleanupInterval);
+        shutdown_wait->timedwait(mutex.get(), m_cleanupInterval);
         if (shutdown)
             break;
         try {
-            reap(NULL);
+            reap(nullptr);
         }
-        catch (exception& ex) {
+        catch (std::exception& ex) {
             m_log.error("cleanup thread swallowed exception: %s", ex.what());
         }
     }
@@ -645,8 +771,7 @@ void ODBCStorageService::cleanup()
     m_log.info("cleanup thread exiting...");
 
     mutex->unlock();
-    delete mutex;
-    Thread::exit(NULL);
+    Thread::exit(nullptr);
 }
 
 void* ODBCStorageService::cleanup_fn(void* cache_p)
@@ -660,7 +785,7 @@ void* ODBCStorageService::cleanup_fn(void* cache_p)
 
   // Now run the cleanup process.
   cache->cleanup();
-  return NULL;
+  return nullptr;
 }
 
 void ODBCStorageService::updateContext(const char *table, const char* context, time_t expiration)
@@ -671,23 +796,21 @@ void ODBCStorageService::updateContext(const char *table, const char* context, t
 
     // Get statement handle.
     ODBCConn conn(getHDBC());
-    ODBCStatement stmt(getHSTMT(conn));
+    SQLHSTMT stmt = getHSTMT(conn);
 
     char timebuf[32];
     timestampFromTime(expiration, timebuf);
 
     char nowbuf[32];
-    timestampFromTime(time(NULL), nowbuf);
+    timestampFromTime(time(nullptr), nowbuf);
 
-    char *scontext = makeSafeSQL(context);
-    string q("UPDATE ");
-    q = q + table + " SET expires = " + timebuf + " WHERE context='" + scontext + "' AND expires > " + nowbuf;
-    freeSafeSQL(scontext, context);
+    SQLString scontext(context);
+    string q = string("UPDATE ") + table + " SET expires = " + timebuf + " WHERE context='" + scontext.tostr() + "' AND expires > " + nowbuf;
 
     m_log.debug("SQL: %s", q.c_str());
 
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
-    if ((sr!=SQL_NO_DATA) && !SQL_SUCCEEDED(sr)) {
+    SQLRETURN sr = SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
+    if ((sr != SQL_NO_DATA) && !SQL_SUCCEEDED(sr)) {
         m_log.error("error updating records (t=%s, c=%s)", table, context ? context : "all");
         log_error(stmt, SQL_HANDLE_STMT);
         throw IOException("ODBC StorageService failed to update context expiration.");
@@ -702,24 +825,23 @@ void ODBCStorageService::reap(const char *table, const char* context)
 
     // Get statement handle.
     ODBCConn conn(getHDBC());
-    ODBCStatement stmt(getHSTMT(conn));
+    SQLHSTMT stmt = getHSTMT(conn);
 
     // Prepare and execute delete statement.
     char nowbuf[32];
-    timestampFromTime(time(NULL), nowbuf);
+    timestampFromTime(time(nullptr), nowbuf);
     string q;
     if (context) {
-        char *scontext = makeSafeSQL(context);
-        q = string("DELETE FROM ") + table + " WHERE context='" + scontext + "' AND expires <= " + nowbuf;
-        freeSafeSQL(scontext, context);
+        SQLString scontext(context);
+        q = string("DELETE FROM ") + table + " WHERE context='" + scontext.tostr() + "' AND expires <= " + nowbuf;
     }
     else {
         q = string("DELETE FROM ") + table + " WHERE expires <= " + nowbuf;
     }
     m_log.debug("SQL: %s", q.c_str());
 
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
-    if ((sr!=SQL_NO_DATA) && !SQL_SUCCEEDED(sr)) {
+    SQLRETURN sr = SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
+    if ((sr != SQL_NO_DATA) && !SQL_SUCCEEDED(sr)) {
         m_log.error("error expiring records (t=%s, c=%s)", table, context ? context : "all");
         log_error(stmt, SQL_HANDLE_STMT);
         throw IOException("ODBC StorageService failed to purge expired records.");
@@ -734,16 +856,15 @@ void ODBCStorageService::deleteContext(const char *table, const char* context)
 
     // Get statement handle.
     ODBCConn conn(getHDBC());
-    ODBCStatement stmt(getHSTMT(conn));
+    SQLHSTMT stmt = getHSTMT(conn);
 
     // Prepare and execute delete statement.
-    char *scontext = makeSafeSQL(context);
-    string q = string("DELETE FROM ") + table + " WHERE context='" + scontext + "'";
-    freeSafeSQL(scontext, context);
+    SQLString scontext(context);
+    string q = string("DELETE FROM ") + table + " WHERE context='" + scontext.tostr() + "'";
     m_log.debug("SQL: %s", q.c_str());
 
-    SQLRETURN sr=SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
-    if ((sr!=SQL_NO_DATA) && !SQL_SUCCEEDED(sr)) {
+    SQLRETURN sr = SQLExecDirect(stmt, (SQLCHAR*)q.c_str(), SQL_NTS);
+    if ((sr != SQL_NO_DATA) && !SQL_SUCCEEDED(sr)) {
         m_log.error("error deleting context (t=%s, c=%s)", table, context);
         log_error(stmt, SQL_HANDLE_STMT);
         throw IOException("ODBC StorageService failed to delete context.");