fix(tonic): Preserve HTTP method in interceptor (#912)

Co-authored-by: Joe Dahlquist <[email protected]>

tonic/src/client/grpc.rs

@@ -248,7 +248,12 @@ impl<T> Grpc<T> {
             })
             .map(BoxBody::new);
 
-        let mut request = request.into_http(uri, SanitizeHeaders::Yes);
+        let mut request = request.into_http(
+            uri,
+            http::Method::POST,
+            http::Version::HTTP_2,
+            SanitizeHeaders::Yes,
+        );
 
         // Add the gRPC related HTTP headers
         request

tonic/src/request.rs

@@ -169,12 +169,14 @@ impl<T> Request<T> {
     pub(crate) fn into_http(
         self,
         uri: http::Uri,
+        method: http::Method,
+        version: http::Version,
         sanitize_headers: SanitizeHeaders,
     ) -> http::Request<T> {
         let mut request = http::Request::new(self.message);
 
-        *request.version_mut() = http::Version::HTTP_2;
+        *request.version_mut() = version;
-        *request.method_mut() = http::Method::POST;
+        *request.method_mut() = method;
         *request.uri_mut() = uri;
         *request.headers_mut() = match sanitize_headers {
             SanitizeHeaders::Yes => self.metadata.into_sanitized_headers(),
@@ -441,7 +443,12 @@ mod tests {
                 .insert(*header, MetadataValue::from_static("invalid"));
         }
 
-        let http_request = r.into_http(Uri::default(), SanitizeHeaders::Yes);
+        let http_request = r.into_http(
+            Uri::default(),
+            http::Method::POST,
+            http::Version::HTTP_2,
+            SanitizeHeaders::Yes,
+        );
         assert!(http_request.headers().is_empty());
     }
 

tonic/src/service/interceptor.rs

@@ -162,7 +162,14 @@ where
     }
 
     fn call(&mut self, req: http::Request<ReqBody>) -> Self::Future {
+        // It is bad practice to modify the body (i.e. Message) of the request via an interceptor.
+        // To avoid exposing the body of the request to the interceptor function, we first remove it
+        // here, allow the interceptor to modify the metadata and extensions, and then recreate the
+        // HTTP request with the body. Tonic requests do not preserve the URI, HTTP version, and
+        // HTTP method of the HTTP request, so we extract them here and then add them back in below.
         let uri = req.uri().clone();
+        let method = req.method().clone();
+        let version = req.version().clone();
         let req = crate::Request::from_http(req);
         let (metadata, extensions, msg) = req.into_parts();
 
@@ -173,7 +180,7 @@ where
             Ok(req) => {
                 let (metadata, extensions, _) = req.into_parts();
                 let req = crate::Request::from_parts(metadata, extensions, msg);
-                let req = req.into_http(uri, SanitizeHeaders::No);
+                let req = req.into_http(uri, method, version, SanitizeHeaders::No);
                 ResponseFuture::future(self.inner.call(req))
             }
             Err(status) => ResponseFuture::status(status),
@@ -333,4 +340,22 @@ mod tests {
         assert_eq!(expected.version(), response.version());
         assert_eq!(expected.headers(), response.headers());
     }
+
+    #[tokio::test]
+    async fn doesnt_change_http_method() {
+        let svc = tower::service_fn(|request: http::Request<hyper::Body>| async move {
+            assert_eq!(request.method(), http::Method::OPTIONS);
+
+            Ok::<_, hyper::Error>(hyper::Response::new(hyper::Body::empty()))
+        });
+
+        let svc = InterceptedService::new(svc, |request: crate::Request<()>| Ok(request));
+
+        let request = http::Request::builder()
+            .method(http::Method::OPTIONS)
+            .body(hyper::Body::empty())
+            .unwrap();
+
+        svc.oneshot(request).await.unwrap();
+    }
 }