From 9d3199258ee33cab46e39812fc910f763e072d31 Mon Sep 17 00:00:00 2001
From: Zihan Dai <99155080+PDGGK@users.noreply.github.com>
Date: Sat, 15 Aug 2026 02:53:23 +1000
Subject: [PATCH] fix(services/webhdfs): encode startAfter and abort the blocks
 that were written

Two independent path defects in this service.

1. The batched lister's startAfter is not encoded.

    let mut url = format!(
        "{}/webhdfs/v1/{}?op=LISTSTATUS_BATCH",
        self.endpoint,
        percent_encode_path(&p),      // the path is encoded
    );
    if !start_after.is_empty() {
        url += format!("&startAfter={start_after}").as_str();   // this is not
    }

start_after is ctx.token, which lister.rs sets verbatim from the last
entry's pathSuffix -- a raw HDFS file name. Measured against
http::Request::get: a name with a space is a hard "invalid uri
character" build error, one with # truncates the marker so the batch
boundary rewinds and a page repeats, and one with & grafts a stray
parameter onto the query.

This is the same defect just fixed for the marker parameter across
obs/swift/cos/azblob/azfile in #8073; it was out of that PR's scope only
because it is a format! concatenation rather than a QueryPairsWriter
push. startAfter is the one query value in this file carrying data the
server chose. user.name is config, at eleven sites, and is a separate
question; &{auth} is deliberately a whole query fragment and must stay
verbatim.

2. abort_block deletes paths that were never written.

write_block creates each block at {atomic_write_dir}{block_id} and
complete_block concatenates from the same strings, but abort_block asked
for {block_id} alone. So aborting a multi-block write -- Writer::abort,
or any mid-write failure -- deleted a path that does not exist and left
every uploaded block sitting in atomic_write_dir for ever.

It now resolves atomic_write_dir the same way write_block does, which
also makes the unsupported case explicit rather than deleting a
top-level path named after a UUID.

No new tests: both are single expressions inside async methods whose
seams are an HTTP round trip, and asserting on either would mean
restructuring the URL builder and the writer. The seven existing unit
tests pass, fmt and clippy are clean.
---
 core/services/webhdfs/src/core.rs   | 2 +-
 core/services/webhdfs/src/writer.rs | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/core/services/webhdfs/src/core.rs b/core/services/webhdfs/src/core.rs
index 6e9a97907448..6cf6ddb51376 100644
--- a/core/services/webhdfs/src/core.rs
+++ b/core/services/webhdfs/src/core.rs
@@ -314,7 +314,7 @@ impl WebhdfsCore {
             percent_encode_path(&p),
         );
         if !start_after.is_empty() {
-            url += format!("&startAfter={start_after}").as_str();
+            url += format!("&startAfter={}", percent_encode_path(start_after)).as_str();
         }
         if let Some(user) = &self.user_name {
             url += format!("&user.name={user}").as_str();
diff --git a/core/services/webhdfs/src/writer.rs b/core/services/webhdfs/src/writer.rs
index ef6650531eaf..7336689228b7 100644
--- a/core/services/webhdfs/src/writer.rs
+++ b/core/services/webhdfs/src/writer.rs
@@ -135,10 +135,16 @@ impl oio::BlockWrite for WebhdfsWriter {
     }
 
     async fn abort_block(&self, block_ids: Vec<Uuid>) -> Result<()> {
+        let Some(ref atomic_write_dir) = self.core.atomic_write_dir else {
+            return Err(Error::new(
+                ErrorKind::Unsupported,
+                "write multi is not supported when atomic is not set",
+            ));
+        };
         for block_id in block_ids {
             let resp = self
                 .core
-                .webhdfs_delete(&self.ctx, &block_id.to_string())
+                .webhdfs_delete(&self.ctx, &format!("{atomic_write_dir}{block_id}"))
                 .await?;
             match resp.status() {
                 StatusCode::OK => {}