From d13cd30c38a496cf9c4f2e8ddaddb4f8fd66dfa0 Mon Sep 17 00:00:00 2001 From: Eric Ridge Date: Wed, 11 Sep 2024 10:17:16 -0400 Subject: [PATCH] fix segfault converting a null `pg_sys::Datum` to a `String` (#1853) Looks like this was introduced in beb79011a377bb96c34fcc50ae2d652aa1419eb5 and we didn't have test coverage around this specific case. --- pgrx-tests/src/tests/from_into_datum_tests.rs | 13 ++++++++ pgrx/src/datum/from.rs | 32 +++++++++++-------- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/pgrx-tests/src/tests/from_into_datum_tests.rs b/pgrx-tests/src/tests/from_into_datum_tests.rs index 7d72636a2..30e816437 100644 --- a/pgrx-tests/src/tests/from_into_datum_tests.rs +++ b/pgrx-tests/src/tests/from_into_datum_tests.rs @@ -43,4 +43,17 @@ mod tests { assert_eq!(cstr, expected); Ok(()) } + + #[pg_test] + fn null_string_is_none() { + let val: pg_sys::Datum = pg_sys::Datum::null(); + let is_null: bool = true; + + unsafe { + if let Some(_) = String::from_datum(val, is_null) { + // <- this seg fault + panic!("converted a null Datum into a valid string") + } + } + } } diff --git a/pgrx/src/datum/from.rs b/pgrx/src/datum/from.rs index 1318f4342..ff2b52940 100644 --- a/pgrx/src/datum/from.rs +++ b/pgrx/src/datum/from.rs @@ -401,23 +401,27 @@ impl FromDatum for String { #[inline] unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, - _is_null: bool, + is_null: bool, _typoid: pg_sys::Oid, ) -> Option { - let varlena = pg_sys::pg_detoast_datum_packed(datum.cast_mut_ptr()); - let converted_varlena = convert_varlena_to_str_memoized(varlena); - let ret_string = converted_varlena.to_owned(); - - // If the datum is EXTERNAL or COMPRESSED, then detoasting creates a pfree-able chunk - // that needs to be freed. We can free it because `to_owned` above creates a Rust copy - // of the string. - if varlena::varatt_is_1b_e(datum.cast_mut_ptr()) - || varlena::varatt_is_4b_c(datum.cast_mut_ptr()) - { - pg_sys::pfree(varlena.cast()); - } + if is_null || datum.is_null() { + return None; + } else { + let varlena = pg_sys::pg_detoast_datum_packed(datum.cast_mut_ptr()); + let converted_varlena = convert_varlena_to_str_memoized(varlena); + let ret_string = converted_varlena.to_owned(); + + // If the datum is EXTERNAL or COMPRESSED, then detoasting creates a pfree-able chunk + // that needs to be freed. We can free it because `to_owned` above creates a Rust copy + // of the string. + if varlena::varatt_is_1b_e(datum.cast_mut_ptr()) + || varlena::varatt_is_4b_c(datum.cast_mut_ptr()) + { + pg_sys::pfree(varlena.cast()); + } - Some(ret_string) + Some(ret_string) + } } }