From 700abab2b02fd38563202945c58e952bf143ac6f Mon Sep 17 00:00:00 2001
From: Steven Murray <steven.murray@cern.ch>
Date: Wed, 27 Jul 2016 23:46:12 +0200
Subject: [PATCH] Made encryptionKey parameter of Catalogue::createTape()
optional
This commit also enforces the interdiction of empty strings being
bound as string database parameters. Either a non-empty string is
bound or no value at all is bound (nullopt).
---
catalogue/Catalogue.hpp | 6 +++-
catalogue/RdbmsCatalogue.cpp | 9 +++--
catalogue/RdbmsCatalogue.hpp | 6 +++-
common/dataStructures/Tape.hpp | 8 +++++
rdbms/OcciStmt.cpp | 22 +++++++++++-
rdbms/OcciStmt.hpp | 22 ++++++++++--
rdbms/SqliteStmt.cpp | 36 +++++++++++++++----
rdbms/SqliteStmt.hpp | 18 +++++++++-
rdbms/Stmt.hpp | 19 +++++++++-
scheduler/SchedulerTest.cpp | 8 ++---
.../daemon/DataTransferSessionTest.cpp | 4 +--
11 files changed, 136 insertions(+), 22 deletions(-)
diff --git a/catalogue/Catalogue.hpp b/catalogue/Catalogue.hpp
index 4eda855901..7b57fa5bb4 100644
--- a/catalogue/Catalogue.hpp
+++ b/catalogue/Catalogue.hpp
@@ -159,13 +159,17 @@ public:
/**
* Creates a tape which is assumed to have logical block protection (LBP)
* enabled.
+ *
+ * @param encryptionKey The optional identifier of the encrption key. This
+ * optional parameter should either have a non-empty string value or no value
+ * at all. Empty strings are prohibited.
*/
virtual void createTape(
const common::dataStructures::SecurityIdentity &cliIdentity,
const std::string &vid,
const std::string &logicalLibraryName,
const std::string &tapePoolName,
- const std::string &encryptionKey,
+ const optional<std::string> &encryptionKey,
const uint64_t capacityInBytes,
const bool disabled,
const bool full,
diff --git a/catalogue/RdbmsCatalogue.cpp b/catalogue/RdbmsCatalogue.cpp
index 6740233903..1f74601d80 100644
--- a/catalogue/RdbmsCatalogue.cpp
+++ b/catalogue/RdbmsCatalogue.cpp
@@ -1074,12 +1074,17 @@ void RdbmsCatalogue::createTape(
const std::string &vid,
const std::string &logicalLibraryName,
const std::string &tapePoolName,
- const std::string &encryptionKey,
+ const optional<std::string> &encryptionKey,
const uint64_t capacityInBytes,
const bool disabled,
const bool full,
const std::string &comment) {
try {
+ if(encryptionKey && encryptionKey.value().empty()) {
+ throw(exception::Exception(std::string("The identifier of the encrption key for tape ") + vid + " has been set "
+ "to the empty string. This optional value should either have a non-empty string value or no value at all"));
+ }
+
auto conn = m_connPool.getConn();
if(tapeExists(*conn, vid)) {
throw exception::UserError(std::string("Cannot create tape ") + vid +
@@ -1134,7 +1139,7 @@ void RdbmsCatalogue::createTape(
stmt->bindString(":VID", vid);
stmt->bindString(":LOGICAL_LIBRARY_NAME", logicalLibraryName);
stmt->bindString(":TAPE_POOL_NAME", tapePoolName);
- stmt->bindString(":ENCRYPTION_KEY", encryptionKey);
+ stmt->bindOptionalString(":ENCRYPTION_KEY", encryptionKey);
stmt->bindUint64(":CAPACITY_IN_BYTES", capacityInBytes);
stmt->bindUint64(":DATA_IN_BYTES", 0);
stmt->bindUint64(":LAST_FSEQ", 0);
diff --git a/catalogue/RdbmsCatalogue.hpp b/catalogue/RdbmsCatalogue.hpp
index c80a6c9c20..a293f8d862 100644
--- a/catalogue/RdbmsCatalogue.hpp
+++ b/catalogue/RdbmsCatalogue.hpp
@@ -147,13 +147,17 @@ public:
/**
* Creates a tape which is assumed to have logical block protection (LBP)
* enabled.
+ *
+ * @param encryptionKey The optional identifier of the encrption key. This
+ * optional parameter should either have a non-empty string value or no value
+ * at all. Empty strings are prohibited.
*/
virtual void createTape(
const common::dataStructures::SecurityIdentity &cliIdentity,
const std::string &vid,
const std::string &logicalLibraryName,
const std::string &tapePoolName,
- const std::string &encryptionKey,
+ const optional<std::string> &encryptionKey,
const uint64_t capacityInBytes,
const bool disabled,
const bool full,
diff --git a/common/dataStructures/Tape.hpp b/common/dataStructures/Tape.hpp
index 95e9e245bc..d844976320 100644
--- a/common/dataStructures/Tape.hpp
+++ b/common/dataStructures/Tape.hpp
@@ -48,7 +48,15 @@ struct Tape {
std::string tapePoolName;
uint64_t capacityInBytes;
uint64_t dataOnTapeInBytes;
+
+ /**
+ * The optional identifier of the encrption key.
+ *
+ * This optional should either have a non-empty string value or no value at
+ * all. Empty strings are prohibited.
+ */
optional<std::string> encryptionKey;
+
bool lbp;
bool full;
bool disabled;
diff --git a/rdbms/OcciStmt.cpp b/rdbms/OcciStmt.cpp
index 56790d2420..222f7eb4e8 100644
--- a/rdbms/OcciStmt.cpp
+++ b/rdbms/OcciStmt.cpp
@@ -102,8 +102,28 @@ void OcciStmt::bindUint64(const std::string ¶mName, const uint64_t paramValu
//------------------------------------------------------------------------------
void OcciStmt::bindString(const std::string ¶mName, const std::string ¶mValue) {
try {
+ bindOptionalString(paramName, paramValue);
+ } catch(exception::Exception &ex) {
+ throw exception::Exception(std::string(__FUNCTION__) + " failed: " + ex.getMessage().str());
+ }
+}
+
+//------------------------------------------------------------------------------
+// bindOptionalString
+//------------------------------------------------------------------------------
+void OcciStmt::bindOptionalString(const std::string ¶mName, const optional<std::string> ¶mValue) {
+ try {
+ if(paramValue && paramValue.value().empty()) {
+ throw exception::Exception(std::string("Optional string parameter ") + paramName + " is an empty string. "
+ " An optional string parameter should either have a non-empty string value or no value at all.");
+ }
+
const unsigned paramIdx = m_paramNameToIdx.getIdx(paramName);
- m_stmt->setString(paramIdx, paramValue);
+ if(paramValue) {
+ m_stmt->setString(paramIdx, paramValue.value());
+ } else {
+ m_stmt->setString(paramIdx, nullptr);
+ }
} catch(exception::Exception &ex) {
throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + getSql() + ": " +
ex.getMessage().str());
diff --git a/rdbms/OcciStmt.hpp b/rdbms/OcciStmt.hpp
index acecf8d06c..eebbfe9a2b 100644
--- a/rdbms/OcciStmt.hpp
+++ b/rdbms/OcciStmt.hpp
@@ -86,14 +86,30 @@ public:
*/
virtual void bindUint64(const std::string ¶mName, const uint64_t paramValue) override;
- /**
- * Binds an SQL parameter.
+ /**
+ * Binds an SQL parameter of type string.
+ *
+ * Please note that this method will throw an exception if the string
+ * parameter is empty. If a null value is to be bound then the
+ * bindOptionalString() method should be used.
*
* @param paramName The name of the parameter.
* @param paramValue The value to be bound.
- */
+ */
virtual void bindString(const std::string ¶mName, const std::string ¶mValue) override;
+ /**
+ * Binds an SQL parameter of type optional-string.
+ *
+ * Please note that this method will throw an exception if the optional string
+ * parameter has the empty string as its value. An optional string parameter
+ * should either have a non-empty string value or no value at all.
+ *
+ * @param paramName The name of the parameter.
+ * @param paramValue The value to be bound.
+ */
+ virtual void bindOptionalString(const std::string ¶mName, const optional<std::string> ¶mValue) override;
+
/**
* Executes the statement and returns the result set.
*
diff --git a/rdbms/SqliteStmt.cpp b/rdbms/SqliteStmt.cpp
index 50d6357560..a7b5366eb5 100644
--- a/rdbms/SqliteStmt.cpp
+++ b/rdbms/SqliteStmt.cpp
@@ -101,12 +101,36 @@ void SqliteStmt::bindUint64(const std::string ¶mName, const uint64_t paramVa
// bind
//------------------------------------------------------------------------------
void SqliteStmt::bindString(const std::string ¶mName, const std::string ¶mValue) {
- const unsigned int paramIdx = m_paramNameToIdx.getIdx(paramName);
- const int bindRc = paramValue.empty() ?
- sqlite3_bind_text(m_stmt, paramIdx, nullptr, 0, SQLITE_TRANSIENT) :
- sqlite3_bind_text(m_stmt, paramIdx, paramValue.c_str(), -1, SQLITE_TRANSIENT);
- if(SQLITE_OK != bindRc) {
- throw exception::Exception(std::string(__FUNCTION__) + "failed for SQL statement " + getSql());
+ try {
+ bindOptionalString(paramName, paramValue);
+ } catch(exception::Exception &ex) {
+ throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + getSql() + ": " +
+ ex.getMessage().str());
+ }
+}
+
+//------------------------------------------------------------------------------
+// bindOptionalString
+//------------------------------------------------------------------------------
+void SqliteStmt::bindOptionalString(const std::string ¶mName, const optional<std::string> ¶mValue) {
+ try {
+ if(paramValue && paramValue.value().empty()) {
+ throw exception::Exception(std::string("Optional string parameter ") + paramName + " is an empty string. "
+ " An optional string parameter should either have a non-empty string value or no value at all.");
+ }
+ const unsigned int paramIdx = m_paramNameToIdx.getIdx(paramName);
+ int bindRc = 0;
+ if(paramValue) {
+ bindRc = sqlite3_bind_text(m_stmt, paramIdx, paramValue.value().c_str(), -1, SQLITE_TRANSIENT);
+ } else {
+ bindRc = sqlite3_bind_text(m_stmt, paramIdx, nullptr, 0, SQLITE_TRANSIENT);
+ }
+ if(SQLITE_OK != bindRc) {
+ throw exception::Exception(getSql());
+ }
+ } catch(exception::Exception &ex) {
+ throw exception::Exception(std::string(__FUNCTION__) + " failed for SQL statement " + getSql() + ": " +
+ ex.getMessage().str());
}
}
diff --git a/rdbms/SqliteStmt.hpp b/rdbms/SqliteStmt.hpp
index dc5edec3e5..717c5274cc 100644
--- a/rdbms/SqliteStmt.hpp
+++ b/rdbms/SqliteStmt.hpp
@@ -84,13 +84,29 @@ public:
virtual void bindUint64(const std::string ¶mName, const uint64_t paramValue) override;
/**
- * Binds an SQL parameter.
+ * Binds an SQL parameter of type string.
+ *
+ * Please note that this method will throw an exception if the string
+ * parameter is empty. If a null value is to be bound then the
+ * bindOptionalString() method should be used.
*
* @param paramName The name of the parameter.
* @param paramValue The value to be bound.
*/
virtual void bindString(const std::string ¶mName, const std::string ¶mValue) override;
+ /**
+ * Binds an SQL parameter of type optional-string.
+ *
+ * Please note that this method will throw an exception if the optional string
+ * parameter has the empty string as its value. An optional string parameter
+ * should either have a non-empty string value or no value at all.
+ *
+ * @param paramName The name of the parameter.
+ * @param paramValue The value to be bound.
+ */
+ virtual void bindOptionalString(const std::string ¶mName, const optional<std::string> ¶mValue) override;
+
/**
* Executes the statement and returns the result set.
*
diff --git a/rdbms/Stmt.hpp b/rdbms/Stmt.hpp
index efc5c57ad1..b480b340b0 100644
--- a/rdbms/Stmt.hpp
+++ b/rdbms/Stmt.hpp
@@ -18,6 +18,7 @@
#pragma once
+#include "common/optional.hpp"
#include "rdbms/Rset.hpp"
#include <memory>
@@ -59,13 +60,29 @@ public:
virtual void bindUint64(const std::string ¶mName, const uint64_t paramValue) = 0;
/**
- * Binds an SQL parameter.
+ * Binds an SQL parameter of type string.
+ *
+ * Please note that this method will throw an exception if the string
+ * parameter is empty. If a null value is to be bound then the
+ * bindOptionalString() method should be used.
*
* @param paramName The name of the parameter.
* @param paramValue The value to be bound.
*/
virtual void bindString(const std::string ¶mName, const std::string ¶mValue) = 0;
+ /**
+ * Binds an SQL parameter of type optional-string.
+ *
+ * Please note that this method will throw an exception if the optional string
+ * parameter has the empty string as its value. An optional string parameter
+ * should either have a non-empty string value or no value at all.
+ *
+ * @param paramName The name of the parameter.
+ * @param paramValue The value to be bound.
+ */
+ virtual void bindOptionalString(const std::string ¶mName, const optional<std::string> ¶mValue) = 0;
+
/**
* Executes the statement and returns the result set.
*
diff --git a/scheduler/SchedulerTest.cpp b/scheduler/SchedulerTest.cpp
index cfcdd5c2dd..fbb9f56e74 100644
--- a/scheduler/SchedulerTest.cpp
+++ b/scheduler/SchedulerTest.cpp
@@ -391,8 +391,8 @@ TEST_P(SchedulerTest, archive_and_retrieve_new_file) {
const std::string tapeComment = "Tape comment";
bool notDisabled = false;
bool notFull = false;
- catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName,
- s_tapePoolName, "", capacityInBytes, notDisabled, notFull, tapeComment);
+ catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName, s_tapePoolName, nullopt, capacityInBytes,
+ notDisabled, notFull, tapeComment);
{
// Emulate a tape server by asking for a mount and then a file (and succeed
@@ -528,8 +528,8 @@ TEST_P(SchedulerTest, retry_archive_until_max_reached) {
const std::string tapeComment = "Tape comment";
bool notDisabled = false;
bool notFull = false;
- catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName,
- s_tapePoolName, "", capacityInBytes, notDisabled, notFull, tapeComment);
+ catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName, s_tapePoolName, nullopt, capacityInBytes, notDisabled,
+ notFull, tapeComment);
{
// Emulate a tape server by asking for a mount and then a file
diff --git a/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp b/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp
index 174a8d3347..66706c8cf3 100644
--- a/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp
+++ b/tapeserver/castor/tape/tapeserver/daemon/DataTransferSessionTest.cpp
@@ -352,8 +352,8 @@ TEST_P(DataTransferSessionTest, DataTransferSessionGooddayRecall) {
const std::string tapeComment = "Tape comment";
bool notDisabled = false;
bool notFull = false;
- catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName,
- s_tapePoolName, "", capacityInBytes, notDisabled, notFull, tapeComment);
+ catalogue.createTape(s_adminOnAdminHost, s_vid, s_libraryName, s_tapePoolName, cta::nullopt, capacityInBytes,
+ notDisabled, notFull, tapeComment);
// 6) Prepare files for reading by writing them to the mock system
{
--
GitLab