From 517b7fc9370b58df5eed4ceffeff405d3df40389 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 26 Sep 2024 15:54:56 +0000 Subject: [PATCH] Use constants for header names (#1933) * tonic: rename header name consts and make public * tonic: use header name constants in more places --- tonic-build/src/server.rs | 2 +- tonic-health/src/generated/grpc_health_v1.rs | 2 +- .../src/generated/grpc_reflection_v1.rs | 2 +- .../src/generated/grpc_reflection_v1alpha.rs | 2 +- tonic-web/src/call.rs | 13 +++++--- tonic-web/src/lib.rs | 17 +++++----- tonic/src/codec/prost.rs | 4 +-- tonic/src/service/router.rs | 8 ++--- tonic/src/status.rs | 31 ++++++++++--------- 9 files changed, 41 insertions(+), 40 deletions(-) diff --git a/tonic-build/src/server.rs b/tonic-build/src/server.rs index 0d88018d9..3ca18260c 100644 --- a/tonic-build/src/server.rs +++ b/tonic-build/src/server.rs @@ -167,7 +167,7 @@ pub(crate) fn generate_internal( _ => Box::pin(async move { let mut response = http::Response::new(empty_body()); let headers = response.headers_mut(); - headers.insert("grpc-status", (tonic::Code::Unimplemented as i32).into()); + headers.insert(tonic::Status::GRPC_STATUS, (tonic::Code::Unimplemented as i32).into()); headers.insert(http::header::CONTENT_TYPE, tonic::metadata::GRPC_CONTENT_TYPE); Ok(response) }), diff --git a/tonic-health/src/generated/grpc_health_v1.rs b/tonic-health/src/generated/grpc_health_v1.rs index 249a5a6ff..f8aca7b38 100644 --- a/tonic-health/src/generated/grpc_health_v1.rs +++ b/tonic-health/src/generated/grpc_health_v1.rs @@ -423,7 +423,7 @@ pub mod health_server { let headers = response.headers_mut(); headers .insert( - "grpc-status", + tonic::Status::GRPC_STATUS, (tonic::Code::Unimplemented as i32).into(), ); headers diff --git a/tonic-reflection/src/generated/grpc_reflection_v1.rs b/tonic-reflection/src/generated/grpc_reflection_v1.rs index 2f454728e..198487632 100644 --- a/tonic-reflection/src/generated/grpc_reflection_v1.rs +++ b/tonic-reflection/src/generated/grpc_reflection_v1.rs @@ -425,7 +425,7 @@ pub mod server_reflection_server { let headers = response.headers_mut(); headers .insert( - "grpc-status", + tonic::Status::GRPC_STATUS, (tonic::Code::Unimplemented as i32).into(), ); headers diff --git a/tonic-reflection/src/generated/grpc_reflection_v1alpha.rs b/tonic-reflection/src/generated/grpc_reflection_v1alpha.rs index 237312987..fd94de936 100644 --- a/tonic-reflection/src/generated/grpc_reflection_v1alpha.rs +++ b/tonic-reflection/src/generated/grpc_reflection_v1alpha.rs @@ -425,7 +425,7 @@ pub mod server_reflection_server { let headers = response.headers_mut(); headers .insert( - "grpc-status", + tonic::Status::GRPC_STATUS, (tonic::Code::Unimplemented as i32).into(), ); headers diff --git a/tonic-web/src/call.rs b/tonic-web/src/call.rs index 301e1780a..98968048a 100644 --- a/tonic-web/src/call.rs +++ b/tonic-web/src/call.rs @@ -522,8 +522,11 @@ mod tests { #[test] fn decode_trailers() { let mut headers = HeaderMap::new(); - headers.insert("grpc-status", 0.into()); - headers.insert("grpc-message", "this is a message".try_into().unwrap()); + headers.insert(Status::GRPC_STATUS, 0.into()); + headers.insert( + Status::GRPC_MESSAGE, + "this is a message".try_into().unwrap(), + ); let trailers = make_trailers_frame(headers.clone()); @@ -566,7 +569,7 @@ mod tests { let trailers = decode_trailers_frame(Bytes::copy_from_slice(&buf[81..])) .unwrap() .unwrap(); - let status = trailers.get("grpc-status").unwrap(); + let status = trailers.get(Status::GRPC_STATUS).unwrap(); assert_eq!(status.to_str().unwrap(), "0") } @@ -624,8 +627,8 @@ mod tests { .unwrap(); let mut expected = HeaderMap::new(); - expected.insert("grpc-status", "0".parse().unwrap()); - expected.insert("grpc-message", "".parse().unwrap()); + expected.insert(Status::GRPC_STATUS, "0".parse().unwrap()); + expected.insert(Status::GRPC_MESSAGE, "".parse().unwrap()); expected.insert("a", "1".parse().unwrap()); expected.insert("b", "2".parse().unwrap()); diff --git a/tonic-web/src/lib.rs b/tonic-web/src/lib.rs index 79024f9a3..296ba500b 100644 --- a/tonic-web/src/lib.rs +++ b/tonic-web/src/lib.rs @@ -109,14 +109,17 @@ mod service; use http::header::HeaderName; use std::time::Duration; -use tonic::{body::BoxBody, server::NamedService}; +use tonic::{body::BoxBody, server::NamedService, Status}; use tower_http::cors::{AllowOrigin, CorsLayer}; use tower_layer::Layer; use tower_service::Service; const DEFAULT_MAX_AGE: Duration = Duration::from_secs(24 * 60 * 60); -const DEFAULT_EXPOSED_HEADERS: [&str; 3] = - ["grpc-status", "grpc-message", "grpc-status-details-bin"]; +const DEFAULT_EXPOSED_HEADERS: [HeaderName; 3] = [ + Status::GRPC_STATUS, + Status::GRPC_MESSAGE, + Status::GRPC_STATUS_DETAILS, +]; const DEFAULT_ALLOW_HEADERS: [&str; 4] = ["x-grpc-web", "content-type", "x-user-agent", "grpc-timeout"]; @@ -136,13 +139,7 @@ where .allow_origin(AllowOrigin::mirror_request()) .allow_credentials(true) .max_age(DEFAULT_MAX_AGE) - .expose_headers( - DEFAULT_EXPOSED_HEADERS - .iter() - .cloned() - .map(HeaderName::from_static) - .collect::>(), - ) + .expose_headers(DEFAULT_EXPOSED_HEADERS.iter().cloned().collect::>()) .allow_headers( DEFAULT_ALLOW_HEADERS .iter() diff --git a/tonic/src/codec/prost.rs b/tonic/src/codec/prost.rs index ec42226a7..d676f41a6 100644 --- a/tonic/src/codec/prost.rs +++ b/tonic/src/codec/prost.rs @@ -267,7 +267,7 @@ mod tests { frame .into_trailers() .expect("got trailers") - .get("grpc-status") + .get(Status::GRPC_STATUS) .expect("grpc-status header"), "11" ); @@ -304,7 +304,7 @@ mod tests { frame .into_trailers() .expect("got trailers") - .get("grpc-status") + .get(Status::GRPC_STATUS) .expect("grpc-status header"), "8" ); diff --git a/tonic/src/service/router.rs b/tonic/src/service/router.rs index 7329d00ce..70dc19136 100644 --- a/tonic/src/service/router.rs +++ b/tonic/src/service/router.rs @@ -2,8 +2,9 @@ use crate::{ body::{boxed, BoxBody}, metadata::GRPC_CONTENT_TYPE, server::NamedService, + Status, }; -use http::{HeaderName, HeaderValue, Request, Response}; +use http::{HeaderValue, Request, Response}; use std::{ convert::Infallible, fmt, @@ -124,10 +125,7 @@ impl From for Routes { async fn unimplemented() -> impl axum::response::IntoResponse { let status = http::StatusCode::OK; let headers = [ - ( - HeaderName::from_static("grpc-status"), - HeaderValue::from_static("12"), - ), + (Status::GRPC_STATUS, HeaderValue::from_static("12")), (http::header::CONTENT_TYPE, GRPC_CONTENT_TYPE), ]; (status, headers) diff --git a/tonic/src/status.rs b/tonic/src/status.rs index d79552925..2cfa5b7b3 100644 --- a/tonic/src/status.rs +++ b/tonic/src/status.rs @@ -21,10 +21,6 @@ const ENCODING_SET: &AsciiSet = &CONTROLS .add(b'{') .add(b'}'); -const GRPC_STATUS_HEADER_CODE: HeaderName = HeaderName::from_static("grpc-status"); -const GRPC_STATUS_MESSAGE_HEADER: HeaderName = HeaderName::from_static("grpc-message"); -const GRPC_STATUS_DETAILS_HEADER: HeaderName = HeaderName::from_static("grpc-status-details-bin"); - /// A gRPC status describing the result of an RPC call. /// /// Values can be created using the `new` function or one of the specialized @@ -442,10 +438,10 @@ impl Status { /// Extract a `Status` from a hyper `HeaderMap`. pub fn from_header_map(header_map: &HeaderMap) -> Option { - header_map.get(GRPC_STATUS_HEADER_CODE).map(|code| { + header_map.get(Self::GRPC_STATUS).map(|code| { let code = Code::from_bytes(code.as_ref()); let error_message = header_map - .get(GRPC_STATUS_MESSAGE_HEADER) + .get(Self::GRPC_MESSAGE) .map(|header| { percent_decode(header.as_bytes()) .decode_utf8() @@ -454,7 +450,7 @@ impl Status { .unwrap_or_else(|| Ok(String::new())); let details = header_map - .get(GRPC_STATUS_DETAILS_HEADER) + .get(Self::GRPC_STATUS_DETAILS) .map(|h| { crate::util::base64::STANDARD .decode(h.as_bytes()) @@ -464,9 +460,9 @@ impl Status { .unwrap_or_default(); let mut other_headers = header_map.clone(); - other_headers.remove(GRPC_STATUS_HEADER_CODE); - other_headers.remove(GRPC_STATUS_MESSAGE_HEADER); - other_headers.remove(GRPC_STATUS_DETAILS_HEADER); + other_headers.remove(Self::GRPC_STATUS); + other_headers.remove(Self::GRPC_MESSAGE); + other_headers.remove(Self::GRPC_STATUS_DETAILS); match error_message { Ok(message) => Status { @@ -525,7 +521,7 @@ impl Status { pub fn add_header(&self, header_map: &mut HeaderMap) -> Result<(), Self> { header_map.extend(self.metadata.clone().into_sanitized_headers()); - header_map.insert(GRPC_STATUS_HEADER_CODE, self.code.to_header_value()); + header_map.insert(Self::GRPC_STATUS, self.code.to_header_value()); if !self.message.is_empty() { let to_write = Bytes::copy_from_slice( @@ -533,7 +529,7 @@ impl Status { ); header_map.insert( - GRPC_STATUS_MESSAGE_HEADER, + Self::GRPC_MESSAGE, HeaderValue::from_maybe_shared(to_write).map_err(invalid_header_value_byte)?, ); } @@ -542,7 +538,7 @@ impl Status { let details = crate::util::base64::STANDARD_NO_PAD.encode(&self.details[..]); header_map.insert( - GRPC_STATUS_DETAILS_HEADER, + Self::GRPC_STATUS_DETAILS, HeaderValue::from_maybe_shared(details).map_err(invalid_header_value_byte)?, ); } @@ -591,6 +587,13 @@ impl Status { self.add_header(response.headers_mut()).unwrap(); response } + + #[doc(hidden)] + pub const GRPC_STATUS: HeaderName = HeaderName::from_static("grpc-status"); + #[doc(hidden)] + pub const GRPC_MESSAGE: HeaderName = HeaderName::from_static("grpc-message"); + #[doc(hidden)] + pub const GRPC_STATUS_DETAILS: HeaderName = HeaderName::from_static("grpc-status-details-bin"); } fn find_status_in_source_chain(err: &(dyn Error + 'static)) -> Option { @@ -1005,7 +1008,7 @@ mod tests { let b64_details = crate::util::base64::STANDARD_NO_PAD.encode(DETAILS); - assert_eq!(header_map[super::GRPC_STATUS_DETAILS_HEADER], b64_details); + assert_eq!(header_map[Status::GRPC_STATUS_DETAILS], b64_details); let status = Status::from_header_map(&header_map).unwrap();