You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "PDGGK (via GitHub)" <gi...@apache.org> on 2026/08/14 16:53:55 UTC

[PR] fix(services/webhdfs): encode startAfter and abort the blocks that were written [opendal]

PDGGK opened a new pull request, #8075:
URL: https://github.com/apache/opendal/pull/8075

   ### Which issue does this PR close?
   
   None — found while reading the service. Two independent path defects, both one expression.
   
   ### 1. The batched lister's `startAfter` is not encoded
   
   `webhdfs_list_status_batch` encodes the path and then appends the marker raw:
   
   ```rust
   let mut url = format!(
       "{}/webhdfs/v1/{}?op=LISTSTATUS_BATCH",
       self.endpoint,
       percent_encode_path(&p),                                 // encoded
   );
   if !start_after.is_empty() {
       url += format!("&startAfter={start_after}").as_str();    // not encoded
   }
   ```
   
   `start_after` is `ctx.token`, which the lister sets verbatim from the last entry's `pathSuffix` — a raw HDFS file name:
   
   ```rust
   ctx.token.clone_from(&file_statuses.last().unwrap().path_suffix);
   ```
   
   Measured by feeding the resulting strings to `http::Request::get`:
   
   | name | what is sent |
   |---|---|
   | `report 2024.csv` | **`BUILD ERROR: invalid uri character`** — the listing aborts |
   | `note#1.txt` | `startAfter=note` — the rest is a fragment, so the batch boundary rewinds and the page repeats |
   | `a&b=c` | `startAfter=a` plus a stray `b=c` parameter |
   
   This is the same defect just fixed for the `marker` parameter across obs/swift/cos/azblob/azfile in #8073 — it was outside 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 that carries data the *server* chose. `user.name` is configuration, appears at eleven sites, and is a separate question; `&{auth}` is deliberately a whole query fragment and must stay verbatim. Neither is touched here.
   
   ### 2. `abort_block` deletes paths that were never written
   
   `write_block` creates each block under the atomic write dir:
   
   ```rust
   .webhdfs_create_object(&self.ctx, &format!("{atomic_write_dir}{block_id}"), ...)
   ```
   
   and `complete_block` concatenates from the same strings. But `abort_block` asked for the bare id:
   
   ```rust
   .webhdfs_delete(&self.ctx, &block_id.to_string())
   ```
   
   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**. WebHDFS answers `DELETE` on a missing path with `200` and `{"boolean": false}`, so the leak is silent.
   
   It now resolves `atomic_write_dir` the same way `write_block` does, which also makes the unsupported case explicit instead of deleting a top-level path named after a UUID.
   
   ### Tests
   
   No new tests, deliberately: both fixes are single expressions inside async methods whose only seam is an HTTP round trip, and asserting on either would mean restructuring the URL builder and the writer — which I would rather not fold into a fix. The 7 existing unit tests pass; `cargo fmt --all -- --check` and `cargo clippy -p opendal-service-webhdfs --all-targets` are clean with zero warnings.
   
   I kept the two together because they are one-liners in one service; happy to split if you would prefer them reviewed separately.
   
   ### Are there any user-facing changes?
   
   Yes, for `services-webhdfs`: a batched listing whose boundary file name contains a reserved character now resumes correctly instead of failing to build the request or repeating a page, and an aborted multi-block write removes its blocks instead of leaving them behind.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(services/webhdfs): encode startAfter and abort the blocks that were written [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #8075:
URL: https://github.com/apache/opendal/pull/8075


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org