Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ongoing fixes #356

Merged
merged 21 commits into from
Aug 3, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions driver/config/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ void readDSNinfo(ConnInfo * ci, bool overwrite) {
GET_CONFIG(server, INI_SERVER, INI_SERVER_DEFAULT);
GET_CONFIG(port, INI_PORT, INI_PORT_DEFAULT);
GET_CONFIG(timeout, INI_TIMEOUT, INI_TIMEOUT_DEFAULT);
GET_CONFIG(verify_connection_early, INI_VERIFYCONNECTIONEARLY, INI_VERIFYCONNECTIONEARLY_DEFAULT);
GET_CONFIG(sslmode, INI_SSLMODE, INI_SSLMODE_DEFAULT);
GET_CONFIG(database, INI_DATABASE, INI_DATABASE_DEFAULT);
GET_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH, INI_STRINGMAXLENGTH_DEFAULT);
Expand Down Expand Up @@ -90,6 +91,7 @@ void writeDSNinfo(const ConnInfo * ci) {
WRITE_CONFIG(server, INI_SERVER);
WRITE_CONFIG(port, INI_PORT);
WRITE_CONFIG(timeout, INI_TIMEOUT);
WRITE_CONFIG(verify_connection_early, INI_VERIFYCONNECTIONEARLY);
WRITE_CONFIG(sslmode, INI_SSLMODE);
WRITE_CONFIG(database, INI_DATABASE);
WRITE_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH);
Expand Down Expand Up @@ -329,6 +331,7 @@ key_value_map_t readDSNInfo(const std::string & dsn_utf8) {
INI_HOST,
INI_PORT,
INI_TIMEOUT,
INI_VERIFYCONNECTIONEARLY,
INI_SSLMODE,
INI_PRIVATEKEYFILE,
INI_CERTIFICATEFILE,
Expand Down
1 change: 1 addition & 0 deletions driver/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ struct ConnInfo {
std::string server;
std::string port;
std::string timeout;
std::string verify_connection_early;
std::string sslmode;
std::string privateKeyFile;
std::string certificateFile;
Expand Down
2 changes: 2 additions & 0 deletions driver/config/ini_defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#define INI_HOST "Host"
#define INI_PORT "Port" /* Port on which the ClickHouse is listening */
#define INI_TIMEOUT "Timeout" /* Connection timeout */
#define INI_VERIFYCONNECTIONEARLY "VerifyConnectionEarly"
#define INI_SSLMODE "SSLMode" /* Use 'require' for https connections */
#define INI_PRIVATEKEYFILE "PrivateKeyFile"
#define INI_CERTIFICATEFILE "CertificateFile"
Expand All @@ -43,6 +44,7 @@
#define INI_SERVER_DEFAULT ""
#define INI_PORT_DEFAULT ""
#define INI_TIMEOUT_DEFAULT "30"
#define INI_VERIFYCONNECTIONEARLY_DEFAULT "no"
#define INI_SSLMODE_DEFAULT ""
#define INI_DATABASE_DEFAULT ""
#define INI_STRINGMAXLENGTH_DEFAULT "1048575"
Expand Down
18 changes: 18 additions & 0 deletions driver/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ void Connection::connect(const std::string & connection_string) {
session->setKeepAlive(true);
session->setTimeout(Poco::Timespan(connection_timeout, 0), Poco::Timespan(timeout, 0), Poco::Timespan(timeout, 0));
session->setKeepAliveTimeout(Poco::Timespan(86400, 0));

if (verify_connection_early) {
verifyConnection();
}
}

void Connection::resetConfiguration() {
Expand Down Expand Up @@ -238,6 +242,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v
timeout = typed_value;
}
}
else if (Poco::UTF8::icompare(key, INI_VERIFYCONNECTIONEARLY) == 0) {
recognized_key = true;
valid_value = (value.empty() || isYesOrNo(value));
if (valid_value) {
verify_connection_early = isYes(value);
}
}
else if (Poco::UTF8::icompare(key, INI_SSLMODE) == 0) {
recognized_key = true;
valid_value = (
Expand Down Expand Up @@ -445,6 +456,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v
stringmaxlength = TypeInfo::string_max_size;
}

void Connection::verifyConnection() {
LOG("Verifying connection and credentials...");
auto & statement = allocateChild<Statement>();
statement.executeQuery("SELECT 1");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can executeQuery() throw an exception? If yes, will the statement be successfully deallocated later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I should have added a proper cleanup there. Will fix.

statement.deallocateSelf();
}

std::string Connection::buildCredentialsString() const {
std::ostringstream user_password_base64;
Poco::Base64Encoder base64_encoder(user_password_base64, Poco::BASE64_URL_ENCODING);
Expand Down
4 changes: 4 additions & 0 deletions driver/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class Connection
std::uint16_t port = 0;
std::uint32_t connection_timeout = 0;
std::uint32_t timeout = 0;
bool verify_connection_early = false;
std::string sslmode;
std::string privateKeyFile;
std::string certificateFile;
Expand Down Expand Up @@ -84,6 +85,9 @@ class Connection
// c) values deduced from values of other fields, if unintialized.
void setConfiguration(const key_value_map_t & cs_fields, const key_value_map_t & dsn_fields);

// Verify the connection and credentials by trying to remotely execute a simple "SELECT 1" query.
void verifyConnection();

private:
std::unordered_map<SQLHANDLE, std::shared_ptr<Descriptor>> descriptors;
std::unordered_map<SQLHANDLE, std::shared_ptr<Statement>> statements;
Expand Down
112 changes: 112 additions & 0 deletions driver/test/misc_it.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,115 @@ TEST_F(MiscellaneousTest, SQLGetData_ZeroOutputBufferSize) {

ASSERT_EQ(SQLFetch(hstmt), SQL_NO_DATA);
}

enum class FailOn {
Connect,
Execute,
Never
};

// First parameter - parameter set name.
// Second parameter - extra name=value semicolon-separated string to append to the connection string.
// Third parameter - when to expect the failure, if any.
class ConnectionFailureReporing
: public ClientTestWithParamBase<std::tuple<std::string, std::string, FailOn>>
{
protected:
virtual void SetUp() override {
using Base = ClientTestWithParamBase<std::tuple<std::string, std::string, FailOn>>;
Base::SetUp();

// As a precondition, check that by default the server is reachable,
// and we are able connect, authenticate, and execute queries successfully.

const std::string query_orig = "SELECT 1";

const auto query = fromUTF8<SQLTCHAR>(query_orig);
auto * query_wptr = const_cast<SQLTCHAR * >(query.c_str());

ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS));
ODBC_CALL_ON_STMT_THROW(hstmt, SQLExecute(hstmt));

// Free the original Connection and Statement instances, and create a new Connection,
// but don't connect it yet - each test will do it on its own.

ODBC_CALL_ON_STMT_THROW(hstmt, SQLFreeHandle(SQL_HANDLE_STMT, hstmt));
hstmt = nullptr;

SQLDisconnect(hdbc);
ODBC_CALL_ON_DBC_THROW(hdbc, SQLFreeHandle(SQL_HANDLE_DBC, hdbc));

ODBC_CALL_ON_ENV_THROW(henv, SQLAllocHandle(SQL_HANDLE_DBC, henv, &hdbc));
}
};

TEST_P(ConnectionFailureReporing, TryQuery) {
const auto & [/* unused */name, cs_extras, fail_on] = GetParam();

{
const auto & dsn_orig = TestEnvironment::getInstance().getDSN();
std::string cs_orig = "DSN={" + dsn_orig + "};" + cs_extras;
const auto cs = fromUTF8<SQLTCHAR>(cs_orig);
auto * cs_wptr = const_cast<SQLTCHAR *>(cs.c_str());

const auto rc = SQLDriverConnect(hdbc, NULL, cs_wptr, SQL_NTS, NULL, 0, NULL, SQL_DRIVER_NOPROMPT);

if (fail_on == FailOn::Connect) {
ASSERT_EQ(rc, SQL_ERROR) << "Expected to fail on Connect!";
return;
}
else {
ODBC_CALL_ON_DBC_THROW(hdbc, rc);
}
}

ODBC_CALL_ON_DBC_THROW(hdbc, SQLAllocHandle(SQL_HANDLE_STMT, hdbc, &hstmt));

{
const std::string query_orig = "SELECT 1";

const auto query = fromUTF8<SQLTCHAR>(query_orig);
auto * query_wptr = const_cast<SQLTCHAR * >(query.c_str());

ODBC_CALL_ON_STMT_THROW(hstmt, SQLPrepare(hstmt, query_wptr, SQL_NTS));

const auto rc = SQLExecute(hstmt);

if (fail_on == FailOn::Execute) {
ASSERT_EQ(rc, SQL_ERROR) << "Expected to fail on Execute!";
return;
}
else {
ODBC_CALL_ON_STMT_THROW(hstmt, rc);
}
}
}

INSTANTIATE_TEST_SUITE_P(
MiscellaneousTest,
ConnectionFailureReporing,
::testing::Values(
std::make_tuple("BadHost_FailOnConnect", "Host=some_bad_hostname;VerifyConnectionEarly=true", FailOn::Connect),
std::make_tuple("BadUsername_FailOnConnect", "UID=some_bad_username;VerifyConnectionEarly=true", FailOn::Connect),
std::make_tuple("BadPassword_FailOnConnect", "PWD=some_bad_password;VerifyConnectionEarly=true", FailOn::Connect),

std::make_tuple("BadHost_FailOnExecute", "Host=some_bad_hostname;VerifyConnectionEarly=false", FailOn::Execute),
std::make_tuple("BadUsername_FailOnExecute", "UID=some_bad_username;VerifyConnectionEarly=false", FailOn::Execute),
std::make_tuple("BadPassword_FailOnExecute", "PWD=some_bad_password;VerifyConnectionEarly=false", FailOn::Execute),

std::make_tuple("BadHost_FailOnExecuteByDefault", "Host=some_bad_hostname", FailOn::Execute),
std::make_tuple("BadUsername_FailOnExecuteByDefault", "UID=some_bad_username", FailOn::Execute),
std::make_tuple("BadPassword_FailOnExecuteByDefault", "PWD=some_bad_password", FailOn::Execute),

std::make_tuple("BadHost_FailOnExecuteByDefault2", "Host=some_bad_hostname;VerifyConnectionEarly=", FailOn::Execute),
std::make_tuple("BadUsername_FailOnExecuteByDefault2", "UID=some_bad_username;VerifyConnectionEarly=", FailOn::Execute),
std::make_tuple("BadPassword_FailOnExecuteByDefault2", "PWD=some_bad_password;VerifyConnectionEarly=", FailOn::Execute),

std::make_tuple("AllGood_VerifyConnectionEarly_Empty", "VerifyConnectionEarly=", FailOn::Never),
std::make_tuple("AllGood_VerifyConnectionEarly_True", "VerifyConnectionEarly=true", FailOn::Never),
std::make_tuple("AllGood_VerifyConnectionEarly_False", "VerifyConnectionEarly=false", FailOn::Never)
),
[] (const auto & param_info) {
return std::get<0>(param_info.param);
}
);