[pbs-devel] [PATCH proxmox] s3 client: fix invalid request for empty delete_objects call

[pbs-devel] [PATCH proxmox] s3 client: fix invalid request for empty delete_objects call

[Christian Ebner]

The s3 client's object_delete method does not correctly handle cases
where an empty list of object keys is passed via the parameters,
resulting in an `invalid request` error.

Handle this by early returning in case of an empty list, thereby also
skipping the unneeded api request altogether.

This indirectly fixes the snapshot directory cleanup for
proxmox-backup-client benchmark or aborted backups, where no objects
have been persisted to the object store within the snapshot prefix
yet, therefore trying to delete the empty list.

Reported-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Note: as this indirectly fixes an issue in PBS, latter must be build
against a bumped version of the proxmox-s3-client.

Further, I'm working on improving the error logging for such invalid
requests, as they currently do not log the error in the response body
(if any) in these cases.

 proxmox-s3-client/src/client.rs          | 4 ++++
 proxmox-s3-client/src/response_reader.rs | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
index eb5fc7d9..3a981bf4 100644
--- a/proxmox-s3-client/src/client.rs
+++ b/proxmox-s3-client/src/client.rs
@@ -423,6 +423,10 @@ impl S3Client {
         &self,
         object_keys: &[S3ObjectKey],
     ) -> Result<DeleteObjectsResponse, Error> {
+        if object_keys.is_empty() {
+            return Ok(DeleteObjectsResponse::default());
+        }
+
         let mut body = String::from(r#"<Delete xmlns="http://s3.amazonaws.com/doc/2006-03-01/">"#);
         for object_key in object_keys {
             body.push_str("<Object><Key>");
diff --git a/proxmox-s3-client/src/response_reader.rs b/proxmox-s3-client/src/response_reader.rs
index 41e8c6d8..f0baa107 100644
--- a/proxmox-s3-client/src/response_reader.rs
+++ b/proxmox-s3-client/src/response_reader.rs
@@ -107,7 +107,7 @@ pub enum PutObjectResponse {
 
 /// Subset of the delete objects response
 /// https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html#API_DeleteObjects_ResponseElements
-#[derive(Deserialize, Debug)]
+#[derive(Deserialize, Debug, Default)]
 #[serde(rename_all = "PascalCase")]
 pub struct DeleteObjectsResponse {
     pub deleted: Option<Vec<DeletedObject>>,
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox] s3 client: fix invalid request for empty delete_objects call

[Christian Ebner]

On 7/23/25 11:15 AM, Christian Ebner wrote:
> The s3 client's object_delete method does not correctly handle cases
> where an empty list of object keys is passed via the parameters,
> resulting in an `invalid request` error.
> 
> Handle this by early returning in case of an empty list, thereby also
> skipping the unneeded api request altogether.
> 
> This indirectly fixes the snapshot directory cleanup for
> proxmox-backup-client benchmark or aborted backups, where no objects
> have been persisted to the object store within the snapshot prefix
> yet, therefore trying to delete the empty list.
> 
> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---

Forgot to include the independent report of the same issue by Lukas, 
which was however incorrectly diagnosed as an of out-of-storage-space issue.

Therefore, please include also:

Reported-by: Lukas Wagner <l.wagner@proxmox.com>


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH proxmox] s3 client: fix invalid request for empty delete_objects call

[Thomas Lamprecht]

Am 23.07.25 um 11:15 schrieb Christian Ebner:
> The s3 client's object_delete method does not correctly handle cases
> where an empty list of object keys is passed via the parameters,
> resulting in an `invalid request` error.
> 
> Handle this by early returning in case of an empty list, thereby also
> skipping the unneeded api request altogether.
> 
> This indirectly fixes the snapshot directory cleanup for
> proxmox-backup-client benchmark or aborted backups, where no objects
> have been persisted to the object store within the snapshot prefix
> yet, therefore trying to delete the empty list.
> 
> Reported-by: Dominik Csapak <d.csapak@proxmox.com>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Note: as this indirectly fixes an issue in PBS, latter must be build
> against a bumped version of the proxmox-s3-client.
> 
> Further, I'm working on improving the error logging for such invalid
> requests, as they currently do not log the error in the response body
> (if any) in these cases.
> 
>  proxmox-s3-client/src/client.rs          | 4 ++++
>  proxmox-s3-client/src/response_reader.rs | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
>

applied, thanks!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel