You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by xu...@apache.org on 2026/07/23 04:11:21 UTC

(opendal) branch main updated: fix(services/gcs): stop double-encoding object paths in writer (#7927)

This is an automated email from the ASF dual-hosted git repository.

Xuanwo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/opendal.git


The following commit(s) were added to refs/heads/main by this push:
     new 19e028238 fix(services/gcs): stop double-encoding object paths in writer (#7927)
19e028238 is described below

commit 19e028238cdc11398099bd25a8ef7895a15d68a7
Author: Chitral Verma <ch...@gmail.com>
AuthorDate: Thu Jul 23 09:41:16 2026 +0530

    fix(services/gcs): stop double-encoding object paths in writer (#7927)
    
    * fix(services/gcs): stop double-encoding object paths in writer
    
    The writer pre-encoded the path with gcs_percent_encode_path before
    calling gcs_insert_object_request / gcs_initiate_multipart_upload, which
    percent-encode again. A key like odfs/w/x.parquet was stored under the
    literal name odfs%2Fw%2Fx.parquet and was unreadable, since the reader
    encodes only once. Forward the raw path so it is encoded exactly once,
    matching the reader and the other multipart calls.
    
    Add regression tests asserting the write request single-encodes the path.
    
    * fix(services/gcs): percent-encode path in multipart initiation
    
    gcs_initiate_multipart_upload interpolated the object path into the URL
    without percent-encoding, unlike gcs_upload_part / gcs_complete_multipart_upload
    / gcs_abort_multipart_upload. Once the writer forwards the raw path, object
    names with characters that must be escaped (spaces, '#', non-ASCII, ...) would
    produce an invalid or wrong initiation URI. Extract the request builder and
    encode the path once, matching the other multipart calls.
    
    Add a regression test asserting the initiation URI single-encodes the path.
    
    * refactor(services/gcs): drop encoding-fix helpers per review
    
    Address review feedback: remove the GcsWriter::insert_object_request
    helper (inline into write_once), the build_test_core test constructor and
    the request-builder unit tests that relied on it, and inline the
    multipart-initiation request builder back into gcs_initiate_multipart_upload.
    
    The source fixes are unchanged: the writer forwards the raw path and
    gcs_initiate_multipart_upload percent-encodes the path once, matching the
    other multipart helpers.
---
 core/services/gcs/src/core.rs   |  7 ++++++-
 core/services/gcs/src/writer.rs | 17 +++++------------
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/core/services/gcs/src/core.rs b/core/services/gcs/src/core.rs
index 80eab2090..acaad74eb 100644
--- a/core/services/gcs/src/core.rs
+++ b/core/services/gcs/src/core.rs
@@ -605,7 +605,12 @@ impl GcsCore {
     ) -> Result<Response<Buffer>> {
         let p = build_abs_path(&self.root, path);
 
-        let url = format!("{}/{}/{}?uploads", self.endpoint, self.bucket, p);
+        let url = format!(
+            "{}/{}/{}?uploads",
+            self.endpoint,
+            self.bucket,
+            gcs_percent_encode_path(&p)
+        );
 
         let mut builder = Request::post(&url)
             .header(CONTENT_LENGTH, 0)
diff --git a/core/services/gcs/src/writer.rs b/core/services/gcs/src/writer.rs
index 769c0fbbe..4217704aa 100644
--- a/core/services/gcs/src/writer.rs
+++ b/core/services/gcs/src/writer.rs
@@ -23,7 +23,6 @@ use http::StatusCode;
 use super::core::CompleteMultipartUploadRequestPart;
 use super::core::GcsCore;
 use super::core::InitiateMultipartUploadResult;
-use super::core::gcs_percent_encode_path;
 use super::core::parse_error;
 use opendal_core::raw::*;
 use opendal_core::*;
@@ -51,12 +50,10 @@ impl GcsWriter {
 impl oio::MultipartWrite for GcsWriter {
     async fn write_once(&self, _: u64, body: Buffer) -> Result<Metadata> {
         let size = body.len() as u64;
-        let req = self.core.gcs_insert_object_request(
-            &gcs_percent_encode_path(&self.path),
-            Some(size),
-            &self.op,
-            body,
-        )?;
+        // Forward the raw path; the core request builder percent-encodes once.
+        let req = self
+            .core
+            .gcs_insert_object_request(&self.path, Some(size), &self.op, body)?;
 
         let req = self.core.sign(&self.ctx, req).await?;
 
@@ -77,11 +74,7 @@ impl oio::MultipartWrite for GcsWriter {
     async fn initiate_part(&self) -> Result<String> {
         let resp = self
             .core
-            .gcs_initiate_multipart_upload(
-                &self.ctx,
-                &gcs_percent_encode_path(&self.path),
-                &self.op,
-            )
+            .gcs_initiate_multipart_upload(&self.ctx, &self.path, &self.op)
             .await?;
 
         if !resp.status().is_success() {