Skip to content

Commit

Permalink
Future null dereference cleanup and 2.4.1 prep
Browse files Browse the repository at this point in the history
  • Loading branch information
mpenick committed Jun 9, 2016
1 parent 049d93b commit 468db35
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 35 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 1,12 @@
2.4.1
===========
June 9, 2016

Other
--------
* Fixed issue where `cass_future_get_result()` and similiar methods would
dereference a NULL pointer if a future is set to an error.

2.4.0
===========
June 1, 2016
Expand Down
2 changes: 1 addition & 1 deletion include/cassandra.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 52,7 @@

#define CASS_VERSION_MAJOR 2
#define CASS_VERSION_MINOR 4
#define CASS_VERSION_PATCH 0
#define CASS_VERSION_PATCH 1
#define CASS_VERSION_SUFFIX ""

#ifdef __cplusplus
Expand Down
64 changes: 32 additions & 32 deletions src/future.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 56,11 @@ const CassResult* cass_future_get_result(CassFuture* future) {
cass::ResponseFuture* response_future =
static_cast<cass::ResponseFuture*>(future->from());

if (response_future->is_error()) return NULL;

cass::SharedRefPtr<cass::ResultResponse> result(response_future->response());
if (!result) return NULL;

if (result) {
result->decode_first_row();
result->inc_ref();
}

result->decode_first_row();
result->inc_ref();
return CassResult::to(result.get());
}

Expand All @@ -75,16 71,16 @@ const CassPrepared* cass_future_get_prepared(CassFuture* future) {
cass::ResponseFuture* response_future =
static_cast<cass::ResponseFuture*>(future->from());

if (response_future->is_error()) return NULL;

cass::SharedRefPtr<cass::ResultResponse> result(response_future->response());
if (result && result->kind() == CASS_RESULT_KIND_PREPARED) {
cass::Prepared* prepared =
new cass::Prepared(result, response_future->statement, response_future->schema_metadata);
if (prepared) prepared->inc_ref();
return CassPrepared::to(prepared);
if (!result || result->kind() != CASS_RESULT_KIND_PREPARED) {
return NULL;
}
return NULL;

cass::Prepared* prepared = new cass::Prepared(result,
response_future->statement,
response_future->schema_metadata);
if (prepared) prepared->inc_ref();
return CassPrepared::to(prepared);
}

const CassErrorResult* cass_future_get_error_result(CassFuture* future) {
Expand All @@ -94,11 90,14 @@ const CassErrorResult* cass_future_get_error_result(CassFuture* future) {
cass::ResponseFuture* response_future =
static_cast<cass::ResponseFuture*>(future->from());

if (!response_future->is_error()) return NULL;
cass::SharedRefPtr<cass::Response> response(response_future->response());
if (!response || response->opcode() != CQL_OPCODE_ERROR) {
return NULL;
}

cass::SharedRefPtr<cass::ErrorResponse> error_result(response_future->response());
if (error_result) error_result->inc_ref();
return CassErrorResult::to(error_result.get());
response->inc_ref();
return CassErrorResult::to(
static_cast<cass::ErrorResponse*>(response.get()));
}

CassError cass_future_error_code(CassFuture* future) {
Expand Down Expand Up @@ -130,6 129,7 @@ size_t cass_future_custom_payload_item_count(CassFuture* future) {
}
cass::SharedRefPtr<cass::Response> response(
static_cast<cass::ResponseFuture*>(future->from())->response());
if (!response) return 0;
return response->custom_payload().size();
}

Expand All @@ -144,20 144,20 @@ CassError cass_future_custom_payload_item(CassFuture* future,
}
cass::SharedRefPtr<cass::Response> response(
static_cast<cass::ResponseFuture*>(future->from())->response());
if (response) {
const cass::Response::CustomPayloadVec& custom_payload =
response->custom_payload();
if (index >= custom_payload.size()) {
return CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS;
}
const cass::Response::CustomPayloadItem& item = custom_payload[index];
*name = item.name.data();
*name_length = item.name.size();
*value = reinterpret_cast<const cass_byte_t*>(item.value.data());
*value_size = item.value.size();
return CASS_OK;
if (!response) return CASS_ERROR_LIB_NO_CUSTOM_PAYLOAD;

const cass::Response::CustomPayloadVec& custom_payload =
response->custom_payload();
if (index >= custom_payload.size()) {
return CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS;
}
return CASS_ERROR_LIB_NO_CUSTOM_PAYLOAD;

const cass::Response::CustomPayloadItem& item = custom_payload[index];
*name = item.name.data();
*name_length = item.name.size();
*value = reinterpret_cast<const cass_byte_t*>(item.value.data());
*value_size = item.value.size();
return CASS_OK;
}

} // extern "C"
Expand Down
2 changes: 0 additions & 2 deletions src/future.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 82,6 @@ class Future : public RefCounted<Future> {
return internal_wait_for(lock, timeout_us);
}

bool is_error() { return get_error() != NULL; }

Error* get_error() {
ScopedMutex lock(&mutex_);
internal_wait(lock);
Expand Down
128 changes: 128 additions & 0 deletions test/integration_tests/src/test_future.cpp
Original file line number Diff line number Diff line change
@@ -0,0 1,128 @@
/*
Copyright (c) 2014-2016 DataStax
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
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.
*/

#include <boost/test/unit_test.hpp>
#include <boost/test/debug.hpp>
#include <boost/lexical_cast.hpp>
#include <boost/cstdint.hpp>
#include <boost/format.hpp>
#include <boost/thread.hpp>
#include <boost/scoped_ptr.hpp>
#include <boost/thread/mutex.hpp>
#include <boost/thread/condition.hpp>

#include "cassandra.h"
#include "test_utils.hpp"

#include <sstream>

struct FuturesTests : public test_utils::MultipleNodesTest {
FuturesTests() : test_utils::MultipleNodesTest(1, 0) {
}
};

BOOST_FIXTURE_TEST_SUITE(future, FuturesTests)

BOOST_AUTO_TEST_CASE(error)
{
test_utils::CassSessionPtr session(cass_session_new());
test_utils::CassFuturePtr connect_future(cass_session_connect(session.get(), cluster));
test_utils::wait_and_check_error(connect_future.get());

test_utils::CassStatementPtr statement(cass_statement_new("MALFORMED QUERY", 0));
test_utils::CassFuturePtr future(cass_session_execute(session.get(), statement.get()));

BOOST_REQUIRE_NE(cass_future_error_code(future.get()), CASS_OK);

// Should not be set
BOOST_CHECK(cass_future_get_result(future.get()) == NULL);
BOOST_CHECK(cass_future_get_error_result(future.get()) == NULL);
BOOST_CHECK(cass_future_get_prepared(future.get()) == NULL);

BOOST_CHECK_EQUAL(cass_future_custom_payload_item_count(future.get()), 0);
{
const char* name;
const cass_byte_t* value;
size_t name_length, value_size;
BOOST_REQUIRE_EQUAL(cass_future_custom_payload_item(future.get(), 0,
&name, &name_length,
&value, &value_size),
CASS_ERROR_LIB_NO_CUSTOM_PAYLOAD);
}
}

BOOST_AUTO_TEST_CASE(result_response)
{
test_utils::CassSessionPtr session(cass_session_new());
test_utils::CassFuturePtr connect_future(cass_session_connect(session.get(), cluster));
test_utils::wait_and_check_error(connect_future.get());

test_utils::CassStatementPtr statement(cass_statement_new("SELECT * FROM system.local", 0));
test_utils::CassFuturePtr future(cass_session_execute(session.get(), statement.get()));

// Expected
BOOST_REQUIRE_EQUAL(cass_future_error_code(future.get()), CASS_OK);
BOOST_CHECK(cass_future_get_result(future.get()) != NULL);

// Should not be set
BOOST_CHECK(cass_future_get_error_result(future.get()) == NULL);
BOOST_CHECK(cass_future_get_prepared(future.get()) == NULL);

BOOST_CHECK_EQUAL(cass_future_custom_payload_item_count(future.get()), 0);
{
const char* name;
const cass_byte_t* value;
size_t name_length, value_size;
BOOST_REQUIRE_EQUAL(cass_future_custom_payload_item(future.get(), 0,
&name, &name_length,
&value, &value_size),
CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS);
}
}

BOOST_AUTO_TEST_CASE(prepare_response)
{
test_utils::CassSessionPtr session(cass_session_new());
test_utils::CassFuturePtr connect_future(cass_session_connect(session.get(), cluster));
test_utils::wait_and_check_error(connect_future.get());

test_utils::CassFuturePtr future(cass_session_prepare(session.get(), "SELECT * FROM system.local"));

// Expected
BOOST_REQUIRE_EQUAL(cass_future_error_code(future.get()), CASS_OK);
BOOST_CHECK(cass_future_get_prepared(future.get()) != NULL);

// This returns a value but probably shouldn't. We should consider fixing
// this, but it could break existing applications.
BOOST_CHECK(cass_future_get_result(future.get()) != NULL);

// Should not be set
BOOST_CHECK(cass_future_get_error_result(future.get()) == NULL);

BOOST_CHECK_EQUAL(cass_future_custom_payload_item_count(future.get()), 0);
{
const char* name;
const cass_byte_t* value;
size_t name_length, value_size;
BOOST_REQUIRE_EQUAL(cass_future_custom_payload_item(future.get(), 0,
&name, &name_length,
&value, &value_size),
CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS);
}
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 468db35

Please sign in to comment.