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 <[email protected]> AuthorDate: Mon Aug 28 15:48:07 2023 +0800 fix(core): Make sure OpenDAL works with http2 on GCS Signed-off-by: Xuanwo <[email protected]> --- 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.
