From 71a28328f73d2d2383490d1396e6b8e2ad62f76f Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Wed, 2 Sep 2026 09:55:34 +1200 Subject: [PATCH 1/2] Check for key retriever in decryption properties conversion --- datafusion/common/src/config.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/datafusion/common/src/config.rs b/datafusion/common/src/config.rs index 4258ce6f003a9..ad3fbdcc26ee7 100644 --- a/datafusion/common/src/config.rs +++ b/datafusion/common/src/config.rs @@ -3672,11 +3672,20 @@ impl TryFrom<&Arc> for ConfigFileDecryptionProperties type Error = DataFusionError; fn try_from(f: &Arc) -> Result { + if f.uses_key_retriever() { + // Getting the keys is not possible without the key metadata from + // a Parquet file if a key retriever is used. + return Err( + DataFusionError::Configuration( + "Cannot convert FileDecryptionProperties that use a key retriever to ConfigFileDecryptionProperties".into() + ) + ); + } + let footer_key = f.footer_key(None).map_err(|e| { + // This shouldn't happen for FileDecryptionProperties that don't use a key retriever. DataFusionError::Configuration(format!( - "Could not retrieve footer key from FileDecryptionProperties. \ - Note that conversion to ConfigFileDecryptionProperties is not supported \ - when using a key retriever: {e}" + "Could not retrieve footer key from FileDecryptionProperties: {e}" )) })?; @@ -4474,8 +4483,10 @@ mod tests { (&decryption_properties).try_into(); assert!(config_file_decryption_properties.is_err()); let err = config_file_decryption_properties.unwrap_err().to_string(); - assert!(err.contains("key retriever")); - assert!(err.contains("Key metadata not provided")); + assert_contains!( + err, + "Cannot convert FileDecryptionProperties that use a key retriever to ConfigFileDecryptionProperties" + ); } #[cfg(feature = "parquet")] From 1665dd0882033f4ac839f704bc5e467c13f9c275 Mon Sep 17 00:00:00 2001 From: Adam Reeve Date: Tue, 8 Sep 2026 12:16:58 +1200 Subject: [PATCH 2/2] Ignore key_metadata in test KeyRetriever implementation --- datafusion/common/src/config.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/datafusion/common/src/config.rs b/datafusion/common/src/config.rs index ad3fbdcc26ee7..7719e2b6af809 100644 --- a/datafusion/common/src/config.rs +++ b/datafusion/common/src/config.rs @@ -4456,14 +4456,10 @@ mod tests { #[cfg(feature = "parquet_encryption")] impl parquet::encryption::decrypt::KeyRetriever for ParquetEncryptionKeyRetriever { - fn retrieve_key(&self, key_metadata: &[u8]) -> parquet::errors::Result> { - if !key_metadata.is_empty() { - Ok(b"1234567890123450".to_vec()) - } else { - Err(parquet::errors::ParquetError::General( - "Key metadata not provided".to_string(), - )) - } + fn retrieve_key(&self, _key_metadata: &[u8]) -> parquet::errors::Result> { + // Ignore key metadata so we can verify that the key retriever isn't used + // even if it can provide a key without using metadata. + Ok(b"1234567890123450".to_vec()) } }