From 836657565e18288d873b0a1507ed2421505ece8c Mon Sep 17 00:00:00 2001
From: milouz1985 <francois.hoyez@gmail.com>
Date: Sun, 15 Mar 2026 10:40:50 +0000
Subject: [PATCH] s3: fix DeleteObjects XML parsing with pretty-printed bodies
 (#1374)

## Summary

This PR fixes S3 `DeleteObjects` XML parsing when the request body is pretty-printed (contains indentation/newlines as whitespace text nodes).

Although PR #1324 already tried to address this, parsing could still fail with:

`InvalidRequest: Bad request: Invalid delete XML query`

because non-element nodes were validated but not actually skipped in the parsing loop.

## What changed

- In `src/api/s3/delete.rs`:
  - Properly skip non-element whitespace text nodes while iterating over `<Delete>` children.
  - Keep rejecting non-whitespace stray text content.
  - Parse the root `<Delete>` element more robustly by selecting the first element child.

## Tests added

New unit tests in `src/api/s3/delete.rs`:

- `parse_delete_objects_xml_with_formatting`
  - pretty-printed valid XML is accepted.
- `parse_delete_objects_xml_accepts_compact_valid_xml`
  - compact valid XML is accepted.
- `parse_delete_objects_xml_rejects_non_whitespace_text_node`
  - compact XML with stray text is rejected.
- `parse_delete_objects_xml_rejects_pretty_print_with_stray_text`
  - pretty-printed XML with stray text is rejected.

## Validation

Executed:

```bash
cargo test -p garage_api_s3 parse_delete_objects_xml -- --nocapture
```

Result: all parser tests pass.
Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1374
Co-authored-by: milouz1985 <francois.hoyez@gmail.com>
Co-committed-by: milouz1985 <francois.hoyez@gmail.com>
---
 src/api/s3/delete.rs | 61 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/src/api/s3/delete.rs b/src/api/s3/delete.rs
index 5370a1e8..43824a04 100644
--- a/src/api/s3/delete.rs
+++ b/src/api/s3/delete.rs
@@ -125,19 +125,20 @@ fn parse_delete_objects_xml(xml: &roxmltree::Document) -> Option<DeleteRequest>
 	};
 
 	let root = xml.root();
-	let delete = root.first_child()?;
+	let delete = root.children().find(|n| n.is_element())?;
 
 	if !delete.has_tag_name("Delete") {
 		return None;
 	}
 
 	for item in delete.children() {
-		// Only parse <Part> nodes
+		// Skip text nodes introduced by formatted XML.
 		if !item.is_element() {
 			// text nodes are allowed only if they contain whitespace characters only
 			if !item.text()?.trim().is_empty() {
 				return None;
 			}
+			continue;
 		}
 
 		if item.has_tag_name("Object") {
@@ -155,3 +156,59 @@ fn parse_delete_objects_xml(xml: &roxmltree::Document) -> Option<DeleteRequest>
 
 	Some(ret)
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn parse_delete_objects_xml_with_formatting() {
+		let body = r#"
+			<Delete xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
+			    <Object>
+			        <Key>1_746573745f66696c65</Key>
+			    </Object>
+			    <Quiet>true</Quiet>
+			</Delete>
+		"#;
+		let xml = roxmltree::Document::parse(body).expect("valid delete XML");
+		let req = parse_delete_objects_xml(&xml).expect("request should be parsed");
+
+		assert_eq!(req.objects.len(), 1);
+		assert_eq!(req.objects[0].key, "1_746573745f66696c65");
+		assert!(req.quiet);
+	}
+
+	#[test]
+	fn parse_delete_objects_xml_rejects_non_whitespace_text_node() {
+		let body = r#"<Delete xmlns="http://s3.amazonaws.com/doc/2006-03-01/">oops<Object><Key>1_746573745f66696c65</Key></Object></Delete>"#;
+		let xml = roxmltree::Document::parse(body).expect("valid XML");
+		let req = parse_delete_objects_xml(&xml);
+		assert!(req.is_none());
+	}
+
+	#[test]
+	fn parse_delete_objects_xml_rejects_pretty_print_with_stray_text() {
+		let body = r#"
+			<Delete xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
+			    oops
+			    <Object>
+			        <Key>1_746573745f66696c65</Key>
+			    </Object>
+			</Delete>
+		"#;
+		let xml = roxmltree::Document::parse(body).expect("valid XML");
+		let req = parse_delete_objects_xml(&xml);
+		assert!(req.is_none());
+	}
+
+	#[test]
+	fn parse_delete_objects_xml_accepts_compact_valid_xml() {
+		let body = r#"<Delete xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Object><Key>1_746573745f66696c65</Key></Object><Quiet>false</Quiet></Delete>"#;
+		let xml = roxmltree::Document::parse(body).expect("valid XML");
+		let req = parse_delete_objects_xml(&xml).expect("request should be parsed");
+		assert_eq!(req.objects.len(), 1);
+		assert_eq!(req.objects[0].key, "1_746573745f66696c65");
+		assert!(!req.quiet);
+	}
+}