[I] bug: GCS Operator `credential_path` broken when I use a client that reads raw gzip [opendal]

["wlinna (via GitHub)" <gi...@apache.org>]

wlinna opened a new issue, #8418:
URL: https://github.com/apache/opendal/issues/8418

   ### OpenDAL version or commit
   
   v0.59.5
   
   ### Affected service or component
   
   services/gcs
   
   ### Describe the bug
   
   Hello,
   
   I just migrated from version 0.55 to version 0.59 series, and I noticed that every version since 0.56 has been broken for my use case whenever I use `credential_path` option of a GCS operator.
   
   In version 0.55, I used to be able to create a GCS operator like this:
   
   ```rust
   let mut headers = HeaderMap::new();
   headers.insert(ACCEPT_ENCODING, HeaderValue::from_static("gzip"));
   let client = reqwest::Client::builder()
       .default_headers(headers)
       .no_gzip()
       .build()?;
   
   let mut gcs = Gcs::default()
       .bucket(&bucket)
       .http_client(HttpClient::with(client));
   if let Ok(path_credential) = std::env::var("PATH_SERVICE_ACCOUNT") {
       gcs = gcs.credential_path(&path_credential);
   }
   let op = Operator::new(gcs)?.finish();
   ```
   
   I ported this to the new paradigm and now it looks something like this:
   
   ```rust
   let mut headers = HeaderMap::new();
   headers.insert(ACCEPT_ENCODING, HeaderValue::from_static("gzip"));
   let client = reqwest::Client::builder()
       .default_headers(headers)
       .no_gzip()
       .build()?;
   
   let mut gcs = Gcs::default().bucket(&bucket);
   if let Ok(path_credential) = std::env::var("PATH_SERVICE_ACCOUNT") {
       gcs = gcs.credential_path(&path_credential);
   }
   let op = Operator::new(gcs)?
       .with_context(OperationContext::new()
           .with_http_transport(HttpTransporter::new(ReqwestTransport::new(client))));
   ```
   
   In my program I need to handle the compression myself, and for that reason I have to accept gzip while also preventing its automatic decompression.
   
   Then when I try to read/write a filie, I get errors such as this 
   ```
   Error in uploading job/progress.json: Unexpected (permanent) at write, context: { called: reqsign::Sign, service: gcs, path: job/progress.json, written: 605 } => signing http request, source: failed to parse token response
   ```
   
   In my opinion, `opendal` should allow me to use this kind of client without causing failures within `opendal`.
   
   
   
   ### Steps to Reproduce
   
   Here's a minimal repro that demonstrates the problem
   
   ```rust
   use http::{Request, Response};
   use opendal::{services::Gcs, Buffer, HttpBody, HttpTransport, HttpTransporter, OperationContext, Operator};
   use opendal_http_transport_reqwest::ReqwestTransport;
   use reqwest::header::{HeaderMap, HeaderValue, ACCEPT_ENCODING};
   
   const SERVICE_ACCOUNT_KEY: &str = "service-account.json";
   // https://cloud.google.com/storage/docs/public-datasets/landsat
   const BUCKET: &str = "gcp-public-data-landsat";
   const OBJECT: &str = "index.csv.gz";
   
   /// Workaround: ask for gzip only on storage requests, so the token request gets plain JSON.
   // struct StorageGzipTransport(ReqwestTransport);
   
   // impl HttpTransport for StorageGzipTransport {
   //     async fn fetch(&self, mut req: Request<Buffer>) -> opendal::Result<Response<HttpBody>> {
   //         if req.uri().host() == Some("storage.googleapis.com") {
   //             req.headers_mut().insert(ACCEPT_ENCODING, HeaderValue::from_static("gzip"));
   //         }
   //         self.0.fetch(req).await
   //     }
   // }
   
   async fn stat(label: &str, transport: HttpTransporter) {
       let gcs = Gcs::default().bucket(BUCKET).credential_path(SERVICE_ACCOUNT_KEY);
       let op = Operator::new(gcs).unwrap().with_context(OperationContext::new().with_http_transport(transport));
       match op.stat(OBJECT).await {
           Ok(meta) => println!("{label}: ok, {} bytes", meta.content_length()),
           Err(err) => println!("{label}: {err}"),
       }
   }
   
   #[tokio::main(flavor = "current_thread")]
   async fn main() {
       stat("plain client", HttpTransporter::new(ReqwestTransport::new(reqwest::Client::new()))).await;
   
       let mut headers = HeaderMap::new();
       headers.insert(ACCEPT_ENCODING, HeaderValue::from_static("gzip"));
       let gzip_client = reqwest::Client::builder().default_headers(headers).no_gzip().build().unwrap();
       stat("client with Accept-Encoding: gzip", HttpTransporter::new(ReqwestTransport::new(gzip_client))).await;
   
       // let raw_client = reqwest::Client::builder().no_gzip().build().unwrap();
   
       // let workaround = StorageGzipTransport(ReqwestTransport::new(raw_client));
       // stat("workaround transport", HttpTransporter::new(workaround)).await;
   }
   
   ```
   
   ### Actual Behavior
   
   
   plain client: ok, 767466786 bytes
   client with Accept-Encoding: gzip: Unexpected (permanent) at stat, context: { called: reqsign::Sign, service: gcs, path: index.csv.gz } => signing http request, source: failed to load signing credential
   
   As you can see, if I create a client that does not handle gzip, the internal machinery of `opendal` fails.
   
   ### Expected Behavior
   
   plain client: ok, 767466786 bytes
   client with Accept-Encoding: gzip: ok, 767466786 bytes
   
   
   
   ### Additional Context
   
     `opendal 0.55`  uses its internal client, which allows me to control what kind of output I get from GCS Operator without causing `opendal` to fail and without having to know about its internals. I think that was the correct model. Or if `opendal` should use the supplied client, it should set the correct headers and other parameters for all the internally generated requests per request (and not force the user to use specific settings).
   
   
   ### Reproduction confirmation
   
   - [x] I reproduced this issue against the affected component. For a service-specific report, I used the actual service rather than only a mock, emulator, or source-code analysis.
   
   ### Are you willing to submit a PR to fix this bug?
   
   - [ ] Yes, I would like to submit a PR.


-- 
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.apache.org

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

Re: [I] bug: GCS Operator `credential_path` broken when I use a client that reads raw gzip [opendal]

["Xuanwo (via GitHub) via commits" <co...@opendal.apache.org>]

Xuanwo closed issue #8418: bug: GCS Operator `credential_path` broken when I use a client that reads raw gzip
URL: https://github.com/apache/opendal/issues/8418


-- 
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: [I] bug: GCS Operator `credential_path` broken when I use a client that reads raw gzip [opendal]

["Xuanwo (via GitHub)" <gi...@apache.org>]

Xuanwo commented on issue #8418:
URL: https://github.com/apache/opendal/issues/8418#issuecomment-6334947401

   It should be better if we can get https://github.com/seanmonstar/reqwest/pull/2667 in


-- 
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