Skip to content

Commit b1ff429

Browse files
muzarskimuzarski
authored
Merge pull request #300 from muzarski/verify-null-ptr
Don't panic when null pointer is provided - log error and return instead
2 parents 2aad281 + 8c64a66 commit b1ff429

18 files changed

+1162
-287
lines changed

‎scylla-rust-wrapper/src/batch.rs

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@ pub unsafe extern "C" fn cass_batch_set_consistency(
6060
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
6161
consistency: CassConsistency,
6262
) -> CassError {
63-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
63+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
64+
tracing::error!("Provided null batch pointer to cass_batch_set_consistency!");
65+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
66+
};
67+
6468
let consistency = match consistency.try_into().ok() {
6569
Some(c) => c,
6670
None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
@@ -77,7 +81,11 @@ pub unsafe extern "C" fn cass_batch_set_serial_consistency(
7781
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
7882
serial_consistency: CassConsistency,
7983
) -> CassError {
80-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
84+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
85+
tracing::error!("Provided null batch pointer to cass_batch_set_serial_consistency!");
86+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
87+
};
88+
8189
let serial_consistency = match serial_consistency.try_into().ok() {
8290
Some(c) => c,
8391
None => return CassError::CASS_ERROR_LIB_BAD_PARAMS,
@@ -94,7 +102,10 @@ pub unsafe extern "C" fn cass_batch_set_retry_policy(
94102
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
95103
retry_policy: CassBorrowedSharedPtr<CassRetryPolicy, CMut>,
96104
) -> CassError {
97-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
105+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
106+
tracing::error!("Provided null batch pointer to cass_batch_set_retry_policy!");
107+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
108+
};
98109

99110
let maybe_arced_retry_policy: Option<Arc<dyn scylla::policies::retry::RetryPolicy>> =
100111
ArcFFI::as_ref(retry_policy).map(|policy| match policy {
@@ -117,7 +128,10 @@ pub unsafe extern "C" fn cass_batch_set_timestamp(
117128
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
118129
timestamp: cass_int64_t,
119130
) -> CassError {
120-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
131+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
132+
tracing::error!("Provided null batch pointer to cass_batch_set_timestamp!");
133+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
134+
};
121135

122136
Arc::make_mut(&mut batch.state)
123137
.batch
@@ -131,7 +145,10 @@ pub unsafe extern "C" fn cass_batch_set_request_timeout(
131145
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
132146
timeout_ms: cass_uint64_t,
133147
) -> CassError {
134-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
148+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
149+
tracing::error!("Provided null batch pointer to cass_batch_set_request_timeout!");
150+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
151+
};
135152
batch.batch_request_timeout_ms = Some(timeout_ms);
136153

137154
CassError::CASS_OK
@@ -142,7 +159,11 @@ pub unsafe extern "C" fn cass_batch_set_is_idempotent(
142159
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
143160
is_idempotent: cass_bool_t,
144161
) -> CassError {
145-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
162+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
163+
tracing::error!("Provided null batch pointer to cass_batch_set_is_idempotent!");
164+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
165+
};
166+
146167
Arc::make_mut(&mut batch.state)
147168
.batch
148169
.set_is_idempotent(is_idempotent != 0);
@@ -155,7 +176,11 @@ pub unsafe extern "C" fn cass_batch_set_tracing(
155176
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
156177
enabled: cass_bool_t,
157178
) -> CassError {
158-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
179+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
180+
tracing::error!("Provided null batch pointer to cass_batch_set_tracing!");
181+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
182+
};
183+
159184
Arc::make_mut(&mut batch.state)
160185
.batch
161186
.set_tracing(enabled != 0);
@@ -168,9 +193,16 @@ pub unsafe extern "C" fn cass_batch_add_statement(
168193
batch: CassBorrowedExclusivePtr<CassBatch, CMut>,
169194
statement: CassBorrowedSharedPtr<CassStatement, CMut>,
170195
) -> CassError {
171-
let batch = BoxFFI::as_mut_ref(batch).unwrap();
196+
let Some(batch) = BoxFFI::as_mut_ref(batch) else {
197+
tracing::error!("Provided null batch pointer to cass_batch_add_statement!");
198+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
199+
};
200+
let Some(statement) = BoxFFI::as_ref(statement) else {
201+
tracing::error!("Provided null statement pointer to cass_batch_add_statement!");
202+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
203+
};
204+
172205
let state = Arc::make_mut(&mut batch.state);
173-
let statement = BoxFFI::as_ref(statement).unwrap();
174206

175207
match &statement.statement {
176208
BoundStatement::Simple(q) => {

‎scylla-rust-wrapper/src/binding.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,12 @@ macro_rules! make_index_binder {
6060
// For some reason detected as unused, which is not true
6161
#[allow(unused_imports)]
6262
use crate::value::CassCqlValue::*;
63+
let Some(this) = BoxFFI::as_mut_ref(this) else {
64+
tracing::error!("Provided null pointer to {}!", stringify!($fn_by_idx));
65+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
66+
};
6367
match ($e)($($arg), *) {
64-
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this).unwrap(), index as usize, v),
68+
Ok(v) => $consume_v(this, index as usize, v),
6569
Err(e) => e,
6670
}
6771
}
@@ -80,9 +84,13 @@ macro_rules! make_name_binder {
8084
// For some reason detected as unused, which is not true
8185
#[allow(unused_imports)]
8286
use crate::value::CassCqlValue::*;
87+
let Some(this) = BoxFFI::as_mut_ref(this) else {
88+
tracing::error!("Provided null pointer to {}!", stringify!($fn_by_name));
89+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
90+
};
8391
let name = unsafe { ptr_to_cstr(name) }.unwrap();
8492
match ($e)($($arg), *) {
85-
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this).unwrap(), name, v),
93+
Ok(v) => $consume_v(this, name, v),
8694
Err(e) => e,
8795
}
8896
}
@@ -102,9 +110,13 @@ macro_rules! make_name_n_binder {
102110
// For some reason detected as unused, which is not true
103111
#[allow(unused_imports)]
104112
use crate::value::CassCqlValue::*;
113+
let Some(this) = BoxFFI::as_mut_ref(this) else {
114+
tracing::error!("Provided null pointer to {}!", stringify!($fn_by_name_n));
115+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
116+
};
105117
let name = unsafe { ptr_to_cstr_n(name, name_length) }.unwrap();
106118
match ($e)($($arg), *) {
107-
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this).unwrap(), name, v),
119+
Ok(v) => $consume_v(this, name, v),
108120
Err(e) => e,
109121
}
110122
}
@@ -122,8 +134,12 @@ macro_rules! make_appender {
122134
// For some reason detected as unused, which is not true
123135
#[allow(unused_imports)]
124136
use crate::value::CassCqlValue::*;
137+
let Some(this) = BoxFFI::as_mut_ref(this) else {
138+
tracing::error!("Provided null pointer to {}!", stringify!($fn_append));
139+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
140+
};
125141
match ($e)($($arg), *) {
126-
Ok(v) => $consume_v(BoxFFI::as_mut_ref(this).unwrap(), v),
142+
Ok(v) => $consume_v(this, v),
127143
Err(e) => e,
128144
}
129145
}

‎scylla-rust-wrapper/src/cass_types.rs

Lines changed: 90 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,11 @@ pub unsafe extern "C" fn cass_data_type_new(
474474
pub unsafe extern "C" fn cass_data_type_new_from_existing(
475475
data_type: CassBorrowedSharedPtr<CassDataType, CConst>,
476476
) -> CassOwnedSharedPtr<CassDataType, CMut> {
477-
let data_type = ArcFFI::as_ref(data_type).unwrap();
477+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
478+
tracing::error!("Provided null data type pointer to cass_data_type_new_from_existing!");
479+
return ArcFFI::null();
480+
};
481+
478482
ArcFFI::into_ptr(CassDataType::new_arced(
479483
unsafe { data_type.get_unchecked() }.clone(),
480484
))
@@ -507,15 +511,23 @@ pub unsafe extern "C" fn cass_data_type_free(data_type: CassOwnedSharedPtr<CassD
507511
pub unsafe extern "C" fn cass_data_type_type(
508512
data_type: CassBorrowedSharedPtr<CassDataType, CConst>,
509513
) -> CassValueType {
510-
let data_type = ArcFFI::as_ref(data_type).unwrap();
514+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
515+
tracing::error!("Provided null data type pointer to cass_data_type_type!");
516+
return CassValueType::CASS_VALUE_TYPE_UNKNOWN;
517+
};
518+
511519
unsafe { data_type.get_unchecked() }.get_value_type()
512520
}
513521

514522
#[unsafe(no_mangle)]
515523
pub unsafe extern "C" fn cass_data_type_is_frozen(
516524
data_type: CassBorrowedSharedPtr<CassDataType, CConst>,
517525
) -> cass_bool_t {
518-
let data_type = ArcFFI::as_ref(data_type).unwrap();
526+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
527+
tracing::error!("Provided null data type pointer to cass_data_type_is_frozen!");
528+
return cass_false;
529+
};
530+
519531
let is_frozen = match unsafe { data_type.get_unchecked() } {
520532
CassDataTypeInner::UDT(udt) => udt.frozen,
521533
CassDataTypeInner::List { frozen, .. } => *frozen,
@@ -533,7 +545,11 @@ pub unsafe extern "C" fn cass_data_type_type_name(
533545
type_name: *mut *const c_char,
534546
type_name_length: *mut size_t,
535547
) -> CassError {
536-
let data_type = ArcFFI::as_ref(data_type).unwrap();
548+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
549+
tracing::error!("Provided null data type pointer to cass_data_type_type_name!");
550+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
551+
};
552+
537553
match unsafe { data_type.get_unchecked() } {
538554
CassDataTypeInner::UDT(UDTDataType { name, .. }) => {
539555
unsafe { write_str_to_c(name, type_name, type_name_length) };
@@ -557,7 +573,11 @@ pub unsafe extern "C" fn cass_data_type_set_type_name_n(
557573
type_name: *const c_char,
558574
type_name_length: size_t,
559575
) -> CassError {
560-
let data_type = ArcFFI::as_ref(data_type_raw).unwrap();
576+
let Some(data_type) = ArcFFI::as_ref(data_type_raw) else {
577+
tracing::error!("Provided null data type pointer to cass_data_type_set_type_name_n!");
578+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
579+
};
580+
561581
let type_name_string = unsafe { ptr_to_cstr_n(type_name, type_name_length) }
562582
.unwrap()
563583
.to_string();
@@ -577,7 +597,11 @@ pub unsafe extern "C" fn cass_data_type_keyspace(
577597
keyspace: *mut *const c_char,
578598
keyspace_length: *mut size_t,
579599
) -> CassError {
580-
let data_type = ArcFFI::as_ref(data_type).unwrap();
600+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
601+
tracing::error!("Provided null data type pointer to cass_data_type_keyspace!");
602+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
603+
};
604+
581605
match unsafe { data_type.get_unchecked() } {
582606
CassDataTypeInner::UDT(UDTDataType { name, .. }) => {
583607
unsafe { write_str_to_c(name, keyspace, keyspace_length) };
@@ -601,7 +625,11 @@ pub unsafe extern "C" fn cass_data_type_set_keyspace_n(
601625
keyspace: *const c_char,
602626
keyspace_length: size_t,
603627
) -> CassError {
604-
let data_type = ArcFFI::as_ref(data_type).unwrap();
628+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
629+
tracing::error!("Provided null data type pointer to cass_data_type_set_keyspace_n!");
630+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
631+
};
632+
605633
let keyspace_string = unsafe { ptr_to_cstr_n(keyspace, keyspace_length) }
606634
.unwrap()
607635
.to_string();
@@ -621,7 +649,11 @@ pub unsafe extern "C" fn cass_data_type_class_name(
621649
class_name: *mut *const ::std::os::raw::c_char,
622650
class_name_length: *mut size_t,
623651
) -> CassError {
624-
let data_type = ArcFFI::as_ref(data_type).unwrap();
652+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
653+
tracing::error!("Provided null data type pointer to cass_data_type_class_name!");
654+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
655+
};
656+
625657
match unsafe { data_type.get_unchecked() } {
626658
CassDataTypeInner::Custom(name) => {
627659
unsafe { write_str_to_c(name, class_name, class_name_length) };
@@ -645,7 +677,11 @@ pub unsafe extern "C" fn cass_data_type_set_class_name_n(
645677
class_name: *const ::std::os::raw::c_char,
646678
class_name_length: size_t,
647679
) -> CassError {
648-
let data_type = ArcFFI::as_ref(data_type).unwrap();
680+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
681+
tracing::error!("Provided null data type pointer to cass_data_type_set_class_name_n!");
682+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
683+
};
684+
649685
let class_string = unsafe { ptr_to_cstr_n(class_name, class_name_length) }
650686
.unwrap()
651687
.to_string();
@@ -662,7 +698,11 @@ pub unsafe extern "C" fn cass_data_type_set_class_name_n(
662698
pub unsafe extern "C" fn cass_data_type_sub_type_count(
663699
data_type: CassBorrowedSharedPtr<CassDataType, CConst>,
664700
) -> size_t {
665-
let data_type = ArcFFI::as_ref(data_type).unwrap();
701+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
702+
tracing::error!("Provided null data type pointer to cass_data_type_sub_type_count!");
703+
return 0;
704+
};
705+
666706
match unsafe { data_type.get_unchecked() } {
667707
CassDataTypeInner::Value(..) => 0,
668708
CassDataTypeInner::UDT(udt_data_type) => udt_data_type.field_types.len() as size_t,
@@ -691,7 +731,11 @@ pub unsafe extern "C" fn cass_data_type_sub_data_type(
691731
data_type: CassBorrowedSharedPtr<CassDataType, CConst>,
692732
index: size_t,
693733
) -> CassBorrowedSharedPtr<CassDataType, CConst> {
694-
let data_type = ArcFFI::as_ref(data_type).unwrap();
734+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
735+
tracing::error!("Provided null data type pointer to cass_data_type_sub_data_type!");
736+
return ArcFFI::null();
737+
};
738+
695739
let sub_type: Option<&Arc<CassDataType>> =
696740
unsafe { data_type.get_unchecked() }.get_sub_data_type(index as usize);
697741

@@ -716,7 +760,13 @@ pub unsafe extern "C" fn cass_data_type_sub_data_type_by_name_n(
716760
name: *const ::std::os::raw::c_char,
717761
name_length: size_t,
718762
) -> CassBorrowedSharedPtr<CassDataType, CConst> {
719-
let data_type = ArcFFI::as_ref(data_type).unwrap();
763+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
764+
tracing::error!(
765+
"Provided null data type pointer to cass_data_type_sub_data_type_by_name_n!"
766+
);
767+
return ArcFFI::null();
768+
};
769+
720770
let name_str = unsafe { ptr_to_cstr_n(name, name_length) }.unwrap();
721771
match unsafe { data_type.get_unchecked() } {
722772
CassDataTypeInner::UDT(udt) => match udt.get_field_by_name(name_str) {
@@ -734,7 +784,11 @@ pub unsafe extern "C" fn cass_data_type_sub_type_name(
734784
name: *mut *const ::std::os::raw::c_char,
735785
name_length: *mut size_t,
736786
) -> CassError {
737-
let data_type = ArcFFI::as_ref(data_type).unwrap();
787+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
788+
tracing::error!("Provided null data type pointer to cass_data_type_sub_type_name!");
789+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
790+
};
791+
738792
match unsafe { data_type.get_unchecked() } {
739793
CassDataTypeInner::UDT(udt) => match udt.field_types.get(index as usize) {
740794
None => CassError::CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS,
@@ -752,10 +806,16 @@ pub unsafe extern "C" fn cass_data_type_add_sub_type(
752806
data_type: CassBorrowedSharedPtr<CassDataType, CMut>,
753807
sub_data_type: CassBorrowedSharedPtr<CassDataType, CConst>,
754808
) -> CassError {
755-
let data_type = ArcFFI::as_ref(data_type).unwrap();
756-
match unsafe { data_type.get_mut_unchecked() }
757-
.add_sub_data_type(ArcFFI::cloned_from_ptr(sub_data_type).unwrap())
758-
{
809+
let Some(data_type) = ArcFFI::as_ref(data_type) else {
810+
tracing::error!("Provided null data type pointer to cass_data_type_add_sub_type!");
811+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
812+
};
813+
let Some(sub_data_type) = ArcFFI::cloned_from_ptr(sub_data_type) else {
814+
tracing::error!("Provided null sub data type pointer to cass_data_type_add_sub_type!");
815+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
816+
};
817+
818+
match unsafe { data_type.get_mut_unchecked() }.add_sub_data_type(sub_data_type) {
759819
Ok(()) => CassError::CASS_OK,
760820
Err(e) => e,
761821
}
@@ -777,12 +837,23 @@ pub unsafe extern "C" fn cass_data_type_add_sub_type_by_name_n(
777837
name_length: size_t,
778838
sub_data_type_raw: CassBorrowedSharedPtr<CassDataType, CConst>,
779839
) -> CassError {
840+
let Some(data_type) = ArcFFI::as_ref(data_type_raw) else {
841+
tracing::error!(
842+
"Provided null data type pointer to cass_data_type_add_sub_type_by_name_n!"
843+
);
844+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
845+
};
846+
let Some(sub_data_type) = ArcFFI::cloned_from_ptr(sub_data_type_raw) else {
847+
tracing::error!(
848+
"Provided null sub data type pointer to cass_data_type_add_sub_type_by_name_n!"
849+
);
850+
return CassError::CASS_ERROR_LIB_BAD_PARAMS;
851+
};
852+
780853
let name_string = unsafe { ptr_to_cstr_n(name, name_length) }
781854
.unwrap()
782855
.to_string();
783-
let sub_data_type = ArcFFI::cloned_from_ptr(sub_data_type_raw).unwrap();
784856

785-
let data_type = ArcFFI::as_ref(data_type_raw).unwrap();
786857
match unsafe { data_type.get_mut_unchecked() } {
787858
CassDataTypeInner::UDT(udt_data_type) => {
788859
// The Cpp Driver does not check whether field_types size

0 commit comments

Comments
 (0)