storage: fix s3 batch delete with unicode filenames

Cargo.toml

@@ -1,5 +1,5 @@
 [workspace]
-resolver = "2" 
+resolver = "2"
 
 members = ["crates/bin/*", "crates/lib/*", "integration_tests"]
 
@@ -38,14 +38,15 @@ chrono = { version = "0.4.11", default-features = false, features = ["clock", "s
 clap = { version = "4.0.22", features = ["derive"] }
 futures-util = "0.3.5"
 http = "1.0.0"
-itertools = "0.15.0" 
+itertools = "0.15.0"
 mime = "0.3.16"
 mockito = "1.0.2"
 num_cpus = "1.15.0"
 opentelemetry = "0.32.0"
 opentelemetry-otlp = { version = "0.32.0", features = ["grpc-tonic", "metrics"] }
 opentelemetry-resource-detectors = "0.11.0"
 opentelemetry_sdk = { version = "0.32.0", features = ["rt-tokio"] }
+percent-encoding = "2.2.0"
 postcard = { version = "1.1.3", default-features = false, features = ["use-std"] }
 pretty_assertions = "1.4.0"
 rand = "0.10"
@@ -75,7 +76,7 @@ dbg_macro = "warn"
 todo = "warn"
 
 [profile.dev]
-# recommendation coming from 
+# recommendation coming from
 # https://doc.rust-lang.org/nightly/cargo/guide/build-performance.html#reduce-amount-of-generated-debug-information
 # for our normal dev work, line-tables are good enough to see line numbers in backtraces.
 debug = "line-tables-only"
@@ -84,7 +85,7 @@ debug = "line-tables-only"
 split-debuginfo = "unpacked"
 
 [profile.dev.build-override]
-# optimize proc macros & build scripts, make them execute faster 
+# optimize proc macros & build scripts, make them execute faster
 # https://corrode.dev/blog/tips-for-faster-rust-compile-times/#avoid-procedural-macro-crates
 opt-level = 3
 

crates/lib/docs_rs_storage/Cargo.toml

@@ -44,6 +44,7 @@ itertools = { workspace = true }
 mime = { workspace = true }
 moka = { version = "0.12.14", features = ["future"] }
 opentelemetry = { workspace = true }
+percent-encoding = { workspace = true }
 rand = { workspace = true, optional = true }
 sqlx = { workspace = true } # for sqlite
 strum = { workspace = true }

crates/lib/docs_rs_storage/src/backends/s3.rs

@@ -16,7 +16,10 @@ use aws_sdk_s3::{
     config::{Region, retry::RetryConfig},
     error::{ProvideErrorMetadata, SdkError},
     primitives::{ByteStream, Length},
-    types::{ChecksumAlgorithm, CompletedMultipartUpload, CompletedPart, Delete, ObjectIdentifier},
+    types::{
+        ChecksumAlgorithm, CompletedMultipartUpload, CompletedPart, Delete, EncodingType,
+        ObjectIdentifier,
+    },
 };
 use aws_smithy_types_convert::date_time::DateTimeExt;
 use base64::{Engine as _, engine::general_purpose::STANDARD as b64};
@@ -26,6 +29,7 @@ use docs_rs_types::CompressionAlgorithm;
 use docs_rs_utils::{retry_backoff, spawn_blocking};
 use futures_util::stream::{self, BoxStream, StreamExt, TryStreamExt};
 use mime::Mime;
+use percent_encoding::percent_decode_str;
 use std::path::Path;
 use tokio::{fs, time};
 use tracing::{error, warn};
@@ -530,14 +534,21 @@ impl StorageBackendMethods for S3Backend {
                     .list_objects_v2()
                     .bucket(&self.bucket)
                     .prefix(prefix)
+                    // Without URL encoding, object keys containing characters
+                    // forbidden by XML 1.0 cannot be represented faithfully in
+                    // the ListObjects response.
+                    .encoding_type(EncodingType::Url)
                     .set_continuation_token(continuation_token)
                     .send()
                     .await?;
 
                 if let Some(contents) = list.contents {
                     for obj in contents {
                         if let Some(key) = obj.key() {
-                            yield key.to_owned();
+                            yield percent_decode_str(key)
+                                .decode_utf8()
+                                .with_context(|| format!("S3 returned a non-UTF-8 object key: {key:?}"))?
+                                .into_owned();
                         }
                     }
                 }
@@ -562,7 +573,27 @@ impl StorageBackendMethods for S3Backend {
 
 impl S3Backend {
     async fn delete_batch_with_retry(&self, keys: Vec<String>) -> Result<(), Error> {
-        let mut remaining = keys;
+        let (mut remaining, xml_incompatible_keys): (Vec<_>, Vec<_>) =
+            keys.into_iter().partition(|key| is_xml_1_0_compatible(key));
+
+        // DeleteObjects puts keys in an XML 1.0 request body. S3 object keys can
+        // contain characters which XML 1.0 cannot represent (for example ESC),
+        // causing the whole request to fail with MalformedXML. Delete those keys
+        // individually instead, since DeleteObject encodes the key in the URI.
+        for key in xml_incompatible_keys {
+            self.client
+                .delete_object()
+                .bucket(&self.bucket)
+                .key(&key)
+                .send()
+                .await
+                .with_context(|| format!("failed to delete file from s3: {key:?}"))?;
+        }
+
+        if remaining.is_empty() {
+            return Ok(());
+        }
+
         for attempt in 1.. {
             // Request-level failures are retried by the AWS SDK. We only retry
             // per-object failures returned in a successful `DeleteObjects`
@@ -642,3 +673,24 @@ impl S3Backend {
             .context("could not build delete request")
     }
 }
+
+/// Whether a string can be represented as character data in an XML 1.0 document.
+fn is_xml_1_0_compatible(value: &str) -> bool {
+    value
+        .chars()
+        .all(|c| matches!(c, '\u{9}' | '\u{a}' | '\u{d}' | '\u{20}'..='\u{d7ff}' | '\u{e000}'..='\u{fffd}' | '\u{10000}'..='\u{10ffff}'))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::is_xml_1_0_compatible;
+
+    #[test]
+    fn detects_xml_1_0_incompatible_characters() {
+        assert!(is_xml_1_0_compatible("sources/crate/1.0.0/lib.rs"));
+        assert!(is_xml_1_0_compatible("tabs\tand\nnewlines\r"));
+        assert!(is_xml_1_0_compatible("unicode-é-🦀"));
+        assert!(!is_xml_1_0_compatible("null-\0"));
+        assert!(!is_xml_1_0_compatible("escape-\u{1b}"));
+    }
+}

crates/lib/docs_rs_storage/src/storage/non_blocking.rs

@@ -1000,10 +1000,11 @@ mod backend_tests {
                 "foo/bar.txt",
                 "foo/bar/baz.txt",
                 "foo/bar/foobar.txt",
+                "foo/bar/\u{1b}",
                 "bar.txt",
             ],
             &["foo.txt", "foo/bar.txt", "bar.txt"],
-            &["foo/bar/baz.txt", "foo/bar/foobar.txt"],
+            &["foo/bar/baz.txt", "foo/bar/foobar.txt", "foo/bar/\u{1b}"],
         )
         .await
     }

crates/lib/docs_rs_uri/Cargo.toml

@@ -8,7 +8,7 @@ edition = "2024"
 [dependencies]
 askama = { workspace = true }
 http = { workspace = true }
-percent-encoding = "2.2.0"
+percent-encoding = { workspace = true }
 serde_with = { workspace = true }
 thiserror = { workspace = true }
 url = { workspace = true }