fix: stop try_ functions from panicking

arrow-arith/src/arity.rs

@@ -247,10 +247,10 @@ where
 ///
 /// Like [`try_unary`] the function is only evaluated for non-null indices
 ///
-/// # Error
+/// # Errors
 ///
-/// Return an error if the arrays have different lengths or
-/// the operation is under erroneous
+/// Returns an error if the arrays have different lengths,
+/// or if the operation returns one.
 pub fn try_binary<A: ArrayAccessor, B: ArrayAccessor, F, O>(
     a: A,
     b: B,

arrow-buffer/src/buffer/mutable.rs

@@ -1105,8 +1105,15 @@ impl MutableBuffer {
     /// if any of the items of the iterator is an error.
     /// Prefer this to `collect` whenever possible, as it is faster ~60% faster.
     ///
+    /// # Errors
+    ///
+    /// Returns the first error yielded by the iterator.
+    ///
     /// # Panics
     ///
+    /// Note that unlike the [`Err`] cases, these panics are violations of the safety contract
+    /// below, and are only checks that happen to be cheap enough to keep:
+    ///
     /// Panics if the iterator does not report an upper bound via `size_hint`, or if the
     /// reported length does not match the number of items produced before an error-free finish,
     /// or if allocating the required buffer fails for the same reasons as

arrow-data/src/transform/mod.rs

@@ -136,9 +136,8 @@ fn build_extend_null_bits(array: &ArrayData, use_nulls: bool) -> ExtendNullBits<
 pub struct MutableArrayData<'a> {
     /// Input arrays: the data being read FROM.
     ///
-    /// Note this is "dead code" because all actual references to the arrays are
-    /// stored in closures for extending values and nulls.
-    #[expect(dead_code)]
+    /// Note all actual reads of the arrays go through the closures for extending
+    /// values and nulls; these references are only kept for bounds checking.
     arrays: Vec<&'a ArrayData>,
 
     /// In progress output array: The data being written TO
@@ -736,13 +735,23 @@ impl<'a> MutableArrayData<'a> {
     /// * `end` - the end index of the chunk (exclusive)
     ///
     /// # Errors
-    /// Returns an error if offset arithmetic overflows the underlying integer type.
-    ///
-    /// # Panics
-    /// This function panics if there is an invalid index,
-    /// i.e. `index` >= the number of source arrays
-    /// or `end` > the length of the `index`th array
+    /// Returns an error if
+    /// * `index` >= the number of source arrays,
+    /// * `start..end` is not a valid range within the `index`th array, or
+    /// * offset arithmetic overflows the underlying integer type.
     pub fn try_extend(&mut self, index: usize, start: usize, end: usize) -> Result<(), ArrowError> {
+        let Some(array_len) = self.arrays.get(index).map(|array| array.len()) else {
+            return Err(ArrowError::InvalidArgumentError(format!(
+                "Source array index {index} is out of bounds: there are {} source arrays",
+                self.arrays.len()
+            )));
+        };
+        if end < start || array_len < end {
+            return Err(ArrowError::InvalidArgumentError(format!(
+                "Invalid range {start}..{end} for source array {index} of length {array_len}"
+            )));
+        }
+
         let len = end - start;
         (self.extend_null_bits[index])(&mut self.data, start, len);
         // Snapshot buffer lengths before attempting the extend so we can roll
@@ -763,17 +772,16 @@ impl<'a> MutableArrayData<'a> {
     /// Extends the in progress array with a region of the input arrays.
     ///
     /// # Panics
-    /// This function panics if there is an invalid index,
-    /// i.e. `index` >= the number of source arrays,
-    /// `end` > the length of the `index`th array,
-    /// or the offset type overflows (e.g. more than 2 GiB in a `StringArray`).
+    /// This function panics if
+    /// * `index` >= the number of source arrays,
+    /// * `start..end` is not a valid range within the `index`th array, or
+    /// * the offset type overflows (e.g. more than 2 GiB in a `StringArray`).
     #[deprecated(
         since = "59.0.0",
         note = "Use `try_extend` which returns an error on overflow instead of panicking"
     )]
     pub fn extend(&mut self, index: usize, start: usize, end: usize) {
-        self.try_extend(index, start, end)
-            .expect("extend failed due to offset overflow")
+        self.try_extend(index, start, end).expect("extend failed")
     }
 
     /// Extends the in progress array with null elements, ignoring the input arrays, returning an
@@ -782,10 +790,19 @@ impl<'a> MutableArrayData<'a> {
     /// Prefer this over [`extend_nulls`](Self::extend_nulls) to handle cases where the run-end
     /// counter overflows (relevant for `RunEndEncoded` arrays).
     ///
-    /// # Panics
+    /// # Errors
     ///
-    /// Panics if [`MutableArrayData`] not created with `use_nulls` or nullable source arrays
+    /// Returns an error if this [`MutableArrayData`] was not created with `use_nulls` and none
+    /// of the source arrays are nullable, or if the run-end counter overflows.
     pub fn try_extend_nulls(&mut self, len: usize) -> Result<(), ArrowError> {
+        if self.data.null_buffer.is_none() {
+            return Err(ArrowError::InvalidArgumentError(
+                "MutableArrayData cannot be extended with nulls: it was created with `use_nulls` \
+                 set to false and no source array is nullable"
+                    .to_owned(),
+            ));
+        }
+
         self.data.len += len;
         let bit_len = bit_util::ceil(self.data.len, 8);
         let nulls = self.data.null_buffer();
@@ -801,15 +818,14 @@ impl<'a> MutableArrayData<'a> {
     ///
     /// # Panics
     ///
-    /// Panics if [`MutableArrayData`] not created with `use_nulls` or nullable source arrays,
-    /// or if the run-end counter overflows for `RunEndEncoded` arrays.
+    /// Panics if this [`MutableArrayData`] was not created with `use_nulls` and none of the
+    /// source arrays are nullable, or if the run-end counter overflows.
     #[deprecated(
         since = "59.0.0",
         note = "Use `try_extend_nulls` which returns an error on overflow instead of panicking"
     )]
     pub fn extend_nulls(&mut self, len: usize) {
-        self.try_extend_nulls(len)
-            .expect("extend_nulls failed due to overflow")
+        self.try_extend_nulls(len).expect("extend_nulls failed")
     }
 
     /// Returns the current length
@@ -905,6 +921,64 @@ mod test {
     use arrow_schema::Field;
     use std::sync::Arc;
 
+    fn int64_array_data(values: Vec<i64>) -> ArrayData {
+        let len = values.len();
+        ArrayData::try_new(
+            DataType::Int64,
+            len,
+            None,
+            0,
+            vec![arrow_buffer::Buffer::from_slice_ref(&values)],
+            vec![],
+        )
+        .unwrap()
+    }
+
+    #[test]
+    fn test_try_extend_invalid_index_and_range() {
+        let array = int64_array_data(vec![1, 2, 3]);
+        let mut mutable = MutableArrayData::new(vec![&array], false, 3);
+
+        let err = mutable.try_extend(1, 0, 1).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "Invalid argument error: Source array index 1 is out of bounds: there are 1 source arrays"
+        );
+
+        let err = mutable.try_extend(0, 0, 4).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "Invalid argument error: Invalid range 0..4 for source array 0 of length 3"
+        );
+
+        // `end < start` used to underflow:
+        let err = mutable.try_extend(0, 2, 1).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "Invalid argument error: Invalid range 2..1 for source array 0 of length 3"
+        );
+
+        // The bounds are inclusive of the full array:
+        mutable.try_extend(0, 3, 3).unwrap();
+        mutable.try_extend(0, 0, 3).unwrap();
+        assert_eq!(mutable.len(), 3);
+    }
+
+    #[test]
+    fn test_try_extend_nulls_without_null_buffer() {
+        let array = int64_array_data(vec![1, 2, 3]);
+        let mut mutable = MutableArrayData::new(vec![&array], false, 3);
+        let err = mutable.try_extend_nulls(1).unwrap_err();
+        assert!(
+            err.to_string().contains("cannot be extended with nulls"),
+            "unexpected error: {err}"
+        );
+
+        let mut mutable = MutableArrayData::new(vec![&array], true, 3);
+        mutable.try_extend_nulls(1).unwrap();
+        assert_eq!(mutable.len(), 1);
+    }
+
     #[test]
     fn test_list_append_with_capacities() {
         let array = ArrayData::new_empty(&DataType::List(Arc::new(Field::new(

arrow-schema/src/ffi.rs

@@ -132,17 +132,26 @@ unsafe extern "C" fn release_schema(schema: *mut FFI_ArrowSchema) {
 }
 
 impl FFI_ArrowSchema {
-    /// create a new [`FFI_ArrowSchema`]. This fails if the fields'
-    /// [`DataType`] is not supported.
+    /// create a new [`FFI_ArrowSchema`].
     ///
-    /// # Panics
+    /// # Errors
     ///
-    /// Panics if `format` contains an interior nul byte
+    /// Errors if the fields' [`DataType`] is not supported,
+    /// or if `format` contains an interior nul byte.
     pub fn try_new(
         format: &str,
         children: Vec<FFI_ArrowSchema>,
         dictionary: Option<FFI_ArrowSchema>,
     ) -> Result<Self, ArrowError> {
+        // Convert the format before leaking any of the children,
+        // so that an error here does not leak memory.
+        let format = CString::new(format).map_err(|err| {
+            ArrowError::CDataInterface(format!(
+                "Null byte at position {} not allowed in format",
+                err.nul_position()
+            ))
+        })?;
+
         let mut this = Self::empty();
 
         let children_ptr = children
@@ -151,7 +160,7 @@ impl FFI_ArrowSchema {
             .map(Box::into_raw)
             .collect::<Box<_>>();
 
-        this.format = CString::new(format).unwrap().into_raw();
+        this.format = format.into_raw();
         this.release = Some(release_schema);
         this.n_children = children_ptr.len() as i64;
 
@@ -932,6 +941,15 @@ mod tests {
         assert_eq!(restored, schema);
     }
 
+    #[test]
+    fn test_try_new_with_interior_nul_byte() {
+        let err = FFI_ArrowSchema::try_new("i\0nt", vec![], None).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "C Data interface error: Null byte at position 1 not allowed in format"
+        );
+    }
+
     #[test]
     fn test_type() {
         round_trip_type(DataType::Int64);

arrow/tests/array_transform.rs

@@ -1029,11 +1029,14 @@ fn test_extend_nulls() {
 }
 
 #[test]
-#[should_panic(expected = "MutableArrayData not nullable")]
-fn test_extend_nulls_panic() {
+fn test_extend_nulls_not_nullable() {
     let int = Int32Array::from(vec![1, 2, 3, 4]).into_data();
     let mut mutable = MutableArrayData::new(vec![&int], false, 4);
-    mutable.try_extend_nulls(2).unwrap();
+    let err = mutable.try_extend_nulls(2).unwrap_err();
+    assert!(
+        err.to_string().contains("cannot be extended with nulls"),
+        "unexpected error: {err}"
+    );
 }
 
 #[test]

parquet-variant-compute/src/variant_array.rs

@@ -409,28 +409,31 @@ impl VariantArray {
     /// Use `try_value` if you need to handle conversion errors gracefully.
     ///
     /// # Panics
-    /// * if the index is out of bounds
-    /// * if the array value is null
-    /// * if `try_value` returns an error.
+    /// Panics if
+    /// * the index is out of bounds,
+    /// * the `metadata`/`value` bytes of the row are invalid, which includes reading a null row, or
+    /// * both `value` and `typed_value` are non-null for a non-struct `typed_value`.
     pub fn value(&self, index: usize) -> Variant<'_, '_> {
-        self.try_value(index).unwrap()
+        self.try_value(index)
+            .unwrap_or_else(|err| panic!("VariantArray::value({index}) failed: {err}"))
     }
 
     /// Return the [`Variant`] instance stored at the given row
     ///
     /// Note: This method does not check for nulls and the value is arbitrary
     /// (but still well-defined) if [`is_null`](Self::is_null) returns true for the index.
     ///
-    /// # Panics
-    ///
-    /// Panics if
-    /// * the index is out of bounds
-    /// * the array value is null
-    ///
     /// # Errors
     ///
     /// Errors if
+    /// - the index is out of bounds
     /// - the data in `typed_value` cannot be interpreted as a valid `Variant`
+    /// - both `value` and `typed_value` are non-null for a non-struct `typed_value`
+    ///
+    /// # Panics
+    ///
+    /// Panics if the unshredded `metadata`/`value` bytes fail basic validation, since those are
+    /// read with [`Variant::new`]. This includes reading a row that is null.
     ///
     /// If this is a shredded variant but has no value at the shredded location, it
     /// will return [`Variant::Null`].
@@ -444,13 +447,22 @@ impl VariantArray {
     /// Note: Does not do deep validation of the [`Variant`], so it is up to the
     /// caller to ensure that the metadata and value were constructed correctly.
     pub fn try_value(&self, index: usize) -> Result<Variant<'_, '_>> {
+        if self.len() <= index {
+            return Err(ArrowError::InvalidArgumentError(format!(
+                "Index {index} out of bounds for VariantArray of length {}",
+                self.len()
+            )));
+        }
+
         let value = self.value_column();
         match self.typed_value_column() {
             // Always prefer typed_value, if available
             Some(typed_value) if typed_value.is_valid(index) => {
                 if !matches!(typed_value.data_type(), DataType::Struct(_)) && value.is_valid(index) {
                     // Only a partially shredded struct is allowed to have values for both columns
-                    panic!("Invalid variant, conflicting value and typed_value");
+                    return Err(ArrowError::InvalidArgumentError(
+                        "Invalid variant, conflicting value and typed_value".to_owned(),
+                    ));
                 }
                 typed_value_to_variant(typed_value, index)
             }
@@ -1595,6 +1607,23 @@ mod test {
         }
     }
 
+    #[test]
+    fn test_try_value_out_of_bounds() {
+        let mut b = VariantArrayBuilder::new(2);
+        b.append_variant(Variant::from(1_i8));
+        b.append_variant(Variant::Null);
+        let v = b.build();
+
+        assert_eq!(v.try_value(0).unwrap(), Variant::Int8(1));
+        assert_eq!(v.try_value(1).unwrap(), Variant::Null);
+
+        let err = v.try_value(2).unwrap_err();
+        assert_eq!(
+            err.to_string(),
+            "Invalid argument error: Index 2 out of bounds for VariantArray of length 2"
+        );
+    }
+
     #[test]
     fn test_variant_array_iterable() {
         let mut b = VariantArrayBuilder::new(6);