[pbs-devel] applied-series: [PATCH proxmox-backup-qemu 1/2] fix: never append archive name extensions twice, use type instead

[Fabian Grünbichler]

thanks!

On January 28, 2025 4:09 pm, Christian Ebner wrote:
> Commit 16298c5f ("update to current PBS master") introduced a helper
> method to generate the backup archive names from a device name.
> 
> However, the helper was also used incorrectly to generate archive
> names from strings already having the `.img.fidx` filename extension.
> As the extension is appended unconditionally, this leads to an
> archive name which will parse successfully, but not be the expected
> archive name to be found in the backup manifest.
> 
> Fix this by dropping the incorrect helper call-sites and adapt the
> function parameters to take a `BackupArchiveName` directly, reducing
> potential for future mishandling.
> 
> Fixes: Commit 16298c5f ("update to current PBS master")
> Reported-by: Markus Frank <m.frank at proxmox.com>
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
>  src/lib.rs     | 14 +++++++++++---
>  src/restore.rs | 10 ++++------
>  2 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/src/lib.rs b/src/lib.rs
> index 0a1239b..ed94c9c 100644
> --- a/src/lib.rs
> +++ b/src/lib.rs
> @@ -8,7 +8,7 @@ use std::sync::{Arc, Condvar, Mutex};
>  
>  use proxmox_lang::try_block;
>  
> -use pbs_api_types::{Authid, BackupDir, BackupNamespace, BackupType, CryptMode};
> +use pbs_api_types::{Authid, BackupArchiveName, BackupDir, BackupNamespace, BackupType, CryptMode};
>  use pbs_client::BackupRepository;
>  
>  pub mod capi_types;
> @@ -979,6 +979,7 @@ pub extern "C" fn proxmox_restore_image(
>      let result: Result<_, Error> = try_block!({
>          let archive_name = tools::utf8_c_string(archive_name)?
>              .ok_or_else(|| format_err!("archive_name must not be NULL"))?;
> +        let archive_name: BackupArchiveName = archive_name.parse()?;
>  
>          let write_data_callback = move |offset: u64, data: &[u8]| {
>              callback(callback_data, offset, data.as_ptr(), data.len() as u64)
> @@ -988,7 +989,7 @@ pub extern "C" fn proxmox_restore_image(
>              move |offset: u64, len: u64| callback(callback_data, offset, std::ptr::null(), len);
>  
>          proxmox_async::runtime::block_on(restore_task.restore_image(
> -            archive_name,
> +            &archive_name,
>              write_data_callback,
>              write_zero_callback,
>              verbose,
> @@ -1051,9 +1052,16 @@ pub extern "C" fn proxmox_restore_open_image_async(
>  
>      param_not_null!(archive_name, callback_info);
>      let archive_name = unsafe { tools::utf8_c_string_lossy_non_null(archive_name) };
> +    let archive_name = match archive_name.parse() {
> +        Ok(archive_name) => archive_name,
> +        Err(err) => {
> +            callback_info.send_result(Err(err));
> +            return;
> +        }
> +    };
>  
>      restore_task.runtime().spawn(async move {
> -        let result = match restore_task.open_image(archive_name).await {
> +        let result = match restore_task.open_image(&archive_name).await {
>              Ok(res) => Ok(res as i32),
>              Err(err) => Err(err),
>          };
> diff --git a/src/restore.rs b/src/restore.rs
> index d3818e9..a879b99 100644
> --- a/src/restore.rs
> +++ b/src/restore.rs
> @@ -3,6 +3,7 @@ use std::sync::{Arc, Mutex};
>  
>  use anyhow::{bail, format_err, Error};
>  use once_cell::sync::OnceCell;
> +use pbs_api_types::BackupArchiveName;
>  use tokio::runtime::Runtime;
>  
>  use proxmox_async::runtime::get_runtime_with_builder;
> @@ -19,7 +20,6 @@ use pbs_tools::crypt_config::CryptConfig;
>  
>  use super::BackupSetup;
>  use crate::capi_types::DataPointer;
> -use crate::commands::archive_name;
>  use crate::registry::Registry;
>  use crate::shared_cache::get_shared_chunk_cache;
>  
> @@ -119,12 +119,11 @@ impl RestoreTask {
>  
>      pub async fn restore_image(
>          &self,
> -        archive_str: String,
> +        archive_name: &BackupArchiveName,
>          write_data_callback: impl Fn(u64, &[u8]) -> i32,
>          write_zero_callback: impl Fn(u64, u64) -> i32,
>          verbose: bool,
>      ) -> Result<(), Error> {
> -        let archive_name = archive_name(&archive_str)?;
>          if verbose {
>              eprintln!("download and verify backup index");
>          }
> @@ -218,8 +217,7 @@ impl RestoreTask {
>          Ok(info.archive_size)
>      }
>  
> -    pub async fn open_image(&self, archive_str: String) -> Result<u8, Error> {
> -        let archive_name = archive_name(&archive_str)?;
> +    pub async fn open_image(&self, archive_name: &BackupArchiveName) -> Result<u8, Error> {
>          let client = match self.client.get() {
>              Some(reader) => Arc::clone(reader),
>              None => bail!("not connected"),
> @@ -254,7 +252,7 @@ impl RestoreTask {
>  
>          let info = ImageAccessInfo {
>              archive_size,
> -            _archive_name: archive_str,
> +            _archive_name: archive_name.to_string(),
>              // useful to debug
>              reader,
>          };
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
>