From 8d51ba9a23b6ebbe47642c341e7065555ed4f801 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Fri, 1 May 2026 17:59:27 +0100 Subject: [PATCH 1/2] add a test triggering config lifetime issue --- src/encoder.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/encoder.rs b/src/encoder.rs index 369ee23..aa9e1a6 100644 --- a/src/encoder.rs +++ b/src/encoder.rs @@ -438,6 +438,38 @@ mod tests { assert_eq!(frames[0].data(), &[0u8; 400 * 400 * 4]); } + #[test] + fn test_per_frame_encoding_config_lifetime() { + let config = EncodingConfig { + quality: 100., + ..Default::default() + }; + let frame = [0u8; 4 * 4 * 4]; + + // Control case: storing the config in the encoder keeps the + // ConfigContainer alive across the WebPAnimEncoderAdd FFI call. + let mut default_config_encoder = Encoder::new_with_options( + (4, 4), + EncoderOptions { + encoding_config: Some(config.clone()), + ..Default::default() + }, + ) + .unwrap(); + default_config_encoder.add_frame(&frame, 0).unwrap(); + assert!(default_config_encoder.finalize(10).unwrap().len() > 0); + + // Reproducer: add_frame_with_config currently creates a temporary + // ConfigContainer while building the raw FFI argument. That temporary is + // dropped before libwebp reads the pointer, so optimized/ASan builds can + // observe dead stack data and reject an otherwise valid configuration. + let mut per_frame_config_encoder = Encoder::new((4, 4)).unwrap(); + per_frame_config_encoder + .add_frame_with_config(&frame, 0, &config) + .unwrap(); + assert!(per_frame_config_encoder.finalize(10).unwrap().len() > 0); + } + #[test] fn test_failures() { let mut encoder = Encoder::new((400, 400)).unwrap(); From 8dca5b61c00555e62f207d89462ee3fb0eac99b7 Mon Sep 17 00:00:00 2001 From: "Sergey \"Shnatsel\" Davidoff" Date: Fri, 1 May 2026 18:01:58 +0100 Subject: [PATCH 2/2] Fix the config lifetime issue --- src/encoder.rs | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/encoder.rs b/src/encoder.rs index aa9e1a6..0058539 100644 --- a/src/encoder.rs +++ b/src/encoder.rs @@ -164,21 +164,29 @@ impl Encoder { self.frame.set_data(data, self.options.color_mode)?; + // Keep the per-frame config container in a local binding so the raw + // pointer passed to libwebp remains valid until WebPAnimEncoderAdd + // returns. Building this inside the FFI argument list would drop the + // temporary before libwebp reads it. + let frame_encoding_config = match config { + Some(config) => Some(config.to_config_container()?), + None => None, + }; + + // Derive the raw pointer from bindings that outlive the unsafe call: + // either the per-frame binding above or the encoder's stored default. + let encoding_config = match (&frame_encoding_config, &self.encoding_config) { + (Some(config), _) => config.as_ptr(), + (None, Some(config)) => config.as_ptr(), + (None, None) => ptr::null(), + }; + if unsafe { webp::WebPAnimEncoderAdd( self.encoder_wr.encoder, self.frame.as_webp_picture_ref(), timestamp, - match config { - Some(config) => { - let config = config.to_config_container()?; - config.as_ptr() - } - None => match &self.encoding_config { - Some(config) => config.as_ptr(), - None => std::ptr::null(), - }, - }, + encoding_config, ) } == 0 { @@ -459,10 +467,10 @@ mod tests { default_config_encoder.add_frame(&frame, 0).unwrap(); assert!(default_config_encoder.finalize(10).unwrap().len() > 0); - // Reproducer: add_frame_with_config currently creates a temporary - // ConfigContainer while building the raw FFI argument. That temporary is - // dropped before libwebp reads the pointer, so optimized/ASan builds can - // observe dead stack data and reject an otherwise valid configuration. + // Regression check: add_frame_with_config used to create a temporary + // ConfigContainer while building the raw FFI argument. That temporary + // was dropped before libwebp read the pointer, so optimized/ASan builds + // could observe dead stack data and reject this valid configuration. let mut per_frame_config_encoder = Encoder::new((4, 4)).unwrap(); per_frame_config_encoder .add_frame_with_config(&frame, 0, &config)