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 2023/08/28 07:48:39 UTC

[incubator-opendal] branch apply-http2-tricks created (now 6c744edaf)

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

xuanwo pushed a change to branch apply-http2-tricks
in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git


      at 6c744edaf fix(core): Make sure OpenDAL works with http2 on GCS

This branch includes the following new commits:

     new 6c744edaf fix(core): Make sure OpenDAL works with http2 on GCS

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[incubator-opendal] 01/01: fix(core): Make sure OpenDAL works with http2 on GCS

Posted by xu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

xuanwo pushed a commit to branch apply-http2-tricks
in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git

commit 6c744edaf5cabd5f5d248514a50d75af5a9accc7
Author: Xuanwo <gi...@xuanwo.io>
AuthorDate: Mon Aug 28 15:48:07 2023 +0800

    fix(core): Make sure OpenDAL works with http2 on GCS
    
    Signed-off-by: Xuanwo <gi...@xuanwo.io>
---
 core/src/services/gcs/backend.rs | 10 +---------
 core/src/services/gcs/core.rs    | 27 +++++++++++++++++++++++++--
 core/src/services/s3/core.rs     | 27 +++++++++++++++++++++++++--
 core/src/types/list.rs           |  5 +++--
 4 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/core/src/services/gcs/backend.rs b/core/src/services/gcs/backend.rs
index 27140c78f..d7d7941b3 100644
--- a/core/src/services/gcs/backend.rs
+++ b/core/src/services/gcs/backend.rs
@@ -21,7 +21,6 @@ use std::fmt::Formatter;
 use std::sync::Arc;
 
 use async_trait::async_trait;
-use http::header::HOST;
 use http::StatusCode;
 use log::debug;
 use reqsign::GoogleCredentialLoader;
@@ -586,14 +585,7 @@ impl Accessor for GcsBackend {
         self.core.sign_query(&mut req, args.expire()).await?;
 
         // We don't need this request anymore, consume it directly.
-        let (mut parts, _) = req.into_parts();
-        // Always remove host header, let users' client to set it based on HTTP
-        // version.
-        //
-        // As discussed in <https://github.com/seanmonstar/reqwest/issues/1809>,
-        // google server could send RST_STREAM of PROTOCOL_ERROR if our request
-        // contains host header.
-        parts.headers.remove(HOST);
+        let (parts, _) = req.into_parts();
 
         Ok(RpPresign::new(PresignedRequest::new(
             parts.method,
diff --git a/core/src/services/gcs/core.rs b/core/src/services/gcs/core.rs
index 3608b445c..9a5a35c48 100644
--- a/core/src/services/gcs/core.rs
+++ b/core/src/services/gcs/core.rs
@@ -26,6 +26,7 @@ use bytes::Bytes;
 use http::header::CONTENT_LENGTH;
 use http::header::CONTENT_RANGE;
 use http::header::CONTENT_TYPE;
+use http::header::HOST;
 use http::header::IF_MATCH;
 use http::header::IF_NONE_MATCH;
 use http::Request;
@@ -107,7 +108,19 @@ impl GcsCore {
     pub async fn sign<T>(&self, req: &mut Request<T>) -> Result<()> {
         let cred = self.load_token().await?;
 
-        self.signer.sign(req, &cred).map_err(new_request_sign_error)
+        self.signer
+            .sign(req, &cred)
+            .map_err(new_request_sign_error)?;
+
+        // Always remove host header, let users' client to set it based on HTTP
+        // version.
+        //
+        // As discussed in <https://github.com/seanmonstar/reqwest/issues/1809>,
+        // google server could send RST_STREAM of PROTOCOL_ERROR if our request
+        // contains host header.
+        req.headers_mut().remove(HOST);
+
+        Ok(())
     }
 
     pub async fn sign_query<T>(&self, req: &mut Request<T>, duration: Duration) -> Result<()> {
@@ -115,7 +128,17 @@ impl GcsCore {
 
         self.signer
             .sign_query(req, duration, &cred)
-            .map_err(new_request_sign_error)
+            .map_err(new_request_sign_error)?;
+
+        // Always remove host header, let users' client to set it based on HTTP
+        // version.
+        //
+        // As discussed in <https://github.com/seanmonstar/reqwest/issues/1809>,
+        // google server could send RST_STREAM of PROTOCOL_ERROR if our request
+        // contains host header.
+        req.headers_mut().remove(HOST);
+
+        Ok(())
     }
 
     #[inline]
diff --git a/core/src/services/s3/core.rs b/core/src/services/s3/core.rs
index 065da8fa7..f775fb99a 100644
--- a/core/src/services/s3/core.rs
+++ b/core/src/services/s3/core.rs
@@ -27,6 +27,7 @@ use http::header::CACHE_CONTROL;
 use http::header::CONTENT_DISPOSITION;
 use http::header::CONTENT_LENGTH;
 use http::header::CONTENT_TYPE;
+use http::header::HOST;
 use http::header::IF_MATCH;
 use http::header::IF_NONE_MATCH;
 use http::HeaderValue;
@@ -127,7 +128,19 @@ impl S3Core {
             return Ok(());
         };
 
-        self.signer.sign(req, &cred).map_err(new_request_sign_error)
+        self.signer
+            .sign(req, &cred)
+            .map_err(new_request_sign_error)?;
+
+        // Always remove host header, let users' client to set it based on HTTP
+        // version.
+        //
+        // As discussed in <https://github.com/seanmonstar/reqwest/issues/1809>,
+        // google server could send RST_STREAM of PROTOCOL_ERROR if our request
+        // contains host header.
+        req.headers_mut().remove(HOST);
+
+        Ok(())
     }
 
     pub async fn sign_query<T>(&self, req: &mut Request<T>, duration: Duration) -> Result<()> {
@@ -139,7 +152,17 @@ impl S3Core {
 
         self.signer
             .sign_query(req, duration, &cred)
-            .map_err(new_request_sign_error)
+            .map_err(new_request_sign_error)?;
+
+        // Always remove host header, let users' client to set it based on HTTP
+        // version.
+        //
+        // As discussed in <https://github.com/seanmonstar/reqwest/issues/1809>,
+        // google server could send RST_STREAM of PROTOCOL_ERROR if our request
+        // contains host header.
+        req.headers_mut().remove(HOST);
+
+        Ok(())
     }
 
     #[inline]
diff --git a/core/src/types/list.rs b/core/src/types/list.rs
index 6e67aa430..566065a69 100644
--- a/core/src/types/list.rs
+++ b/core/src/types/list.rs
@@ -185,11 +185,12 @@ impl Iterator for BlockingLister {
 
 #[cfg(test)]
 mod tests {
-    use super::*;
-    use crate::services::Azblob;
     use futures::future;
     use futures::StreamExt;
 
+    use super::*;
+    use crate::services::Azblob;
+
     /// Inspired by <https://gist.github.com/kyle-mccarthy/1e6ae89cc34495d731b91ebf5eb5a3d9>
     ///
     /// Invalid lister should not panic nor endless loop.