diff --git a/Cargo.toml b/Cargo.toml index 809df0e..1b3e662 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,11 @@ bytes = "1" # gRPC client (to upstream service) tonic = "0.14" tonic-health = "0.14" +# Canonical google.rpc.Status / error_details descriptors (FILE_DESCRIPTOR_SET) +# and the Status message used to decode `grpc-status-details-bin`, so REST error +# bodies can render typed details even when the product descriptors do not +# import google/rpc/*. +tonic-types = "0.14" prost = "0.14" # Async runtime @@ -147,3 +152,6 @@ ed25519-dalek = "3" rand = "0.10" chrono = "0.4" tokio-stream = "0.1" +# Compiles the in-memory test protos (google.api.http routes) of the +# error-details integration test without a protoc binary. +protox = "0.9" diff --git a/README.md b/README.md index 6338373..e8cbc3e 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ Works with **any** gRPC service via proto descriptor files. No code generation, - **Auto-generated OpenAPI** documentation from proto messages, served at `/openapi.json` - **Server-streaming** RPC → NDJSON by default, or Server-Sent Events via `Accept: text/event-stream` negotiation - **gRPC → HTTP status mapping** following the standard `google.rpc.Code` table +- **Typed error details**: the upstream's `google.rpc.Status` details (`ErrorInfo`, `BadRequest`, `RetryInfo`, ...) reach the HTTP client as ProtoJSON, switchable globally and per route (see [Error responses](#error-responses)) - **Header forwarding** from HTTP requests to gRPC metadata (configurable allow-list) - **Context propagation**: W3C trace-context (`traceparent` forwarded or synthesized) and client deadlines (`grpc-timeout`) carried across the REST↔gRPC boundary - **Path aliasing** for route remapping (e.g. `/oauth2/*` → `/v1/oauth2/*`) @@ -106,6 +107,24 @@ streaming: # SSE keep-alive interval (seconds). Comment frames keep idle streams alive # through load balancers / nginx read timeouts. Default: 15. sse_keep_alive_secs: 15 + # Wrap every NDJSON line as {"result": ...} / {"error": ...} (see "Error + # responses"). Default: false. + ndjson_envelope: false + +# Optional: google.rpc.Status details in error bodies (see "Error responses"). +# On everywhere by default. `opaque` forwards details of types no descriptor +# describes as `opaqueDetails` instead of withholding them (off by default). +# Rules are checked in order; for each switch a rule sets, the first rule whose +# pattern matches the mounted route decides; `*` stays within one path segment +# (a path parameter counts as one), `**` spans segments. +error_details: + enabled: true + opaque: false + routes: + - pattern: "/v1/internal/**" + enabled: false + - pattern: "/v1/partner/**" + opaque: true # Rate limiting (Shield) # @@ -245,18 +264,174 @@ there is no boundary burst on top of this lag. See the `shield:` block under [Configuration](#configuration) for the full schema. +## Error responses + +A failed gRPC call becomes a JSON body with the status of the gRPC → HTTP +mapping (`INVALID_ARGUMENT` → 400, `NOT_FOUND` → 404, ...): + +```json +{ + "error": "INVALID_ARGUMENT", + "code": 3, + "message": "invalid email", + "details": [ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "EMAIL_TAKEN", + "domain": "identity.example.com", + "metadata": { "email": "a@b.c" } + }, + { + "@type": "type.googleapis.com/google.rpc.BadRequest", + "fieldViolations": [{ "field": "email", "description": "already registered" }] + } + ] +} +``` + +- `code`, `message` and `details` follow `google.rpc.Status`; `error` is the + code's name. A client that parses the body as `google.rpc.Status` with a + strict ProtoJSON parser must let it ignore unknown fields. +- `details` is the upstream's `grpc-status-details-bin` trailer, one entry per + `Any`, in [ProtoJSON](https://protobuf.dev/programming-guides/json/#any) + form: `@type` plus the message fields, or `@type` plus `value` for a + well-known type with a special JSON representation (`google.protobuf.Duration` + as `"1.500s"`). Types resolve from the service's descriptors first, then from + the canonical `google/rpc/status.proto` and `error_details.proto`, which are + always available. An upstream that sends no trailer yields `"details": []`. +- `google.rpc.DebugInfo` is never forwarded: it carries stack traces and server + internals meant for the service's operators. It is removed wherever the + proxy knows the schema; a detail whose type is in neither descriptor set + cannot be checked, so it is withheld too unless the opaque-detail extension + below is switched on. +- Details are on for every route. They can be switched off globally or per + route (see below); on such a route the `details` and `opaqueDetails` keys are + absent and the body is `{"error", "code", "message"}`. +- Errors the proxy raises itself on a transcoded route use the same body: a + request that cannot be mapped onto the RPC (`INVALID_ARGUMENT`, 400), an + upstream that is not reachable (`UNAVAILABLE`, 503), a response that cannot + be serialized (`INTERNAL`, 500). Their `details` is empty. +- A broken upstream error status is never passed on in part or reinterpreted: + a trailer that is not a `google.rpc.Status` or disagrees with `grpc-status` / + `grpc-message`, a type URL without a `/` or whose last segment is not a + protobuf full name, or a detail of a known type whose bytes do not decode or + whose value has no valid JSON form (a `Duration` beyond its range), turns the + whole error into + `{"error": "INTERNAL", "code": 13, "message": "upstream returned a malformed error status", "details": []}` + (500, or the terminal frame of a started stream). The cause is logged by the + proxy and not sent to the client. With details switched off for a route the + trailer is not read, so this does not apply there. + +**Opaque-detail extension.** ProtoJSON cannot represent an `Any` whose type is +unknown to the writer, so a detail whose type is in neither descriptor set has +no place in `details`. By default such a detail is withheld: its bytes cannot +be inspected, and a `DebugInfo` in one of its fields would otherwise reach the +client. When switched on (`opaque: true`, globally or in a route rule, or +`ErrorDetailsPolicy::with_opaque_details` / `opaque_route`), structured-proxy +keeps it in a separate `opaqueDetails` array instead. Switch it on only for +upstreams trusted not to nest a `DebugInfo` in types the proxy has no +descriptor for. The array is structured-proxy's own extension and **not** part +of ProtoJSON or `google.rpc.Status`: + +```json +{ + "error": "FAILED_PRECONDITION", + "code": 9, + "message": "quota exhausted", + "details": [ + { "@type": "type.googleapis.com/google.rpc.ErrorInfo", "reason": "QUOTA", "domain": "acme.example.com" } + ], + "opaqueDetails": [ + { "index": 1, "typeUrl": "type.googleapis.com/acme.v1.QuotaTicket", "bytes": "CgNULTE=" } + ] +} +``` + +- `typeUrl` is the original type URL and `bytes` the standard base64 of the + original bytes. An entry has no `@type` and never appears in `details`, so + `details` stays an array of ProtoJSON `Any` and nothing in the extension can + be taken for a message of the named type. +- `index` is the entry's position among the forwarded details: merging + `details` and `opaqueDetails` by position restores the upstream's order. +- `opaqueDetails` appears only when at least one detail went there. A client + that knows the type base64-decodes `bytes` and parses the protobuf itself; a + client that does not handle the extension ignores the key. +- Only an unknown type goes there. A detail of a known type that fails to + decode is a broken upstream status (see above), never an opaque entry. + +**Errors in server-streaming responses.** An upstream that rejects the call +outright (a gRPC trailers-only response, with no response headers or messages) +gets the mapped HTTP status and the body above. Once the upstream has accepted +the call, the proxy answers `200` and starts the stream right away, so that +headers and SSE keep-alives are not held back waiting for the first message. +Any later failure, including one that arrives before the first message, is then +delivered as a terminal frame whose payload is exactly that body, after which +the stream ends and no further data follows: + +- **NDJSON**: the last line, framed by an extra + `"@type": "type.googleapis.com/google.rpc.Status"` next to the error body. A + data line is the ProtoJSON of a response message, which has a top-level + `@type` only when the RPC streams `google.protobuf.Any`, while `Struct`, + `Value` and `ListValue` messages can carry any key at all. For those RPCs no + in-band marker is collision-free: set `streaming.ndjson_envelope: true` (or + `ProxyServer::with_ndjson_envelope(true)`) and every line is wrapped instead, + `{"result": }` for data and `{"error": }` for the + terminal error, the grpc-gateway stream shape. The envelope changes data + lines too, so it is off by default. +- **SSE**: one event with type `stream-error` (listen with + `addEventListener("stream-error", ...)`), distinct from the `EventSource` + `onerror` that fires on transport failures. The event type is the framing, + so the event data is exactly the error body, without the NDJSON marker. + +The same applies to a message the proxy cannot serialize mid-stream: the +stream ends with an `INTERNAL` terminal frame. + +This is the HTTP/JSON transcoding format. It is not the Connect protocol's error +format, and it is not an OAuth 2.0 token endpoint error body (RFC 6749 §5.2). + +**Switching details off.** In the config file, `error_details:` (see +[Configuration](#configuration)) is read by the standalone binary and by +`ProxyServer::from_yaml_str` / `ProxyServer::from_file`; it is not part of +`ProxyConfig`, so `ProxyConfig::from_yaml_str` alone ignores it. Both log a +warning for a top-level or `streaming:` key no setting reads, so a misspelled +`error_detail:` or `ndjson_envelop:` shows up at startup instead of silently +leaving the default in force. An embedding +service can choose in code with `ProxyServer::with_error_details`. Overrides are +checked in the order they are added; for each switch (`enabled`, `opaque`) the +first rule whose pattern matches the mounted route and that sets the switch +decides, otherwise the global value; `*` stays within one path segment (a path +parameter counts as one) and `**` spans segments. A config rule that sets +neither switch is rejected: + +```rust +use structured_proxy::transcode::error::ErrorDetailsPolicy; +use structured_proxy::{config::ProxyConfig, ProxyServer}; + +# fn build(config: ProxyConfig) -> Result { +// Details everywhere except the internal admin surface. +let policy = ErrorDetailsPolicy::default().route("/v1/admin/**", false)?; +// Or: off everywhere except a public sub-route. +// let policy = ErrorDetailsPolicy::disabled().route("/v1/public/**", true)?; +// Unknown detail types as `opaqueDetails` for one trusted partner surface. +let policy = policy.opaque_route("/v1/partner/**", true)?; +Ok(ProxyServer::from_config(config).with_error_details(policy)) +# } +``` + ## Library Usage ```rust use std::path::Path; -use structured_proxy::{config::ProxyConfig, ProxyServer}; +use structured_proxy::ProxyServer; #[tokio::main] async fn main() -> anyhow::Result<()> { - let config = ProxyConfig::from_file(Path::new("my-service.yaml"))?; + // Reads the whole config file, including `error_details` and + // `streaming.ndjson_envelope`, which live outside `ProxyConfig`. + let server = ProxyServer::from_file(Path::new("my-service.yaml"))?; // Run the proxy on the configured listen address. - ProxyServer::from_config(config).serve().await?; + server.serve().await?; Ok(()) } ``` @@ -324,6 +499,8 @@ The hooks are: discovery. - **`with_extra_routes`** — registers extra stateless routes through a framework-agnostic adapter (request parts in, response parts out). +- **`with_error_details`** — chooses which transcoded routes return the + upstream's `google.rpc.Status` details (see [Error responses](#error-responses)). ## JWT verification diff --git a/src/config.rs b/src/config.rs index 76e3b9d..fc3f4c0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -123,6 +123,161 @@ impl Default for StreamingConfig { } } +/// Transcoding settings read from the same YAML document as [`ProxyConfig`] by +/// [`ProxyServer::from_yaml_str`](crate::ProxyServer::from_yaml_str) and +/// [`ProxyServer::from_file`](crate::ProxyServer::from_file): +/// +/// ```yaml +/// error_details: +/// enabled: true +/// opaque: false +/// routes: +/// - pattern: "/v1/admin/**" +/// enabled: false +/// - pattern: "/v1/partner/**" +/// opaque: true +/// streaming: +/// ndjson_envelope: true +/// ``` +/// +/// They live outside [`ProxyConfig`] so that embedders who build it as a +/// struct literal are not affected. +#[derive(Debug, Default, Deserialize)] +pub(crate) struct TranscodeFileConfig { + #[serde(default)] + error_details: Option, + #[serde(default)] + streaming: StreamingFileConfig, +} + +/// `error_details:`. A typo here would silently change what clients learn +/// about server-side failures, so unknown keys are rejected. +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct ErrorDetailsFileConfig { + /// Default for routes no rule matches. + #[serde(default = "default_true")] + enabled: bool, + /// Default for routes no rule matches: whether details of types no + /// descriptor describes go to `opaqueDetails` instead of being withheld. + #[serde(default)] + opaque: bool, + /// Per-route overrides; for each switch, the first matching rule that + /// sets it wins. + #[serde(default)] + routes: Vec, +} + +/// A rule sets `enabled`, `opaque` or both; one setting neither is rejected. +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct ErrorDetailsRouteFileConfig { + pattern: String, + #[serde(default)] + enabled: Option, + #[serde(default)] + opaque: Option, +} + +/// The `streaming:` keys [`StreamingConfig`] does not hold. Both read the same +/// section, so neither can reject the other's keys; [`unknown_config_keys`] +/// reports the keys neither reads. +#[derive(Debug, Default, Deserialize)] +struct StreamingFileConfig { + #[serde(default)] + ndjson_envelope: bool, +} + +/// Every top-level key a config file may use: the [`ProxyConfig`] fields plus +/// the transcoding settings kept outside it. +pub(crate) const KNOWN_TOP_LEVEL_KEYS: &[&str] = &[ + "upstream", + "descriptors", + "listen", + "service", + "aliases", + "openapi", + "auth", + "shield", + "oidc_discovery", + "health", + "metrics", + "maintenance", + "cors", + "logging", + "metrics_classes", + "forwarded_headers", + "streaming", + "error_details", +]; + +/// Every `streaming:` key: the [`StreamingConfig`] fields plus the ones +/// [`StreamingFileConfig`] reads. +pub(crate) const KNOWN_STREAMING_KEYS: &[&str] = &["sse_keep_alive_secs", "ndjson_envelope"]; + +/// The keys of `yaml` that no setting reads, typically typos (`error_detail:` +/// for `error_details:`) that would otherwise leave a default silently in +/// force. They are reported, not rejected, so a file that loaded before keeps +/// loading. +pub(crate) fn unknown_config_keys(yaml: &str) -> Vec { + let Ok(serde_yaml::Value::Mapping(map)) = serde_yaml::from_str(yaml) else { + return Vec::new(); + }; + let mut unknown: Vec = map + .keys() + .filter_map(|key| key.as_str()) + .filter(|key| !KNOWN_TOP_LEVEL_KEYS.contains(key)) + .map(str::to_owned) + .collect(); + // `streaming:` is split between StreamingConfig and StreamingFileConfig, + // so neither can deny unknown fields; its keys are checked here instead. + if let Some(serde_yaml::Value::Mapping(streaming)) = map.get("streaming") { + unknown.extend( + streaming + .keys() + .filter_map(|key| key.as_str()) + .filter(|key| !KNOWN_STREAMING_KEYS.contains(key)) + .map(|key| format!("streaming.{key}")), + ); + } + unknown +} + +impl TranscodeFileConfig { + /// Compile into the options the transcoded routes are built with. + /// + /// # Errors + /// + /// An `error_details` route pattern that is relative or not a valid glob, + /// or a route rule that sets neither `enabled` nor `opaque`. + pub(crate) fn options(&self) -> Result { + use crate::transcode::error::ErrorDetailsPolicy; + let mut options = crate::transcode::TranscodeOptions::default() + .with_ndjson_envelope(self.streaming.ndjson_envelope); + if let Some(cfg) = &self.error_details { + let base = if cfg.enabled { + ErrorDetailsPolicy::default() + } else { + ErrorDetailsPolicy::disabled() + } + .with_opaque_details(cfg.opaque); + let policy = cfg.routes.iter().try_fold(base, |policy, rule| { + // A rule without a switch would match routes and change + // nothing, which is a config mistake, not an intent. + if rule.enabled.is_none() && rule.opaque.is_none() { + return Err(format!( + "error details route {:?} sets neither enabled nor opaque", + rule.pattern + )); + } + policy.rule(&rule.pattern, rule.enabled, rule.opaque) + })?; + options = options.with_error_details(policy); + } + Ok(options) + } +} + /// Upstream gRPC service configuration. #[derive(Debug, Clone, Deserialize)] pub struct UpstreamConfig { @@ -865,358 +1020,4 @@ impl ProxyConfig { } #[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_minimal_config_deserialize() { - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -"#; - let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.upstream.default, "grpc://localhost:4180"); - assert_eq!(config.listen.http, "0.0.0.0:8080"); - assert_eq!(config.service.name, "structured-proxy"); - assert_eq!(config.streaming.sse_keep_alive_secs, 15); - assert!(config.descriptors.is_empty()); - assert!(config.auth.is_none()); - assert!(config.shield.is_none()); - } - - #[test] - fn health_and_metrics_defaults_and_overrides() { - // Defaults: enabled, conventional paths. - let min: ProxyConfig = - serde_yaml::from_str("upstream:\n default: \"grpc://x:1\"\n").unwrap(); - assert!(min.health.enabled); - assert_eq!(min.health.path, "/health"); - assert_eq!(min.health.ready_path, "/health/ready"); - assert!(min.metrics.enabled); - assert_eq!(min.metrics.path, "/metrics"); - - // Overrides apply; unspecified sub-paths keep their defaults. - let yaml = r#" -upstream: - default: "grpc://x:1" -health: - path: "/internal/health" -metrics: - enabled: false - path: "/internal/metrics" -"#; - let cfg: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(cfg.health.path, "/internal/health"); - // live_path was not overridden, so it stays at the default. - assert_eq!(cfg.health.live_path, "/health/live"); - assert!(!cfg.metrics.enabled); - assert_eq!(cfg.metrics.path, "/internal/metrics"); - } - - #[test] - fn duplicate_probe_paths_are_rejected() { - // health.path set to the default live_path collides on a single GET - // route; reject at load instead of panicking in the router. - let yaml = r#" -upstream: - default: "grpc://x:1" -health: - path: "/health/live" -"#; - let err = ProxyConfig::from_yaml_str(yaml).unwrap_err(); - assert!(err.to_string().contains("duplicate endpoint path")); - - // A health path colliding with the metrics path is also rejected. - let yaml2 = r#" -upstream: - default: "grpc://x:1" -metrics: - path: "/health" -"#; - let err2 = ProxyConfig::from_yaml_str(yaml2).unwrap_err(); - assert!(err2.to_string().contains("duplicate endpoint path")); - - // Disabling a group frees its paths from the collision check. - let yaml3 = r#" -upstream: - default: "grpc://x:1" -health: - enabled: false - path: "/metrics" -"#; - assert!(ProxyConfig::from_yaml_str(yaml3).is_ok()); - } - - #[test] - fn malformed_edge_path_is_rejected() { - // A path without a leading '/' would make axum reject the route at - // construction; catch it at config load with a clear message. - let yaml = r#" -upstream: - default: "grpc://x:1" -health: - path: "health" -"#; - let err = ProxyConfig::from_yaml_str(yaml).unwrap_err(); - assert!(err.to_string().contains("must start with '/'")); - } - - #[test] - fn test_zero_sse_keep_alive_is_rejected() { - // A zero keep-alive would make axum's SSE timer fire continuously - // instead of acting as a periodic heartbeat — reject it at load time. - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -streaming: - sse_keep_alive_secs: 0 -"#; - let err = ProxyConfig::from_yaml_str(yaml).unwrap_err(); - assert!(err.to_string().contains("sse_keep_alive_secs")); - } - - #[test] - fn test_full_config_deserialize() { - let yaml = r#" -upstream: - default: "grpc://sid-identity:4180" - -descriptors: - - file: "/etc/proxy/sid.descriptor.bin" - -listen: - http: "0.0.0.0:9090" - -service: - name: "sid-proxy" - -aliases: - - from: "/oauth2/{path}" - to: "/v1/oauth2/{path}" - -auth: - mode: "jwt" - jwt: - issuer: "https://auth.example.com" - public_key_pem_file: "/etc/proxy/signing.pub" - claims_headers: - sub: "x-forwarded-user" - acr: "x-sid-auth-level" - forward_auth: - enabled: true - path: "/auth/verify" - policies: - - path: "/v1/admin/**" - require_auth: true - required_roles: ["admin"] - - path: "/v1/public/**" - require_auth: false - authz: - enabled: true - endpoint: "http://opa:9191" # Envoy ext_authz server (gRPC) - timeout_ms: 200 - failure_mode_allow: false # fail closed: deny if authz is unreachable - -shield: - enabled: true - profiles: - auth: { rate: "20/min", burst: 5 } - default: { rate: "100/min" } - premium: { rate: "1000/min", burst: 50 } - default_profile: "default" - jwt_limits: - tier_claim: "ratelimit_tier" - rules: - - pattern: "/v1/auth/**" - key: { type: ip } - profile: "auth" - - pattern: "/v1/**" - key: { type: jwt_claim, claim: "sub" } - trusted_proxies: ["10.0.0.0/8"] - -oidc_discovery: - enabled: true - issuer: "https://auth.example.com" - -maintenance: - enabled: false - exempt_paths: - - "/health/**" - - "/.well-known/**" - -cors: - origins: - - "https://app.example.com" - -metrics_classes: - - pattern: "/v1/auth/**" - class: "auth" - - pattern: "/v1/admin/**" - class: "admin" - -forwarded_headers: - - "authorization" - - "dpop" - - "x-request-id" -"#; - let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.upstream.default, "grpc://sid-identity:4180"); - assert_eq!(config.listen.http, "0.0.0.0:9090"); - assert_eq!(config.service.name, "sid-proxy"); - assert_eq!(config.aliases.len(), 1); - assert!(config.auth.is_some()); - let authz = config.auth.as_ref().unwrap().authz.as_ref().unwrap(); - assert!(authz.enabled); - assert_eq!(authz.endpoint, "http://opa:9191"); - assert_eq!(authz.timeout_ms, 200); - assert!(!authz.failure_mode_allow); - assert!(config.shield.is_some()); - assert!(config.oidc_discovery.is_some()); - assert_eq!(config.cors.origins.len(), 1); - assert_eq!(config.metrics_classes.len(), 2); - assert_eq!(config.forwarded_headers.len(), 3); - } - - #[test] - fn authz_disabled_without_endpoint_parses() { - // A disabled authz block need not supply an endpoint. - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -descriptors: - - file: "/x.bin" -auth: - mode: "jwt" - authz: - enabled: false -"#; - let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - let authz = config.auth.unwrap().authz.unwrap(); - assert!(!authz.enabled); - assert_eq!(authz.endpoint, ""); - } - - #[test] - fn test_descriptor_source_file() { - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -descriptors: - - file: "/etc/proxy/service.descriptor.bin" -"#; - let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - assert_eq!(config.descriptors.len(), 1); - match &config.descriptors[0] { - DescriptorSource::File { file } => { - assert_eq!(file.to_str().unwrap(), "/etc/proxy/service.descriptor.bin"); - } - _ => panic!("expected File descriptor source"), - } - } - - #[test] - fn test_descriptor_source_reflection() { - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -descriptors: - - reflection: "grpc://localhost:4180" -"#; - let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - match &config.descriptors[0] { - DescriptorSource::Reflection { reflection } => { - assert_eq!(reflection, "grpc://localhost:4180"); - } - _ => panic!("expected Reflection descriptor source"), - } - } - - #[test] - fn test_parse_rate() { - assert_eq!(ProxyConfig::parse_rate("20/min"), Some(20)); - assert_eq!(ProxyConfig::parse_rate("100/min"), Some(100)); - assert_eq!(ProxyConfig::parse_rate("5/min"), Some(5)); - assert_eq!(ProxyConfig::parse_rate("invalid"), None); - } - - #[test] - fn shield_rejects_unknown_field() { - // A typo in a shield-config field (here `profil` for `profile`) must be a - // hard error, not silently ignored: a misspelled security-control key - // would otherwise leave the intended limit unapplied. `deny_unknown_fields` - // on the shield structs turns the typo into a startup failure. - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -shield: - enabled: true - profiles: - auth: { rate: "20/min", burst: 5 } - rules: - - pattern: "/v1/**" - key: { type: ip } - profil: "auth" -"#; - let err = serde_yaml::from_str::(yaml); - assert!(err.is_err(), "unknown shield field must be rejected"); - } - - #[test] - fn shield_rejects_unknown_field_in_rule_key() { - // A stray field inside a rule key (here `name` on an `ip` key, a copy-edit - // leftover) must be a hard error. Silently ignoring it would keep the rule - // IP-keyed instead of the intended per-header limit, weakening the control. - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -shield: - enabled: true - profiles: - auth: { rate: "20/min", burst: 5 } - rules: - - pattern: "/v1/**" - key: { type: ip, name: x-api-key } - profile: "auth" -"#; - let err = serde_yaml::from_str::(yaml); - assert!(err.is_err(), "unknown field in a rule key must be rejected"); - } - - #[test] - fn test_openapi_config_deserialize() { - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -openapi: - enabled: true - path: "/api/openapi.json" - docs_path: "/api/docs" - title: "Test API" - version: "2.0.0" -"#; - let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - let openapi = config.openapi.unwrap(); - assert!(openapi.enabled); - assert_eq!(openapi.path, "/api/openapi.json"); - assert_eq!(openapi.docs_path, "/api/docs"); - assert_eq!(openapi.title.unwrap(), "Test API"); - assert_eq!(openapi.version.unwrap(), "2.0.0"); - } - - #[test] - fn test_openapi_config_defaults() { - let yaml = r#" -upstream: - default: "grpc://localhost:4180" -openapi: - enabled: true -"#; - let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); - let openapi = config.openapi.unwrap(); - assert!(openapi.enabled); - assert_eq!(openapi.path, "/openapi.json"); - assert_eq!(openapi.docs_path, "/docs"); - assert!(openapi.title.is_none()); - assert!(openapi.version.is_none()); - } -} +mod tests; diff --git a/src/config/tests.rs b/src/config/tests.rs new file mode 100644 index 0000000..8cdf264 --- /dev/null +++ b/src/config/tests.rs @@ -0,0 +1,535 @@ +use super::*; + +#[test] +fn test_minimal_config_deserialize() { + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +"#; + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.upstream.default, "grpc://localhost:4180"); + assert_eq!(config.listen.http, "0.0.0.0:8080"); + assert_eq!(config.service.name, "structured-proxy"); + assert_eq!(config.streaming.sse_keep_alive_secs, 15); + assert!(config.descriptors.is_empty()); + assert!(config.auth.is_none()); + assert!(config.shield.is_none()); +} + +#[test] +fn health_and_metrics_defaults_and_overrides() { + // Defaults: enabled, conventional paths. + let min: ProxyConfig = serde_yaml::from_str("upstream:\n default: \"grpc://x:1\"\n").unwrap(); + assert!(min.health.enabled); + assert_eq!(min.health.path, "/health"); + assert_eq!(min.health.ready_path, "/health/ready"); + assert!(min.metrics.enabled); + assert_eq!(min.metrics.path, "/metrics"); + + // Overrides apply; unspecified sub-paths keep their defaults. + let yaml = r#" +upstream: + default: "grpc://x:1" +health: + path: "/internal/health" +metrics: + enabled: false + path: "/internal/metrics" +"#; + let cfg: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(cfg.health.path, "/internal/health"); + // live_path was not overridden, so it stays at the default. + assert_eq!(cfg.health.live_path, "/health/live"); + assert!(!cfg.metrics.enabled); + assert_eq!(cfg.metrics.path, "/internal/metrics"); +} + +#[test] +fn duplicate_probe_paths_are_rejected() { + // health.path set to the default live_path collides on a single GET + // route; reject at load instead of panicking in the router. + let yaml = r#" +upstream: + default: "grpc://x:1" +health: + path: "/health/live" +"#; + let err = ProxyConfig::from_yaml_str(yaml).unwrap_err(); + assert!(err.to_string().contains("duplicate endpoint path")); + + // A health path colliding with the metrics path is also rejected. + let yaml2 = r#" +upstream: + default: "grpc://x:1" +metrics: + path: "/health" +"#; + let err2 = ProxyConfig::from_yaml_str(yaml2).unwrap_err(); + assert!(err2.to_string().contains("duplicate endpoint path")); + + // Disabling a group frees its paths from the collision check. + let yaml3 = r#" +upstream: + default: "grpc://x:1" +health: + enabled: false + path: "/metrics" +"#; + assert!(ProxyConfig::from_yaml_str(yaml3).is_ok()); +} + +#[test] +fn malformed_edge_path_is_rejected() { + // A path without a leading '/' would make axum reject the route at + // construction; catch it at config load with a clear message. + let yaml = r#" +upstream: + default: "grpc://x:1" +health: + path: "health" +"#; + let err = ProxyConfig::from_yaml_str(yaml).unwrap_err(); + assert!(err.to_string().contains("must start with '/'")); +} + +#[test] +fn test_zero_sse_keep_alive_is_rejected() { + // A zero keep-alive would make axum's SSE timer fire continuously + // instead of acting as a periodic heartbeat — reject it at load time. + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +streaming: + sse_keep_alive_secs: 0 +"#; + let err = ProxyConfig::from_yaml_str(yaml).unwrap_err(); + assert!(err.to_string().contains("sse_keep_alive_secs")); +} + +#[test] +fn test_full_config_deserialize() { + let yaml = r#" +upstream: + default: "grpc://sid-identity:4180" + +descriptors: + - file: "/etc/proxy/sid.descriptor.bin" + +listen: + http: "0.0.0.0:9090" + +service: + name: "sid-proxy" + +aliases: + - from: "/oauth2/{path}" + to: "/v1/oauth2/{path}" + +auth: + mode: "jwt" + jwt: + issuer: "https://auth.example.com" + public_key_pem_file: "/etc/proxy/signing.pub" + claims_headers: + sub: "x-forwarded-user" + acr: "x-sid-auth-level" + forward_auth: + enabled: true + path: "/auth/verify" + policies: + - path: "/v1/admin/**" + require_auth: true + required_roles: ["admin"] + - path: "/v1/public/**" + require_auth: false + authz: + enabled: true + endpoint: "http://opa:9191" # Envoy ext_authz server (gRPC) + timeout_ms: 200 + failure_mode_allow: false # fail closed: deny if authz is unreachable + +shield: + enabled: true + profiles: + auth: { rate: "20/min", burst: 5 } + default: { rate: "100/min" } + premium: { rate: "1000/min", burst: 50 } + default_profile: "default" + jwt_limits: + tier_claim: "ratelimit_tier" + rules: + - pattern: "/v1/auth/**" + key: { type: ip } + profile: "auth" + - pattern: "/v1/**" + key: { type: jwt_claim, claim: "sub" } + trusted_proxies: ["10.0.0.0/8"] + +oidc_discovery: + enabled: true + issuer: "https://auth.example.com" + +maintenance: + enabled: false + exempt_paths: + - "/health/**" + - "/.well-known/**" + +cors: + origins: + - "https://app.example.com" + +metrics_classes: + - pattern: "/v1/auth/**" + class: "auth" + - pattern: "/v1/admin/**" + class: "admin" + +forwarded_headers: + - "authorization" + - "dpop" + - "x-request-id" +"#; + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.upstream.default, "grpc://sid-identity:4180"); + assert_eq!(config.listen.http, "0.0.0.0:9090"); + assert_eq!(config.service.name, "sid-proxy"); + assert_eq!(config.aliases.len(), 1); + assert!(config.auth.is_some()); + let authz = config.auth.as_ref().unwrap().authz.as_ref().unwrap(); + assert!(authz.enabled); + assert_eq!(authz.endpoint, "http://opa:9191"); + assert_eq!(authz.timeout_ms, 200); + assert!(!authz.failure_mode_allow); + assert!(config.shield.is_some()); + assert!(config.oidc_discovery.is_some()); + assert_eq!(config.cors.origins.len(), 1); + assert_eq!(config.metrics_classes.len(), 2); + assert_eq!(config.forwarded_headers.len(), 3); +} + +#[test] +fn authz_disabled_without_endpoint_parses() { + // A disabled authz block need not supply an endpoint. + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +descriptors: + - file: "/x.bin" +auth: + mode: "jwt" + authz: + enabled: false +"#; + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + let authz = config.auth.unwrap().authz.unwrap(); + assert!(!authz.enabled); + assert_eq!(authz.endpoint, ""); +} + +#[test] +fn test_descriptor_source_file() { + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +descriptors: + - file: "/etc/proxy/service.descriptor.bin" +"#; + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.descriptors.len(), 1); + match &config.descriptors[0] { + DescriptorSource::File { file } => { + assert_eq!(file.to_str().unwrap(), "/etc/proxy/service.descriptor.bin"); + } + _ => panic!("expected File descriptor source"), + } +} + +#[test] +fn test_descriptor_source_reflection() { + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +descriptors: + - reflection: "grpc://localhost:4180" +"#; + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + match &config.descriptors[0] { + DescriptorSource::Reflection { reflection } => { + assert_eq!(reflection, "grpc://localhost:4180"); + } + _ => panic!("expected Reflection descriptor source"), + } +} + +#[test] +fn test_parse_rate() { + assert_eq!(ProxyConfig::parse_rate("20/min"), Some(20)); + assert_eq!(ProxyConfig::parse_rate("100/min"), Some(100)); + assert_eq!(ProxyConfig::parse_rate("5/min"), Some(5)); + assert_eq!(ProxyConfig::parse_rate("invalid"), None); +} + +#[test] +fn shield_rejects_unknown_field() { + // A typo in a shield-config field (here `profil` for `profile`) must be a + // hard error, not silently ignored: a misspelled security-control key + // would otherwise leave the intended limit unapplied. `deny_unknown_fields` + // on the shield structs turns the typo into a startup failure. + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +shield: + enabled: true + profiles: + auth: { rate: "20/min", burst: 5 } + rules: + - pattern: "/v1/**" + key: { type: ip } + profil: "auth" +"#; + let err = serde_yaml::from_str::(yaml); + assert!(err.is_err(), "unknown shield field must be rejected"); +} + +#[test] +fn shield_rejects_unknown_field_in_rule_key() { + // A stray field inside a rule key (here `name` on an `ip` key, a copy-edit + // leftover) must be a hard error. Silently ignoring it would keep the rule + // IP-keyed instead of the intended per-header limit, weakening the control. + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +shield: + enabled: true + profiles: + auth: { rate: "20/min", burst: 5 } + rules: + - pattern: "/v1/**" + key: { type: ip, name: x-api-key } + profile: "auth" +"#; + let err = serde_yaml::from_str::(yaml); + assert!(err.is_err(), "unknown field in a rule key must be rejected"); +} + +#[test] +fn test_openapi_config_deserialize() { + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +openapi: + enabled: true + path: "/api/openapi.json" + docs_path: "/api/docs" + title: "Test API" + version: "2.0.0" +"#; + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + let openapi = config.openapi.unwrap(); + assert!(openapi.enabled); + assert_eq!(openapi.path, "/api/openapi.json"); + assert_eq!(openapi.docs_path, "/api/docs"); + assert_eq!(openapi.title.unwrap(), "Test API"); + assert_eq!(openapi.version.unwrap(), "2.0.0"); +} + +#[test] +fn test_openapi_config_defaults() { + let yaml = r#" +upstream: + default: "grpc://localhost:4180" +openapi: + enabled: true +"#; + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + let openapi = config.openapi.unwrap(); + assert!(openapi.enabled); + assert_eq!(openapi.path, "/openapi.json"); + assert_eq!(openapi.docs_path, "/docs"); + assert!(openapi.title.is_none()); + assert!(openapi.version.is_none()); +} + +#[test] +fn misspelled_top_level_key_is_reported() { + // `error_detail:` for `error_details:` would leave details on silently; + // it is reported, while known keys (including the ones read outside + // ProxyConfig) are not. + let yaml = r#" +upstream: + default: "grpc://x:1" +streaming: + ndjson_envelope: true +error_details: + enabled: false +error_detail: + enabled: false +maintenence: + enabled: true +"#; + assert_eq!( + unknown_config_keys(yaml), + vec!["error_detail".to_string(), "maintenence".to_string()] + ); +} + +#[test] +fn misspelled_streaming_key_is_reported() { + // `streaming:` is read by two structs that must accept each other's keys, + // so neither rejects a typo such as `ndjson_envelop:`, which would leave + // unenveloped framing silently in force; it is reported by its path. + let yaml = r#" +upstream: + default: "grpc://x:1" +streaming: + sse_keep_alive_secs: 5 + ndjson_envelope: true + ndjson_envelop: true +"#; + assert_eq!( + unknown_config_keys(yaml), + vec!["streaming.ndjson_envelop".to_string()] + ); +} + +#[test] +fn known_top_level_keys_cover_every_proxy_config_field() { + // Exhaustive destructuring: adding a ProxyConfig field fails to compile + // here until the field is also listed as a known top-level key. + let config: ProxyConfig = + serde_yaml::from_str("upstream:\n default: \"grpc://x:1\"\n").unwrap(); + let ProxyConfig { + upstream: _, + descriptors: _, + listen: _, + service: _, + aliases: _, + openapi: _, + auth: _, + shield: _, + oidc_discovery: _, + health: _, + metrics: _, + maintenance: _, + cors: _, + logging: _, + metrics_classes: _, + forwarded_headers: _, + streaming: _, + } = config; + for key in [ + "upstream", + "descriptors", + "listen", + "service", + "aliases", + "openapi", + "auth", + "shield", + "oidc_discovery", + "health", + "metrics", + "maintenance", + "cors", + "logging", + "metrics_classes", + "forwarded_headers", + "streaming", + "error_details", + ] { + assert!(KNOWN_TOP_LEVEL_KEYS.contains(&key), "{key}"); + } + assert_eq!(KNOWN_TOP_LEVEL_KEYS.len(), 18); +} + +#[test] +fn known_streaming_keys_cover_both_streaming_structs() { + // Exhaustive destructuring: a new field in either struct reading + // `streaming:` fails to compile here until it is listed as known. + let StreamingConfig { + sse_keep_alive_secs: _, + } = StreamingConfig::default(); + let StreamingFileConfig { ndjson_envelope: _ } = StreamingFileConfig::default(); + assert_eq!( + KNOWN_STREAMING_KEYS, + &["sse_keep_alive_secs", "ndjson_envelope"] + ); +} + +/// The transcoding options a YAML document compiles to. +fn transcode_options(yaml: &str) -> Result { + serde_yaml::from_str::(yaml) + .map_err(|e| e.to_string())? + .options() +} + +#[test] +fn transcode_settings_default_to_details_everywhere_and_no_envelope() { + // A file that names neither key keeps the defaults: details on every + // route, bare NDJSON lines. + let options = transcode_options("upstream:\n default: \"grpc://x:1\"\n").unwrap(); + assert!(options.error_details.enabled_for("/v1/users/{id}")); + assert!(!options.ndjson_envelope); +} + +#[test] +fn transcode_settings_read_error_details_and_the_ndjson_envelope() { + // Both keys come from the same file as the ProxyConfig, next to its own + // `streaming` keys, and the route rules keep their order. + let yaml = r#" +upstream: + default: "grpc://x:1" +streaming: + sse_keep_alive_secs: 30 + ndjson_envelope: true +error_details: + enabled: false + routes: + - pattern: "/v1/public/internal/*" + enabled: false + - pattern: "/v1/public/**" + enabled: true +"#; + let options = transcode_options(yaml).unwrap(); + assert!(options.ndjson_envelope); + assert!(options.error_details.enabled_for("/v1/public/items")); + assert!(!options + .error_details + .enabled_for("/v1/public/internal/{id}")); + assert!(!options.error_details.enabled_for("/v1/admin/items")); + // The ProxyConfig still reads its own streaming key from the same section. + let config: ProxyConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(config.streaming.sse_keep_alive_secs, 30); +} + +#[test] +fn transcode_settings_reject_unknown_error_details_key() { + // `enable` for `enabled` would otherwise leave details on silently. + let err = transcode_options( + "upstream:\n default: \"grpc://x:1\"\nerror_details:\n enable: false\n", + ) + .unwrap_err(); + assert!(err.contains("enable"), "{err}"); +} + +#[test] +fn transcode_settings_require_enabled_on_a_route_rule() { + // A rule without `enabled` states no decision. + let err = transcode_options( + "upstream:\n default: \"grpc://x:1\"\nerror_details:\n routes:\n - pattern: \"/v1/**\"\n", + ) + .unwrap_err(); + assert!(err.contains("enabled"), "{err}"); +} + +#[test] +fn transcode_settings_reject_a_relative_route_pattern() { + // `v1/admin/**` never matches a route path; accepting it would leave + // details on where the operator meant to turn them off. + let err = transcode_options( + "upstream:\n default: \"grpc://x:1\"\nerror_details:\n routes:\n - pattern: \"v1/admin/**\"\n enabled: false\n", + ) + .unwrap_err(); + assert!(err.contains("must start with '/'"), "{err}"); +} diff --git a/src/lib.rs b/src/lib.rs index 2c54afc..d7d02d9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,6 +114,8 @@ pub struct ProxyServer { verify_path: Option, /// Embedder-supplied JWT verifier, replacing the built-in one. token_verifier: Option>, + /// How the transcoded routes render errors and frame NDJSON streams. + transcode: transcode::TranscodeOptions, } impl ProxyServer { @@ -134,9 +136,53 @@ impl ProxyServer { extra_routes: Vec::new(), verify_path: None, token_verifier: None, + transcode: transcode::TranscodeOptions::default(), } } + /// Create from a YAML document: the [`ProxyConfig`] plus the transcoding + /// settings it does not hold (`error_details:` and + /// `streaming.ndjson_envelope`), applied as [`with_error_details`] and + /// [`with_ndjson_envelope`] would. A top-level or `streaming:` key no + /// setting reads is logged as a warning. + /// + /// # Errors + /// + /// Invalid YAML, a [`ProxyConfig`] that fails + /// [`validate`](ProxyConfig::validate), or an `error_details` route pattern + /// that is relative or not a valid glob. + /// + /// [`with_error_details`]: Self::with_error_details + /// [`with_ndjson_envelope`]: Self::with_ndjson_envelope + pub fn from_yaml_str(yaml: &str) -> anyhow::Result { + let config = ProxyConfig::from_yaml_str(yaml)?; + for key in config::unknown_config_keys(yaml) { + tracing::warn!(%key, "unknown config key is ignored"); + } + let settings: config::TranscodeFileConfig = serde_yaml::from_str(yaml)?; + let options = settings + .options() + .map_err(|e| anyhow::anyhow!("invalid error_details config: {e}"))?; + let mut server = Self::from_config(config); + server.transcode = options; + Ok(server) + } + + /// [`from_yaml_str`](Self::from_yaml_str) on the contents of a file. + /// + /// # Errors + /// + /// The file cannot be read, or its contents are rejected by + /// [`from_yaml_str`](Self::from_yaml_str). + pub fn from_file(path: &std::path::Path) -> anyhow::Result { + Self::from_yaml_str(&std::fs::read_to_string(path)?) + } + + /// The configuration this server was created with. + pub fn config(&self) -> &ProxyConfig { + &self.config + } + /// Create with an embedded descriptor pool (for sid-proxy backward compat). pub fn with_descriptors(mut self, pool: DescriptorPool) -> Self { self.descriptor_pool = Some(pool); @@ -200,6 +246,22 @@ impl ProxyServer { self } + /// Choose which transcoded routes return the upstream's + /// `google.rpc.Status` details in their error bodies. Every route does by + /// default; see [`transcode::error::ErrorDetailsPolicy`] to switch them + /// off globally or per route. + pub fn with_error_details(mut self, policy: transcode::error::ErrorDetailsPolicy) -> Self { + self.transcode = self.transcode.with_error_details(policy); + self + } + + /// Wrap NDJSON stream lines in `{"result"}` / `{"error"}` envelopes; see + /// [`transcode::TranscodeOptions::with_ndjson_envelope`]. + pub fn with_ndjson_envelope(mut self, enabled: bool) -> Self { + self.transcode = self.transcode.with_ndjson_envelope(enabled); + self + } + /// Load descriptor pool from configured sources. /// /// Multiple descriptor files are merged into a single pool, @@ -422,7 +484,8 @@ impl ProxyServer { let cors = self.build_cors(); // Build transcoding routes from descriptor pool. - let mut transcode_routes = transcode::routes(&pool, &self.config.aliases); + let mut transcode_routes = + transcode::routes_with_options(&pool, &self.config.aliases, &self.transcode); // External authorization (Envoy ext_authz) gates only the proxied API // routes, never health / metrics / discovery. It runs inside the auth diff --git a/src/main.rs b/src/main.rs index 81fe4e9..9e3bbdc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,8 +25,10 @@ async fn main() -> anyhow::Result<()> { .init(); let cli = Cli::parse(); - let config = - structured_proxy::config::ProxyConfig::from_file(std::path::Path::new(&cli.config))?; + // Reads the ProxyConfig and the transcoding settings kept outside it + // (error_details, streaming.ndjson_envelope) from the same file. + let server = structured_proxy::ProxyServer::from_file(std::path::Path::new(&cli.config))?; + let config = server.config(); tracing::info!( service = %config.service.name, @@ -36,6 +38,5 @@ async fn main() -> anyhow::Result<()> { "Starting structured-proxy" ); - let server = structured_proxy::ProxyServer::from_config(config); server.serve().await } diff --git a/src/shield/matcher.rs b/src/shield/matcher.rs index d75effb..5469021 100644 --- a/src/shield/matcher.rs +++ b/src/shield/matcher.rs @@ -67,7 +67,7 @@ pub fn short_hash(input: &str) -> String { /// Build a glob matcher where `*` stays within a path segment and `**` spans /// segments, matching the `google.api.http` / maintenance path convention. -fn path_glob(pattern: &str) -> Result { +pub(crate) fn path_glob(pattern: &str) -> Result { globset::GlobBuilder::new(pattern) .literal_separator(true) .build() diff --git a/src/transcode/codec.rs b/src/transcode/codec.rs index f3e800b..2385e5d 100644 --- a/src/transcode/codec.rs +++ b/src/transcode/codec.rs @@ -3,9 +3,8 @@ //! Allows sending/receiving protobuf messages without compile-time type information, //! using `MessageDescriptor` for runtime encoding/decoding. -use prost::bytes::Buf; use prost::Message; -use prost_reflect::{DynamicMessage, MessageDescriptor}; +use prost_reflect::{Cardinality, DynamicMessage, Kind, MessageDescriptor, ReflectMessage, Value}; use tonic::codec::{BufferSettings, Codec, DecodeBuf, Decoder, EncodeBuf, Encoder}; use tonic::Status; @@ -31,11 +30,18 @@ impl Encoder for DynamicEncoder { #[derive(Debug, Clone)] pub struct DynamicDecoder { desc: MessageDescriptor, + /// Whether `desc` has a proto2 `required` field at any depth, so decoded + /// messages must be checked for it; false for every proto3 type. + check_required: bool, } impl DynamicDecoder { pub fn new(desc: MessageDescriptor) -> Self { - Self { desc } + let check_required = has_required_fields(&desc); + Self { + desc, + check_required, + } } } @@ -43,13 +49,22 @@ impl Decoder for DynamicDecoder { type Item = DynamicMessage; type Error = Status; + /// Decode one gRPC frame. tonic calls this once per complete frame, so an + /// empty buffer is a message whose fields all hold their defaults (e.g. + /// `google.protobuf.Empty`), not the absence of one. A message missing a + /// proto2 `required` field is rejected, as protobuf parsers do by default + /// (Go `proto.Unmarshal`, C++ `ParseFromString`); the decoder itself + /// accepts it partially. fn decode(&mut self, buf: &mut DecodeBuf<'_>) -> Result, Status> { - let remaining = buf.remaining(); - if remaining == 0 { - return Ok(None); - } - let msg = DynamicMessage::decode(self.desc.clone(), buf.copy_to_bytes(remaining)) + let msg = DynamicMessage::decode(self.desc.clone(), buf) .map_err(|e| Status::internal(format!("decode error: {e}")))?; + if self.check_required { + if let Some(field) = missing_required(&msg) { + return Err(Status::internal(format!( + "decode error: required field {field} is missing" + ))); + } + } Ok(Some(msg)) } @@ -62,11 +77,25 @@ impl Decoder for DynamicDecoder { #[derive(Debug, Clone)] pub struct DynamicCodec { response_desc: MessageDescriptor, + check_required: bool, } impl DynamicCodec { pub fn new(response_desc: MessageDescriptor) -> Self { - Self { response_desc } + let check_required = has_required_fields(&response_desc); + Self::with_required_check(response_desc, check_required) + } + + /// [`new`](Self::new) with [`has_required_fields`] of `response_desc` + /// already known, so a route computes it once rather than per call. + pub(crate) fn with_required_check( + response_desc: MessageDescriptor, + check_required: bool, + ) -> Self { + Self { + response_desc, + check_required, + } } } @@ -81,22 +110,72 @@ impl Codec for DynamicCodec { } fn decoder(&mut self) -> Self::Decoder { - DynamicDecoder::new(self.response_desc.clone()) + DynamicDecoder { + desc: self.response_desc.clone(), + check_required: self.check_required, + } } } -#[cfg(test)] -mod tests { - #[test] - fn test_dynamic_codec_creation() { - // Use google.protobuf.Empty as a universal test message - let pool = prost_reflect::DescriptorPool::decode( - prost_reflect::DescriptorPool::global() - .encode_to_vec() - .as_slice(), - ) - .unwrap_or_else(|_| prost_reflect::DescriptorPool::new()); - // Basic smoke test — codec can be created with any message descriptor - let _ = pool; +/// Whether a message of `desc`, or one it can contain at any depth (fields, +/// repeated and map values, extensions), has a proto2 `required` field. +pub(crate) fn has_required_fields(desc: &MessageDescriptor) -> bool { + let mut seen = std::collections::HashSet::new(); + let mut pending = vec![desc.clone()]; + while let Some(desc) = pending.pop() { + if !seen.insert(desc.full_name().to_owned()) { + continue; + } + for field in desc.fields() { + if field.cardinality() == Cardinality::Required { + return true; + } + if let Kind::Message(inner) = field.kind() { + pending.push(inner); + } + } + for extension in desc.extensions() { + if let Kind::Message(inner) = extension.kind() { + pending.push(inner); + } + } } + false } + +/// The full name of the first proto2 `required` field left unset in `msg` or +/// a message nested in it, if any. +pub(crate) fn missing_required(msg: &DynamicMessage) -> Option { + let desc = msg.descriptor(); + if let Some(field) = desc + .fields() + .find(|field| field.cardinality() == Cardinality::Required && !msg.has_field(field)) + { + return Some(field.full_name().to_owned()); + } + let values = msg + .fields() + .map(|(_, value)| value) + .chain(msg.extensions().map(|(_, value)| value)); + for value in values { + let missing = match value { + Value::Message(inner) => missing_required(inner), + Value::List(items) => items.iter().find_map(|item| match item { + Value::Message(inner) => missing_required(inner), + _ => None, + }), + Value::Map(entries) => entries.values().find_map(|entry| match entry { + Value::Message(inner) => missing_required(inner), + _ => None, + }), + _ => None, + }; + if missing.is_some() { + return missing; + } + } + None +} + +#[cfg(test)] +mod tests; diff --git a/src/transcode/codec/tests.rs b/src/transcode/codec/tests.rs new file mode 100644 index 0000000..31541bd --- /dev/null +++ b/src/transcode/codec/tests.rs @@ -0,0 +1,78 @@ +use super::*; + +/// A pool compiled from one in-memory `.proto` source. +fn pool(source: &str) -> prost_reflect::DescriptorPool { + struct OneFile(String); + impl protox::file::FileResolver for OneFile { + fn open_file(&self, name: &str) -> Result { + if name == "test.proto" { + protox::file::File::from_source(name, &self.0) + } else { + protox::file::GoogleFileResolver::new().open_file(name) + } + } + } + protox::Compiler::with_file_resolver(OneFile(source.to_owned())) + .open_file("test.proto") + .expect("test proto compiles") + .descriptor_pool() +} + +const REQUIRED: &str = "syntax = \"proto2\"; package t; \ + message Leaf { required string id = 1; } \ + message Holder { optional Leaf one = 1; repeated Leaf many = 2; map by_key = 3; } \ + message Tree { optional string name = 1; repeated Tree children = 2; } \ + message Loose { optional string note = 1; }"; + +fn leaf(pool: &prost_reflect::DescriptorPool, id: Option<&str>) -> Value { + let mut leaf = DynamicMessage::new(pool.get_message_by_name("t.Leaf").unwrap()); + if let Some(id) = id { + leaf.set_field_by_name("id", Value::String(id.into())); + } + Value::Message(leaf) +} + +#[test] +fn required_fields_are_found_at_any_depth_and_recursion_ends() { + // Holder has no required field itself but contains Leaf; Tree refers to + // itself and has none, so the walk must stop rather than loop. + let pool = pool(REQUIRED); + let desc = |name: &str| pool.get_message_by_name(name).unwrap(); + assert!(has_required_fields(&desc("t.Leaf"))); + assert!(has_required_fields(&desc("t.Holder"))); + assert!(!has_required_fields(&desc("t.Tree"))); + assert!(!has_required_fields(&desc("t.Loose"))); +} + +#[test] +fn missing_required_field_is_reported_wherever_it_sits() { + // An unset required field is reported through a singular field, a + // repeated element or a map value; a fully set message reports nothing. + let pool = pool(REQUIRED); + let holder = || DynamicMessage::new(pool.get_message_by_name("t.Holder").unwrap()); + + let mut one = holder(); + one.set_field_by_name("one", leaf(&pool, None)); + let mut many = holder(); + many.set_field_by_name( + "many", + Value::List(vec![leaf(&pool, Some("a")), leaf(&pool, None)]), + ); + let mut by_key = holder(); + let mut entries = std::collections::HashMap::new(); + entries.insert(prost_reflect::MapKey::String("k".into()), leaf(&pool, None)); + by_key.set_field_by_name("by_key", Value::Map(entries)); + for msg in [one, many, by_key] { + assert_eq!( + missing_required(&msg).as_deref(), + Some("t.Leaf.id"), + "{msg:?}" + ); + } + + let mut complete = holder(); + complete.set_field_by_name("one", leaf(&pool, Some("a"))); + complete.set_field_by_name("many", Value::List(vec![leaf(&pool, Some("b"))])); + assert_eq!(missing_required(&complete), None); + assert_eq!(missing_required(&holder()), None); +} diff --git a/src/transcode/error.rs b/src/transcode/error.rs index 0bc6860..cb10d9b 100644 --- a/src/transcode/error.rs +++ b/src/transcode/error.rs @@ -1,11 +1,26 @@ //! gRPC → HTTP error mapping. //! //! Converts `tonic::Status` to appropriate HTTP status codes and JSON error bodies -//! following the gRPC-HTTP status code mapping from the gRPC specification. +//! following the gRPC-HTTP status code mapping from the gRPC specification, and +//! renders the typed `google.rpc.Status` details the upstream attached in the +//! `grpc-status-details-bin` trailer. use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use axum::Json; +use base64::Engine as _; +use globset::GlobMatcher; +use prost::Message as _; +use prost_reflect::{ + Cardinality, DescriptorPool, DynamicMessage, Kind, MessageDescriptor, ReflectMessage, + SerializeOptions, Value as PbValue, +}; +use serde_json::{Map, Value}; + +/// Full name of `google.rpc.DebugInfo`. It carries stack traces and server +/// internals meant for the service's operators, so it never reaches an HTTP +/// client. +const DEBUG_INFO: &str = "google.rpc.DebugInfo"; /// Map a gRPC status code to the corresponding HTTP status code. /// @@ -32,15 +47,90 @@ pub fn grpc_to_http_status(code: tonic::Code) -> StatusCode { } } -/// Convert a `tonic::Status` into an axum HTTP response with JSON error body. +/// Message of the `INTERNAL` a client gets instead of an upstream error whose +/// details cannot be rendered faithfully. The cause stays in the proxy's log. +const MALFORMED_STATUS_MESSAGE: &str = "upstream returned a malformed error status"; + +/// An upstream error status whose details cannot be rendered faithfully: a +/// trailer that is not a `google.rpc.Status`, or a detail of a known type whose +/// bytes do not decode or whose value has no valid JSON form. Its cause is +/// logged where it is detected. +#[derive(Debug)] +struct MalformedStatus; + +/// Convert a `tonic::Status` into an axum HTTP response with a JSON error body +/// `{"error", "message", "code"}`, without status details. +/// +/// Use [`status_to_response_with_details`] to also render the upstream's +/// `google.rpc.Status` details. pub fn status_to_response(status: tonic::Status) -> Response { - let http_status = grpc_to_http_status(status.code()); - let body = serde_json::json!({ - "error": grpc_code_name(status.code()), - "message": status.message(), - "code": status.code() as i32, - }); - (http_status, Json(body)).into_response() + status_to_response_with_details(&status, None) +} + +/// Convert a `tonic::Status` into an axum HTTP response with a JSON error body. +/// +/// The body is `{"error", "message", "code"}`, plus a `details` array when +/// `details` is given (see [`error_body`]). The HTTP status follows the code the +/// body reports, so a malformed upstream status answers 500. +pub fn status_to_response_with_details( + status: &tonic::Status, + details: Option<&StatusDetails>, +) -> Response { + let (code, body) = render(status, details); + (grpc_to_http_status(code), Json(body)).into_response() +} + +/// The JSON error body for a failed call, shared by the unary response and the +/// terminal frame of a stream so a client parses one shape everywhere. +/// +/// `error` is the gRPC code name, `code` its number and `message` the status +/// message. With `details`, the body also carries `details`: the upstream's +/// `google.rpc.Status.details` in proto3 JSON form (empty when the upstream sent +/// none), plus `opaqueDetails` for details whose type no descriptor describes +/// when the renderer forwards them ([`StatusDetails::with_opaque_details`]); +/// without it, both keys are absent and the trailer is not read. When the +/// details cannot be rendered faithfully the whole body is a generic `INTERNAL` +/// instead, never a partial or reinterpreted set of details. +pub fn error_body(status: &tonic::Status, details: Option<&StatusDetails>) -> Value { + render(status, details).1 +} + +/// The error body and the gRPC code it reports: the upstream's own, or +/// `INTERNAL` when its details cannot be rendered faithfully. +fn render(status: &tonic::Status, details: Option<&StatusDetails>) -> (tonic::Code, Value) { + let Some(details) = details else { + return (status.code(), body(status.code(), status.message(), None)); + }; + match details.render(status) { + Ok(rendered) => ( + status.code(), + body(status.code(), status.message(), Some(rendered)), + ), + Err(MalformedStatus) => ( + tonic::Code::Internal, + body( + tonic::Code::Internal, + MALFORMED_STATUS_MESSAGE, + Some(RenderedDetails::default()), + ), + ), + } +} + +/// `opaqueDetails` appears only when a detail went to the extension, so a +/// client that does not handle it sees nothing new otherwise. +fn body(code: tonic::Code, message: &str, details: Option) -> Value { + let mut body = Map::with_capacity(5); + body.insert("error".into(), grpc_code_name(code).into()); + body.insert("message".into(), message.into()); + body.insert("code".into(), (code as i32).into()); + if let Some(rendered) = details { + body.insert("details".into(), Value::Array(rendered.details)); + if !rendered.opaque.is_empty() { + body.insert("opaqueDetails".into(), Value::Array(rendered.opaque)); + } + } + Value::Object(body) } /// Human-readable gRPC code name for JSON error responses. @@ -66,69 +156,672 @@ pub(crate) fn grpc_code_name(code: tonic::Code) -> &'static str { } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_grpc_to_http_mapping() { - assert_eq!(grpc_to_http_status(tonic::Code::Ok), StatusCode::OK); - assert_eq!( - grpc_to_http_status(tonic::Code::InvalidArgument), - StatusCode::BAD_REQUEST - ); - assert_eq!( - grpc_to_http_status(tonic::Code::NotFound), - StatusCode::NOT_FOUND - ); - assert_eq!( - grpc_to_http_status(tonic::Code::AlreadyExists), - StatusCode::CONFLICT - ); - assert_eq!( - grpc_to_http_status(tonic::Code::PermissionDenied), - StatusCode::FORBIDDEN - ); - assert_eq!( - grpc_to_http_status(tonic::Code::Unauthenticated), - StatusCode::UNAUTHORIZED - ); - assert_eq!( - grpc_to_http_status(tonic::Code::ResourceExhausted), - StatusCode::TOO_MANY_REQUESTS - ); - assert_eq!( - grpc_to_http_status(tonic::Code::Unimplemented), - StatusCode::NOT_IMPLEMENTED - ); - assert_eq!( - grpc_to_http_status(tonic::Code::Internal), - StatusCode::INTERNAL_SERVER_ERROR - ); - assert_eq!( - grpc_to_http_status(tonic::Code::Unavailable), - StatusCode::SERVICE_UNAVAILABLE - ); - assert_eq!( - grpc_to_http_status(tonic::Code::DeadlineExceeded), - StatusCode::GATEWAY_TIMEOUT - ); - } - - #[test] - fn test_grpc_code_name() { - assert_eq!(grpc_code_name(tonic::Code::Ok), "OK"); - assert_eq!(grpc_code_name(tonic::Code::NotFound), "NOT_FOUND"); - assert_eq!( - grpc_code_name(tonic::Code::Unauthenticated), - "UNAUTHENTICATED" - ); - } - - #[test] - fn test_status_to_response() { - let status = tonic::Status::not_found("user not found"); - let response = status_to_response(status); - assert_eq!(response.status(), StatusCode::NOT_FOUND); +/// Which routes return `details` in their error bodies. +/// +/// A global switch plus per-route overrides, checked in the order they were +/// added: the first whose pattern matches the route decides. A pattern is a +/// glob over the mounted route path, where `*` stays within one segment and +/// `**` spans segments; every path parameter counts as one segment, so +/// `/v1/users/*` matches the route `/v1/users/{id}`. Decided once per route +/// when the router is built, so a request pays nothing for it. +/// +/// # Examples +/// +/// ``` +/// use structured_proxy::transcode::error::ErrorDetailsPolicy; +/// +/// // Details everywhere except the internal admin surface. +/// let policy = ErrorDetailsPolicy::default() +/// .route("/v1/admin/**", false) +/// .unwrap(); +/// assert!(policy.enabled_for("/v1/users/{id}")); +/// assert!(!policy.enabled_for("/v1/admin/users/{id}")); +/// +/// // Off by default, back on for one sub-route. +/// let policy = ErrorDetailsPolicy::disabled() +/// .route("/v1/public/**", true) +/// .unwrap(); +/// assert!(!policy.enabled_for("/v1/users/{id}")); +/// assert!(policy.enabled_for("/v1/public/items")); +/// +/// // The opaque-detail extension, off unless switched on, here for one route. +/// let policy = ErrorDetailsPolicy::default() +/// .opaque_route("/v1/partner/**", true) +/// .unwrap(); +/// assert!(!policy.opaque_for("/v1/users/{id}")); +/// assert!(policy.opaque_for("/v1/partner/orders")); +/// ``` +#[derive(Debug, Clone)] +pub struct ErrorDetailsPolicy { + enabled: bool, + opaque: bool, + routes: Vec, +} + +/// One override: a route glob and the switches it sets. A switch it leaves +/// `None` is decided by a later rule or the global value. +#[derive(Debug, Clone)] +struct RouteRule { + matcher: GlobMatcher, + enabled: Option, + opaque: Option, +} + +impl ErrorDetailsPolicy { + /// No details on any route, until a [`route`](Self::route) switches them on. + pub fn disabled() -> Self { + Self { + enabled: false, + opaque: false, + routes: Vec::new(), + } + } + + /// Add an override for the routes `pattern` matches, checked after the + /// ones added before it. + /// + /// # Errors + /// + /// A pattern that does not start with `/` (it could never match a route) + /// or is not a valid glob. + pub fn route(self, pattern: &str, enabled: bool) -> Result { + self.rule(pattern, Some(enabled), None) + } + + /// Set, for every route without an override, whether a detail whose type + /// no descriptor describes goes to `opaqueDetails` (see + /// [`StatusDetails::with_opaque_details`]). Off by default: such bytes + /// cannot be checked for a nested DebugInfo. + pub fn with_opaque_details(mut self, enabled: bool) -> Self { + self.opaque = enabled; + self + } + + /// Add an override of the opaque-detail extension for the routes `pattern` + /// matches, checked after the ones added before it. + /// + /// # Errors + /// + /// As [`route`](Self::route). + pub fn opaque_route(self, pattern: &str, enabled: bool) -> Result { + self.rule(pattern, None, Some(enabled)) + } + + /// Add an override setting either switch or both; the first matching rule + /// that sets a switch decides it. + /// + /// # Errors + /// + /// As [`route`](Self::route). + pub(crate) fn rule( + mut self, + pattern: &str, + enabled: Option, + opaque: Option, + ) -> Result { + // Route paths always start with `/`; a relative pattern is a + // missing-slash typo that would silently never apply. + if !pattern.starts_with('/') { + return Err(format!( + "error details route pattern {pattern:?} must start with '/'" + )); + } + self.routes.push(RouteRule { + matcher: crate::shield::matcher::path_glob(pattern)?, + enabled, + opaque, + }); + Ok(self) + } + + /// Whether the route mounted at `route_path` (axum form, e.g. + /// `/v1/users/{id}`) returns details: the first matching rule that sets + /// it decides, otherwise the global switch. + pub fn enabled_for(&self, route_path: &str) -> bool { + self.decide(route_path, |rule| rule.enabled) + .unwrap_or(self.enabled) + } + + /// Whether the route mounted at `route_path` forwards details of unknown + /// types in `opaqueDetails`: the first matching rule that sets it decides, + /// otherwise the global switch. + pub fn opaque_for(&self, route_path: &str) -> bool { + self.decide(route_path, |rule| rule.opaque) + .unwrap_or(self.opaque) + } + + fn decide( + &self, + route_path: &str, + switch: impl Fn(&RouteRule) -> Option, + ) -> Option { + self.routes + .iter() + .filter_map(|rule| switch(rule).map(|value| (rule, value))) + .find(|(rule, _)| rule.matcher.is_match(route_path)) + .map(|(_, value)| value) + } +} + +impl Default for ErrorDetailsPolicy { + /// Details on every route, the opaque-detail extension off. + fn default() -> Self { + Self { + enabled: true, + opaque: false, + routes: Vec::new(), + } + } +} + +/// Whether `full_name` is a well-known type with a special ProtoJSON +/// representation, which an `Any` carries under `value` +/// (). The same set +/// prost-reflect wraps when it serializes an `Any`. +fn has_special_json(full_name: &str) -> bool { + matches!( + full_name, + "google.protobuf.Any" + | "google.protobuf.Timestamp" + | "google.protobuf.Duration" + | "google.protobuf.Struct" + | "google.protobuf.FloatValue" + | "google.protobuf.DoubleValue" + | "google.protobuf.Int32Value" + | "google.protobuf.Int64Value" + | "google.protobuf.UInt32Value" + | "google.protobuf.UInt64Value" + | "google.protobuf.BoolValue" + | "google.protobuf.StringValue" + | "google.protobuf.BytesValue" + | "google.protobuf.FieldMask" + | "google.protobuf.ListValue" + | "google.protobuf.Value" + | "google.protobuf.Empty" + ) +} + +/// Full name of `google.protobuf.Any`. +const ANY: &str = "google.protobuf.Any"; + +/// The type name an `Any.type_url` names, or `None` when the URL is malformed. +/// +/// The URL must contain a `/`, and the part after the last one is the type's +/// full name (`google/protobuf/any.proto`). Anything else (a bare name, a +/// trailing `/`, a query suffix, an empty segment) could disguise a +/// DebugInfo, so callers refuse the whole status on `None`. +fn any_type_name(type_url: &str) -> Option<&str> { + let (_, name) = type_url.rsplit_once('/')?; + is_full_name(name).then_some(name) +} + +/// Whether `name` is a protobuf full name: dot-separated identifiers, each a +/// letter or `_` followed by letters, digits or `_`. +fn is_full_name(name: &str) -> bool { + !name.is_empty() + && name.split('.').all(|part| { + let mut chars = part.chars(); + matches!(chars.next(), Some(c) if c.is_ascii_alphabetic() || c == '_') + && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') + }) +} + +/// The rendered details of one status. +#[derive(Debug, Default)] +struct RenderedDetails { + /// ProtoJSON `Any` entries, in upstream order. + details: Vec, + /// Opaque-detail extension entries, in upstream order. + opaque: Vec, +} + +/// An entry of the structured-proxy opaque-detail extension, for a detail whose +/// type no descriptor describes: `{"index", "typeUrl", "bytes"}`, where `index` +/// is its position among the forwarded details (so merging `details` and +/// `opaqueDetails` by position restores the upstream order), `typeUrl` the +/// original type URL and `bytes` the standard base64 of the original bytes. +/// It carries no `@type` and lives outside `details`, so it cannot be taken for +/// a ProtoJSON `Any`. +fn opaque_entry(index: usize, type_url: &str, value: &[u8]) -> Value { + let mut out = Map::with_capacity(3); + out.insert("index".into(), index.into()); + out.insert("typeUrl".into(), type_url.into()); + out.insert( + "bytes".into(), + base64::engine::general_purpose::STANDARD + .encode(value) + .into(), + ); + Value::Object(out) +} + +/// Add to `pool` every top-level message and enum of `canonical` it does not +/// define, per type rather than per file. +/// +/// A product may define some canonical types itself: in a file of another name, +/// or in its own revision of the canonical file holding only a subset. Those +/// definitions win. The canonical types it lacks still go in, through a copy +/// of the canonical file renamed under `structured-proxy/canonical/` and +/// reduced to the missing types; the copy also imports the product files that +/// define the types it dropped, so its references resolve to the product's +/// revisions. +fn complete_with_canonical(pool: &mut DescriptorPool, canonical: &DescriptorPool) { + use std::collections::{BTreeSet, HashMap}; + + // Canonical file name → the pool files that now provide its types in its + // place (its renamed copy, product files defining some of them). A file + // importing it imports those instead. + let mut stand_ins: HashMap> = HashMap::new(); + // `files()` lists dependencies before their dependents, so each file finds + // its imports already merged. + for file in canonical.files() { + let mut proto = file.file_descriptor_proto().clone(); + let package = proto.package().to_owned(); + let full = |name: &str| { + if package.is_empty() { + name.to_owned() + } else { + format!("{package}.{name}") + } + }; + // Files of the pool that already define a type of this file. + let mut defining_files = BTreeSet::new(); + proto.message_type.retain(|message| { + match pool.get_message_by_name(&full(message.name())) { + Some(existing) => { + defining_files.insert(existing.parent_file().name().to_owned()); + false + } + None => true, + } + }); + proto + .enum_type + .retain(|en| match pool.get_enum_by_name(&full(en.name())) { + Some(existing) => { + defining_files.insert(existing.parent_file().name().to_owned()); + false + } + None => true, + }); + let everything_present = proto.message_type.is_empty() && proto.enum_type.is_empty(); + let file_present = pool.get_file_by_name(file.name()).is_some(); + if everything_present { + // Typically the very same file. When the types live in files of + // other names, importers of this one import those instead. + if !file_present { + stand_ins.insert(file.name().to_owned(), defining_files); + } + continue; + } + // A copy that leaves types out, or whose name the product already uses, + // goes in under its own name; importers then need it plus the product + // files that define the rest. + let renamed = !defining_files.is_empty() || file_present; + let mut provided_by = defining_files.clone(); + if renamed { + let name = format!("structured-proxy/canonical/{}", file.name()); + proto.name = Some(name.clone()); + provided_by.insert(name); + } + let mut dependencies = BTreeSet::new(); + for dep in &proto.dependency { + match stand_ins.get(dep) { + Some(files) => { + // A product revision of the dependency stays importable + // next to the copy that completes it. + if pool.get_file_by_name(dep).is_some() { + dependencies.insert(dep.clone()); + } + dependencies.extend(files.iter().cloned()); + } + None => { + dependencies.insert(dep.clone()); + } + } + } + dependencies.extend(defining_files); + proto.dependency = dependencies.into_iter().collect(); + // Indexes into the old dependency list; nothing here needs them. + proto.public_dependency.clear(); + proto.weak_dependency.clear(); + proto.source_code_info = None; + match pool.add_file_descriptor_proto(proto) { + Ok(()) => { + if renamed { + stand_ins.insert(file.name().to_owned(), provided_by); + } + } + Err(e) => { + tracing::debug!(file = %file.name(), "canonical descriptor not merged: {e}"); + } + } + } +} + +/// Renders the typed details of a gRPC status (`grpc-status-details-bin`) as +/// proto3 JSON. +/// +/// Detail types resolve in one pool: the product descriptors, completed with +/// the well-known types and the canonical `google/rpc/status.proto` and +/// `error_details.proto` for whatever the product does not define itself. A +/// service's own detail messages (and its own `google.rpc` revision) render as +/// it defines them, the canonical ones are always available even when the +/// product descriptors do not import them, and a type packed inside another +/// detail resolves from the same pool as the detail itself. +/// +/// Details use the canonical proto3 JSON mapping (unset fields omitted, 64-bit +/// integers as strings), the form clients of the `google.rpc` model expect. +#[derive(Debug, Clone)] +pub struct StatusDetails { + pool: DescriptorPool, + opaque: bool, +} + +impl StatusDetails { + /// Build a renderer over `product`, completed with the canonical + /// descriptors it lacks. The opaque-detail extension starts off. + pub fn new(product: &DescriptorPool) -> Self { + let mut canonical = DescriptorPool::global(); + canonical + .decode_file_descriptor_set(tonic_types::pb::FILE_DESCRIPTOR_SET) + .expect("tonic-types ships a valid google.rpc descriptor set"); + let mut pool = product.clone(); + complete_with_canonical(&mut pool, &canonical); + Self { + pool, + opaque: false, + } + } + + /// Whether a detail whose type no descriptor describes goes to + /// `opaqueDetails` (see [`opaque_entry`]) instead of being withheld. Its + /// bytes cannot be inspected, so a DebugInfo it carries in a field would + /// reach the client: switch it on only for upstreams trusted not to nest + /// one in types the proxy has no descriptor for. + pub fn with_opaque_details(mut self, enabled: bool) -> Self { + self.opaque = enabled; + self + } + + /// The details of `status`, `google.rpc.DebugInfo` left out. + /// + /// A detail whose type resolves goes to `details` in its ProtoJSON `Any` + /// form: `@type` plus the message fields, or `@type` plus `value` for a + /// well-known type with a special JSON representation. A type no descriptor + /// describes has no ProtoJSON form (the mapping requires the type) and its + /// bytes cannot be checked for a DebugInfo, so it is withheld, or goes to + /// the opaque-detail extension (see [`opaque_entry`]) when switched on. + /// + /// # Errors + /// + /// [`MalformedStatus`] when the trailer is not a `google.rpc.Status`, or a + /// detail of a known type does not decode or has no valid JSON form. Such a + /// detail is never passed on as opaque bytes, since that would present a + /// broken value as an unknown one. + fn render(&self, status: &tonic::Status) -> Result { + let mut rendered = RenderedDetails::default(); + let raw = status.details(); + if raw.is_empty() { + return Ok(rendered); + } + let decoded = tonic_types::pb::Status::decode(raw).map_err(|e| { + tracing::error!("malformed grpc-status-details-bin trailer: {e}"); + MalformedStatus + })?; + // The trailer must describe the same error as grpc-status and + // grpc-message (gRPC richer error model); otherwise its details would + // be attached to an error they were not written for. + if decoded.code != status.code() as i32 || decoded.message != status.message() { + tracing::error!( + trailer_code = decoded.code, + status_code = status.code() as i32, + "grpc-status-details-bin disagrees with grpc-status / grpc-message" + ); + return Err(MalformedStatus); + } + rendered.details.reserve(decoded.details.len()); + // Position among the forwarded details: DebugInfo takes no index, so + // the numbering does not reveal that one was withheld. + let mut index = 0usize; + for any in &decoded.details { + let type_url = any.type_url.as_str(); + let Some(type_name) = any_type_name(type_url) else { + tracing::error!(%type_url, "error detail with a malformed type URL"); + return Err(MalformedStatus); + }; + if type_name == DEBUG_INFO { + continue; + } + match self.resolve(type_name) { + Some(desc) => match self.typed_entry(type_url, type_name, desc, &any.value)? { + Some(entry) => rendered.details.push(entry), + // An Any detail packing DebugInfo, withheld like a direct one. + None => continue, + }, + None if self.opaque => { + rendered + .opaque + .push(opaque_entry(index, type_url, &any.value)); + } + None => { + tracing::debug!(%type_url, "error detail of an unknown type withheld"); + continue; + } + } + index += 1; + } + Ok(rendered) + } + + /// The ProtoJSON `Any` form of a detail whose type resolved, with every + /// DebugInfo packed inside it removed; `None` when the detail is itself an + /// `Any` packing a DebugInfo. + fn typed_entry( + &self, + type_url: &str, + type_name: &str, + desc: MessageDescriptor, + value: &[u8], + ) -> Result, MalformedStatus> { + // ProtoJSON puts a well-known type with a special JSON representation + // under `value` whatever that JSON looks like (a Struct is an object, + // yet still wrapped), so the choice follows the type, not the shape. + // Empty is one of them (JSON `{}` in the ProtoJSON table), as in Go's + // protojson and prost-reflect, so it is wrapped too. + let wrapped = has_special_json(desc.full_name()); + let mut msg = DynamicMessage::decode(desc, value).map_err(|e| { + tracing::error!(detail = %type_name, "undecodable error detail: {e}"); + MalformedStatus + })?; + if self.scrub(&mut msg)?.is_none() { + return Ok(None); + } + let json = msg + .serialize_with_options(serde_json::value::Serializer, &SerializeOptions::new()) + .map_err(|e| { + tracing::error!(detail = %type_name, "error detail has no valid JSON form: {e}"); + MalformedStatus + })?; + let mut out = Map::new(); + out.insert("@type".into(), type_url.into()); + match json { + Value::Object(fields) if !wrapped => { + // A field whose JSON name is `@type` would replace the Any's + // own type URL, and there is no faithful ProtoJSON form for it. + if fields.contains_key("@type") { + tracing::error!(detail = %type_name, "error detail has a field named @type"); + return Err(MalformedStatus); + } + out.extend(fields); + } + other => { + out.insert("value".into(), other); + } + } + Ok(Some(Value::Object(out))) + } + + fn resolve(&self, type_name: &str) -> Option { + self.pool.get_message_by_name(type_name) + } + + /// Remove every DebugInfo anywhere below `msg`: a message field typed as + /// DebugInfo, or an `Any` packing one, whether singular, repeated or a map + /// value. Only the message types decide, so data such as a `Struct` key + /// named `@type` is never mistaken for one. Returns `None` when `msg` is + /// itself a DebugInfo or an `Any` packing one (the caller drops it), + /// otherwise whether anything changed. A message missing a proto2 + /// `required` field is [`MalformedStatus`]: decoding accepts it, but it has + /// no valid ProtoJSON form. + fn scrub(&self, msg: &mut DynamicMessage) -> Result, MalformedStatus> { + let desc = msg.descriptor(); + if desc.full_name() == ANY { + return self.scrub_any(msg); + } + if desc.full_name() == DEBUG_INFO { + return Ok(None); + } + for field in desc.fields() { + if field.cardinality() == Cardinality::Required && !msg.has_field(&field) { + tracing::error!( + message = %desc.full_name(), + field = %field.name(), + "error detail lacks a required field" + ); + return Err(MalformedStatus); + } + } + let mut changed = false; + let mut dropped_fields = Vec::new(); + for (field, value) in msg.fields_mut() { + if !matches!(field.kind(), Kind::Message(_)) { + continue; + } + if !self.scrub_value(value, &mut changed)? { + // A proto2 `required` field cannot be cleared without making + // the message invalid, so the whole message goes instead. + if field.cardinality() == Cardinality::Required { + return Ok(None); + } + dropped_fields.push(field); + } + } + // Extensions are not among the message's own fields, yet ProtoJSON + // renders them (as `[full.name]`), so they are scrubbed the same way. + let mut dropped_extensions = Vec::new(); + for (extension, value) in msg.extensions_mut() { + if !matches!(extension.kind(), Kind::Message(_)) { + continue; + } + if !self.scrub_value(value, &mut changed)? { + dropped_extensions.push(extension); + } + } + for field in dropped_fields { + msg.clear_field(&field); + changed = true; + } + for extension in dropped_extensions { + msg.clear_extension(&extension); + changed = true; + } + Ok(Some(changed)) + } + + /// [`scrub`](Self::scrub) applied to one set field or extension value: + /// repeated elements and map entries that must go are removed in place. + /// Returns `false` when the value is a singular message that must go, so + /// the caller clears the field. + fn scrub_value( + &self, + value: &mut PbValue, + changed: &mut bool, + ) -> Result { + match value { + PbValue::Message(inner) => match self.scrub(inner)? { + Some(inner_changed) => *changed |= inner_changed, + None => return Ok(false), + }, + PbValue::List(items) => { + let before = items.len(); + let mut failed = None; + items.retain_mut(|item| match item { + PbValue::Message(inner) if failed.is_none() => match self.scrub(inner) { + Ok(Some(inner_changed)) => { + *changed |= inner_changed; + true + } + Ok(None) => false, + Err(e) => { + failed = Some(e); + true + } + }, + _ => true, + }); + if let Some(e) = failed { + return Err(e); + } + *changed |= items.len() != before; + } + PbValue::Map(entries) => { + let mut dropped = Vec::new(); + for (key, value) in entries.iter_mut() { + if let PbValue::Message(inner) = value { + match self.scrub(inner)? { + Some(inner_changed) => *changed |= inner_changed, + None => dropped.push(key.clone()), + } + } + } + for key in dropped { + entries.remove(&key); + *changed = true; + } + } + _ => {} + } + Ok(true) + } + + /// [`scrub`](Self::scrub) for an `Any`: its type URL is checked like a + /// top-level detail's, a DebugInfo is reported for dropping, and a packed + /// message of a known type is scrubbed and re-packed if it changed. + fn scrub_any(&self, any: &mut DynamicMessage) -> Result, MalformedStatus> { + let type_url = match any.get_field_by_name("type_url").as_deref() { + Some(PbValue::String(url)) => url.clone(), + _ => String::new(), + }; + let Some(type_name) = any_type_name(&type_url) else { + tracing::error!(%type_url, "packed Any with a malformed type URL"); + return Err(MalformedStatus); + }; + if type_name == DEBUG_INFO { + return Ok(None); + } + // An unknown packed type has no JSON form; serialization reports it. + let Some(desc) = self.resolve(type_name) else { + return Ok(Some(false)); + }; + let bytes = match any.get_field_by_name("value").as_deref() { + Some(PbValue::Bytes(bytes)) => bytes.clone(), + _ => Default::default(), + }; + let mut inner = DynamicMessage::decode(desc, bytes).map_err(|e| { + tracing::error!(detail = %type_name, "undecodable packed Any: {e}"); + MalformedStatus + })?; + match self.scrub(&mut inner)? { + None => Ok(None), + Some(false) => Ok(Some(false)), + Some(true) => { + any.set_field_by_name("value", PbValue::Bytes(inner.encode_to_vec().into())); + Ok(Some(true)) + } + } } } + +#[cfg(test)] +mod tests; diff --git a/src/transcode/error/tests.rs b/src/transcode/error/tests.rs new file mode 100644 index 0000000..b82b0a0 --- /dev/null +++ b/src/transcode/error/tests.rs @@ -0,0 +1,1151 @@ +use super::*; + +use serde_json::json; +use tonic_types::{BadRequest, DebugInfo, ErrorDetail, ErrorInfo, FieldViolation, StatusExt}; + +#[test] +fn test_grpc_to_http_mapping() { + assert_eq!(grpc_to_http_status(tonic::Code::Ok), StatusCode::OK); + assert_eq!( + grpc_to_http_status(tonic::Code::InvalidArgument), + StatusCode::BAD_REQUEST + ); + assert_eq!( + grpc_to_http_status(tonic::Code::NotFound), + StatusCode::NOT_FOUND + ); + assert_eq!( + grpc_to_http_status(tonic::Code::AlreadyExists), + StatusCode::CONFLICT + ); + assert_eq!( + grpc_to_http_status(tonic::Code::PermissionDenied), + StatusCode::FORBIDDEN + ); + assert_eq!( + grpc_to_http_status(tonic::Code::Unauthenticated), + StatusCode::UNAUTHORIZED + ); + assert_eq!( + grpc_to_http_status(tonic::Code::ResourceExhausted), + StatusCode::TOO_MANY_REQUESTS + ); + assert_eq!( + grpc_to_http_status(tonic::Code::Unimplemented), + StatusCode::NOT_IMPLEMENTED + ); + assert_eq!( + grpc_to_http_status(tonic::Code::Internal), + StatusCode::INTERNAL_SERVER_ERROR + ); + assert_eq!( + grpc_to_http_status(tonic::Code::Unavailable), + StatusCode::SERVICE_UNAVAILABLE + ); + assert_eq!( + grpc_to_http_status(tonic::Code::DeadlineExceeded), + StatusCode::GATEWAY_TIMEOUT + ); +} + +#[test] +fn test_grpc_code_name() { + assert_eq!(grpc_code_name(tonic::Code::Ok), "OK"); + assert_eq!(grpc_code_name(tonic::Code::NotFound), "NOT_FOUND"); + assert_eq!( + grpc_code_name(tonic::Code::Unauthenticated), + "UNAUTHENTICATED" + ); +} + +#[test] +fn test_status_to_response() { + let status = tonic::Status::not_found("user not found"); + let response = status_to_response(status); + assert_eq!(response.status(), StatusCode::NOT_FOUND); +} + +// --- fixtures ------------------------------------------------------------- + +/// A renderer over an empty product pool: only the canonical google.rpc +/// descriptors resolve. +fn canonical_only() -> StatusDetails { + StatusDetails::new(&DescriptorPool::new()) +} + +/// [`canonical_only`] with the opaque-detail extension switched on. +fn canonical_with_opaque() -> StatusDetails { + canonical_only().with_opaque_details(true) +} + +/// An `INVALID_ARGUMENT` status carrying `ErrorInfo` + `BadRequest`, the shape +/// the issue's acceptance criterion names. +fn rich_status() -> tonic::Status { + tonic::Status::with_error_details_vec( + tonic::Code::InvalidArgument, + "invalid email", + [ + ErrorDetail::from(ErrorInfo::new( + "EMAIL_TAKEN", + "identity.example.com", + [("email".to_string(), "a@b.c".to_string())] + .into_iter() + .collect::>(), + )), + ErrorDetail::from(BadRequest::new(vec![FieldViolation::new( + "email", + "already registered", + )])), + ], + ) +} + +/// A status whose trailer holds the given raw `(type_url, value)` details, +/// for types tonic-types has no builder for. +fn status_with_raw_details(details: &[(&str, Vec)]) -> tonic::Status { + let mut rpc = tonic_types::pb::Status { + code: tonic::Code::FailedPrecondition as i32, + message: "raw".into(), + ..Default::default() + }; + for (type_url, value) in details { + rpc.details.push(Default::default()); + let any = rpc.details.last_mut().expect("just pushed"); + any.type_url = (*type_url).to_string(); + any.value = value.clone(); + } + tonic::Status::with_details( + tonic::Code::FailedPrecondition, + "raw", + bytes::Bytes::from(rpc.encode_to_vec()), + ) +} + +/// A product pool compiled from one in-memory `.proto` source. +fn product_pool(name: &str, source: &str) -> DescriptorPool { + struct OneFile { + name: String, + source: String, + } + impl protox::file::FileResolver for OneFile { + fn open_file(&self, name: &str) -> Result { + if name == self.name { + protox::file::File::from_source(name, &self.source) + } else { + // google/protobuf/*.proto for imports such as any.proto. + protox::file::GoogleFileResolver::new().open_file(name) + } + } + } + protox::Compiler::with_file_resolver(OneFile { + name: name.to_owned(), + source: source.to_owned(), + }) + .open_file(name) + .expect("test proto compiles") + .descriptor_pool() +} + +// --- body shape ------------------------------------------------------------- + +#[test] +fn body_without_renderer_keeps_the_details_key_absent() { + // A route with details switched off must answer with exactly the + // pre-existing shape: no `details` key at all, not an empty array, so the + // off switch is observable and nothing about the upstream leaks. + let body = error_body(&rich_status(), None); + assert_eq!( + body, + json!({"error": "INVALID_ARGUMENT", "message": "invalid email", "code": 3}) + ); +} + +#[test] +fn body_without_trailer_has_empty_details() { + // No `grpc-status-details-bin` from the upstream: the enabled shape still + // carries `details`, as an empty array, so clients never branch on presence. + let status = tonic::Status::not_found("user not found"); + let body = error_body(&status, Some(&canonical_only())); + assert_eq!( + body, + json!({"error": "NOT_FOUND", "message": "user not found", "code": 5, "details": []}) + ); +} + +#[test] +fn error_info_and_bad_request_render_as_canonical_json() { + // The two most common details come out in the ProtoJSON form of `Any`: + // `@type` plus the message fields in lowerCamelCase, unset fields omitted. + let body = error_body(&rich_status(), Some(&canonical_only())); + assert_eq!( + body, + json!({ + "error": "INVALID_ARGUMENT", + "message": "invalid email", + "code": 3, + "details": [ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "EMAIL_TAKEN", + "domain": "identity.example.com", + "metadata": {"email": "a@b.c"} + }, + { + "@type": "type.googleapis.com/google.rpc.BadRequest", + "fieldViolations": [ + {"field": "email", "description": "already registered"} + ] + } + ] + }) + ); +} + +#[test] +fn status_to_response_keeps_http_mapping_with_details() { + // Rendering details must not change the HTTP status chosen by the + // gRPC → HTTP mapping. + let resp = status_to_response_with_details(&rich_status(), Some(&canonical_only())); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); +} + +#[test] +fn debug_info_is_never_rendered() { + // DebugInfo carries stack traces for operators; it is dropped while the + // details around it still render, in their original order. + let status = tonic::Status::with_error_details_vec( + tonic::Code::Internal, + "boom", + [ + ErrorDetail::from(ErrorInfo::new( + "DB_DOWN", + "store.example.com", + std::collections::HashMap::new(), + )), + ErrorDetail::from(DebugInfo::new( + vec!["at store::write (store.rs:42)".to_string()], + "connection refused to 10.0.0.7:5432", + )), + ], + ); + let details = canonical_only().render(&status).unwrap().details; + assert_eq!( + details, + vec![json!({ + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "DB_DOWN", + "domain": "store.example.com" + })] + ); + let text = serde_json::to_string(&details).unwrap(); + assert!(!text.contains("store.rs:42") && !text.contains("10.0.0.7")); +} + +#[test] +fn debug_info_is_dropped_under_any_type_url_prefix() { + // The type is identified by the last URL segment, so a non-default prefix + // cannot smuggle DebugInfo past the filter. + let debug = tonic_types::pb::DebugInfo { + stack_entries: vec!["secret frame".into()], + detail: "secret".into(), + }; + let status = status_with_raw_details(&[( + "example.com/types/google.rpc.DebugInfo", + debug.encode_to_vec(), + )]); + let rendered = canonical_only().render(&status).unwrap(); + assert!(rendered.details.is_empty() && rendered.opaque.is_empty()); +} + +#[test] +fn unknown_detail_type_is_withheld_by_default() { + // Bytes of a type no descriptor describes cannot be inspected, so they may + // carry a DebugInfo in a field the proxy cannot see. Unless the extension + // is switched on, such a detail is withheld and `opaqueDetails` is absent. + let debug = tonic_types::pb::DebugInfo { + detail: "password=hunter2".into(), + ..Default::default() + }; + let status = + status_with_raw_details(&[("type.googleapis.com/acme.v1.Wrapper", debug.encode_to_vec())]); + assert_eq!( + error_body(&status, Some(&canonical_only())), + json!({"error": "FAILED_PRECONDITION", "message": "raw", "code": 9, "details": []}) + ); +} + +#[test] +fn unknown_detail_type_goes_to_opaque_details() { + // With the extension on, a type neither pool knows (it has no ProtoJSON + // form) is kept, with its original type URL and bytes, in `opaqueDetails` + // next to `details`, never inside `details` and never under `@type`, so no + // client can take it for a message of that type. + let status = status_with_raw_details(&[( + "type.googleapis.com/acme.v1.Unknown", + vec![0x08, 0x96, 0x01], + )]); + assert_eq!( + error_body(&status, Some(&canonical_with_opaque())), + json!({ + "error": "FAILED_PRECONDITION", + "message": "raw", + "code": 9, + "details": [], + "opaqueDetails": [{ + "index": 0, + "typeUrl": "type.googleapis.com/acme.v1.Unknown", + "bytes": "CJYB" + }] + }) + ); +} + +#[test] +fn opaque_index_is_the_position_among_forwarded_details() { + // Upstream order: ErrorInfo, DebugInfo, unknown, ErrorInfo. DebugInfo is + // not forwarded and leaves no gap, so merging `details` and + // `opaqueDetails` by `index` restores the order the client may see: + // ErrorInfo (0), unknown (1), ErrorInfo (2). + let info = |reason: &str| { + tonic_types::pb::ErrorInfo { + reason: reason.into(), + ..Default::default() + } + .encode_to_vec() + }; + let debug = tonic_types::pb::DebugInfo { + detail: "secret".into(), + ..Default::default() + }; + let status = status_with_raw_details(&[ + ("type.googleapis.com/google.rpc.ErrorInfo", info("FIRST")), + ( + "type.googleapis.com/google.rpc.DebugInfo", + debug.encode_to_vec(), + ), + ("type.googleapis.com/acme.v1.Unknown", vec![0x08, 0x01]), + ("type.googleapis.com/google.rpc.ErrorInfo", info("LAST")), + ]); + let body = error_body(&status, Some(&canonical_with_opaque())); + assert_eq!( + body["details"], + json!([ + {"@type": "type.googleapis.com/google.rpc.ErrorInfo", "reason": "FIRST"}, + {"@type": "type.googleapis.com/google.rpc.ErrorInfo", "reason": "LAST"} + ]) + ); + assert_eq!( + body["opaqueDetails"], + json!([{"index": 1, "typeUrl": "type.googleapis.com/acme.v1.Unknown", "bytes": "CAE="}]) + ); +} + +#[test] +fn object_valued_well_known_details_go_under_value() { + // ProtoJSON wraps every well-known type with a special JSON representation + // under `value`, decided by the type, not by the shape of its JSON: a + // Struct is a JSON object yet still goes under `value`, and its own + // `@type` key cannot overwrite the detail's. + use prost_reflect::prost_types::{value::Kind, Struct, Value as PbValue}; + let mut fields = std::collections::BTreeMap::new(); + fields.insert( + "@type".to_string(), + PbValue { + kind: Some(Kind::StringValue("forged".into())), + }, + ); + fields.insert( + "retry".to_string(), + PbValue { + kind: Some(Kind::BoolValue(true)), + }, + ); + let status = status_with_raw_details(&[ + ( + "type.googleapis.com/google.protobuf.Struct", + Struct { fields }.encode_to_vec(), + ), + ("type.googleapis.com/google.protobuf.Empty", Vec::new()), + ]); + assert_eq!( + canonical_only().render(&status).unwrap().details, + vec![ + json!({ + "@type": "type.googleapis.com/google.protobuf.Struct", + "value": {"@type": "forged", "retry": true} + }), + json!({"@type": "type.googleapis.com/google.protobuf.Empty", "value": {}}), + ] + ); +} + +#[test] +fn debug_info_behind_a_malformed_type_url_is_not_forwarded() { + // A type URL whose name part is empty or not a protobuf full name cannot + // be told apart from a disguised DebugInfo, so it fails the error safely + // instead of shipping its bytes as an opaque detail. + let debug = tonic_types::pb::DebugInfo { + detail: "connection refused to 10.0.0.7:5432".into(), + ..Default::default() + }; + for type_url in [ + "type.googleapis.com/google.rpc.DebugInfo/", + "type.googleapis.com/google.rpc.DebugInfo?v=1", + "", + "type.googleapis.com/google..rpc.DebugInfo", + ] { + let status = status_with_raw_details(&[(type_url, debug.encode_to_vec())]); + let body = error_body(&status, Some(&canonical_only())); + assert_eq!(body, malformed_upstream_status_body(), "{type_url:?}"); + } +} + +#[test] +fn trailer_code_or_message_disagreeing_with_the_status_fails_safely() { + // The rich status in the trailer must describe the same error as + // grpc-status / grpc-message; otherwise its details would be attached to + // an error they were not written for. + let details = canonical_only(); + for (code, message) in [ + (tonic::Code::NotFound, "raw"), + (tonic::Code::FailedPrecondition, "something else"), + ] { + let rpc = tonic_types::pb::Status { + code: code as i32, + message: message.into(), + ..Default::default() + }; + let status = tonic::Status::with_details( + tonic::Code::FailedPrecondition, + "raw", + bytes::Bytes::from(rpc.encode_to_vec()), + ); + assert_eq!( + error_body(&status, Some(&details)), + malformed_upstream_status_body(), + "{code:?} {message:?}" + ); + } +} + +/// `acme.v1.Wrapper { Any cause = 1; repeated Any causes = 2; }`, a product +/// detail that imports any.proto but not google/rpc. +fn wrapper_pool() -> DescriptorPool { + product_pool( + "acme.proto", + "syntax = \"proto3\"; package acme.v1; import \"google/protobuf/any.proto\"; \ + message Wrapper { google.protobuf.Any cause = 1; repeated google.protobuf.Any causes = 2; }", + ) +} + +/// An `Any` packing `message` under `type_url`, as a reflected value. +fn packed(pool: &DescriptorPool, type_url: &str, message: Vec) -> prost_reflect::Value { + let mut any = DynamicMessage::new(pool.get_message_by_name("google.protobuf.Any").unwrap()); + any.set_field_by_name("type_url", prost_reflect::Value::String(type_url.into())); + any.set_field_by_name("value", prost_reflect::Value::Bytes(message.into())); + prost_reflect::Value::Message(any) +} + +fn secret_debug_info() -> Vec { + tonic_types::pb::DebugInfo { + stack_entries: vec!["at db::write (db.rs:7)".into()], + detail: "password=hunter2".into(), + } + .encode_to_vec() +} + +fn error_info(reason: &str) -> Vec { + tonic_types::pb::ErrorInfo { + reason: reason.into(), + ..Default::default() + } + .encode_to_vec() +} + +#[test] +fn nested_canonical_type_resolves_inside_a_product_detail() { + // The product descriptors know Wrapper but not google.rpc.ErrorInfo; the + // ErrorInfo packed inside it must still render from the canonical + // descriptors instead of failing the whole error. + let pool = wrapper_pool(); + let mut wrapper = DynamicMessage::new(pool.get_message_by_name("acme.v1.Wrapper").unwrap()); + wrapper.set_field_by_name( + "cause", + packed( + &pool, + "type.googleapis.com/google.rpc.ErrorInfo", + error_info("NESTED"), + ), + ); + let status = status_with_raw_details(&[( + "type.googleapis.com/acme.v1.Wrapper", + wrapper.encode_to_vec(), + )]); + assert_eq!( + StatusDetails::new(&pool).render(&status).unwrap().details, + vec![json!({ + "@type": "type.googleapis.com/acme.v1.Wrapper", + "cause": {"@type": "type.googleapis.com/google.rpc.ErrorInfo", "reason": "NESTED"} + })] + ); +} + +#[test] +fn debug_info_nested_in_a_product_detail_is_removed() { + // DebugInfo packed in an Any field, singular or repeated, is withheld at + // any depth, while the rest of the detail stays. + let pool = wrapper_pool(); + let mut wrapper = DynamicMessage::new(pool.get_message_by_name("acme.v1.Wrapper").unwrap()); + wrapper.set_field_by_name( + "cause", + packed( + &pool, + "type.googleapis.com/google.rpc.DebugInfo", + secret_debug_info(), + ), + ); + wrapper.set_field_by_name( + "causes", + prost_reflect::Value::List(vec![ + packed( + &pool, + "type.googleapis.com/google.rpc.ErrorInfo", + error_info("KEPT"), + ), + packed( + &pool, + "type.googleapis.com/google.rpc.DebugInfo", + secret_debug_info(), + ), + ]), + ); + let status = status_with_raw_details(&[( + "type.googleapis.com/acme.v1.Wrapper", + wrapper.encode_to_vec(), + )]); + let body = error_body(&status, Some(&StatusDetails::new(&pool))); + assert_eq!( + body["details"], + json!([{ + "@type": "type.googleapis.com/acme.v1.Wrapper", + "causes": [{"@type": "type.googleapis.com/google.rpc.ErrorInfo", "reason": "KEPT"}] + }]) + ); + let text = body.to_string(); + assert!( + !text.contains("hunter2") && !text.contains("db.rs"), + "{text}" + ); +} + +#[test] +fn debug_info_packed_in_an_any_detail_is_dropped() { + // A detail that is itself an Any wrapping DebugInfo is dropped like a + // direct DebugInfo and takes no index. + let pool = wrapper_pool(); + let DynamicMessageValue(any) = DynamicMessageValue::from(packed( + &pool, + "type.googleapis.com/google.rpc.DebugInfo", + secret_debug_info(), + )); + let status = status_with_raw_details(&[ + ( + "type.googleapis.com/google.protobuf.Any", + any.encode_to_vec(), + ), + ("type.googleapis.com/acme.v1.Unknown", vec![0x08, 0x01]), + ]); + let renderer = StatusDetails::new(&pool).with_opaque_details(true); + let body = error_body(&status, Some(&renderer)); + assert_eq!(body["details"], json!([])); + assert_eq!(body["opaqueDetails"][0]["index"], 0); + let text = body.to_string(); + assert!( + !text.contains("hunter2") && !text.contains("db.rs"), + "{text}" + ); +} + +fn bad_request(field: &str) -> Vec { + tonic_types::pb::BadRequest { + field_violations: vec![tonic_types::pb::bad_request::FieldViolation { + field: field.into(), + description: "bad".into(), + ..Default::default() + }], + } + .encode_to_vec() +} + +#[test] +fn canonical_types_stay_available_when_the_product_defines_one_elsewhere() { + // The product defines its own google.rpc.ErrorInfo in a file of another + // name. That revision wins, and the rest of the canonical + // error_details.proto (BadRequest here) still renders typed instead of + // falling back to opaque bytes. + let pool = product_pool( + "acme/errors.proto", + "syntax = \"proto3\"; package google.rpc; message ErrorInfo { string reason = 1; string tenant = 9; }", + ); + let desc = pool.get_message_by_name("google.rpc.ErrorInfo").unwrap(); + let mut info = DynamicMessage::new(desc); + info.set_field_by_name("tenant", prost_reflect::Value::String("t-7".into())); + let status = status_with_raw_details(&[ + ( + "type.googleapis.com/google.rpc.ErrorInfo", + info.encode_to_vec(), + ), + ( + "type.googleapis.com/google.rpc.BadRequest", + bad_request("email"), + ), + ]); + let body = error_body(&status, Some(&StatusDetails::new(&pool))); + assert_eq!( + body["details"], + json!([ + {"@type": "type.googleapis.com/google.rpc.ErrorInfo", "tenant": "t-7"}, + { + "@type": "type.googleapis.com/google.rpc.BadRequest", + "fieldViolations": [{"field": "email", "description": "bad"}] + } + ]) + ); + assert!(body.get("opaqueDetails").is_none(), "{body}"); +} + +#[test] +fn canonical_types_missing_from_a_shadowed_file_stay_available() { + // The product ships its own google/rpc/error_details.proto with only a + // revised ErrorInfo; BadRequest from the canonical file must still + // resolve. + let pool = product_pool( + "google/rpc/error_details.proto", + "syntax = \"proto3\"; package google.rpc; message ErrorInfo { string reason = 1; string tenant = 9; }", + ); + let status = status_with_raw_details(&[( + "type.googleapis.com/google.rpc.BadRequest", + bad_request("name"), + )]); + let body = error_body(&status, Some(&StatusDetails::new(&pool))); + assert_eq!( + body["details"], + json!([{ + "@type": "type.googleapis.com/google.rpc.BadRequest", + "fieldViolations": [{"field": "name", "description": "bad"}] + }]) + ); +} + +#[test] +fn type_url_without_a_slash_fails_safely() { + // Any.type_url must contain a `/`; a bare type name is not a valid Any. + let status = status_with_raw_details(&[("google.rpc.ErrorInfo", error_info("R"))]); + assert_eq!( + error_body(&status, Some(&canonical_only())), + malformed_upstream_status_body() + ); +} + +#[test] +fn field_json_named_at_type_fails_safely() { + // A product field whose JSON name is `@type` would overwrite the Any's + // own type URL when its fields are placed next to it. + let pool = product_pool( + "acme.proto", + "syntax = \"proto3\"; package acme.v1; message Tagged { string kind = 1 [json_name = \"@type\"]; }", + ); + let desc = pool.get_message_by_name("acme.v1.Tagged").unwrap(); + let mut tagged = DynamicMessage::new(desc); + tagged.set_field_by_name( + "kind", + prost_reflect::Value::String("type.googleapis.com/google.rpc.ErrorInfo".into()), + ); + let status = + status_with_raw_details(&[("type.googleapis.com/acme.v1.Tagged", tagged.encode_to_vec())]); + assert_eq!( + error_body(&status, Some(&StatusDetails::new(&pool))), + malformed_upstream_status_body() + ); +} + +#[test] +fn struct_keys_that_look_like_debug_info_are_kept() { + // DebugInfo is withheld where an Any packs it. A Struct is data: a key + // `@type` naming DebugInfo is just a string there, and the Struct must + // stay whole and validly wrapped. + use prost_reflect::prost_types::{value::Kind, Struct, Value as PbValue}; + let mut fields = std::collections::BTreeMap::new(); + fields.insert( + "@type".to_string(), + PbValue { + kind: Some(Kind::StringValue( + "type.googleapis.com/google.rpc.DebugInfo".into(), + )), + }, + ); + fields.insert( + "note".to_string(), + PbValue { + kind: Some(Kind::StringValue("keep".into())), + }, + ); + let status = status_with_raw_details(&[( + "type.googleapis.com/google.protobuf.Struct", + Struct { fields }.encode_to_vec(), + )]); + assert_eq!( + canonical_only().render(&status).unwrap().details, + vec![json!({ + "@type": "type.googleapis.com/google.protobuf.Struct", + "value": {"@type": "type.googleapis.com/google.rpc.DebugInfo", "note": "keep"} + })] + ); +} + +#[test] +fn debug_info_typed_fields_are_removed() { + // A field typed as google.rpc.DebugInfo itself (not packed in an Any), + // singular, repeated or as a map value, is withheld like a packed one, + // while the rest of the message stays. + let pool = product_pool( + "google/rpc/debug_holder.proto", + "syntax = \"proto3\"; package google.rpc; \ + message DebugInfo { repeated string stack_entries = 1; string detail = 2; } \ + message Holder { DebugInfo one = 1; repeated DebugInfo many = 2; \ + map by_key = 3; string keep = 4; }", + ); + let debug_desc = pool.get_message_by_name("google.rpc.DebugInfo").unwrap(); + let secret = || { + let mut debug = DynamicMessage::new(debug_desc.clone()); + debug.set_field_by_name( + "detail", + prost_reflect::Value::String("password=hunter2".into()), + ); + prost_reflect::Value::Message(debug) + }; + let holder_desc = pool.get_message_by_name("google.rpc.Holder").unwrap(); + let mut holder = DynamicMessage::new(holder_desc); + holder.set_field_by_name("one", secret()); + holder.set_field_by_name("many", prost_reflect::Value::List(vec![secret(), secret()])); + let mut by_key = std::collections::HashMap::new(); + by_key.insert(prost_reflect::MapKey::String("k".into()), secret()); + holder.set_field_by_name("by_key", prost_reflect::Value::Map(by_key)); + holder.set_field_by_name("keep", prost_reflect::Value::String("kept".into())); + let status = status_with_raw_details(&[( + "type.googleapis.com/google.rpc.Holder", + holder.encode_to_vec(), + )]); + let body = error_body(&status, Some(&StatusDetails::new(&pool))); + assert_eq!( + body["details"], + json!([{"@type": "type.googleapis.com/google.rpc.Holder", "keep": "kept"}]) + ); + assert!(!body.to_string().contains("hunter2"), "{body}"); +} + +#[test] +fn debug_info_extensions_are_removed() { + // A proto2 extension typed as DebugInfo is not one of the message's own + // fields, yet ProtoJSON renders it as `[google.rpc.debug]`; it is withheld + // like a regular field while the rest of the message stays. + let pool = product_pool( + "google/rpc/debug_extension.proto", + "syntax = \"proto2\"; package google.rpc; \ + message DebugInfo { repeated string stack_entries = 1; optional string detail = 2; } \ + message Extendable { optional string keep = 1; extensions 100 to 199; } \ + extend Extendable { optional DebugInfo debug = 100; }", + ); + let mut debug = DynamicMessage::new(pool.get_message_by_name("google.rpc.DebugInfo").unwrap()); + debug.set_field_by_name( + "detail", + prost_reflect::Value::String("password=hunter2".into()), + ); + let mut extendable = + DynamicMessage::new(pool.get_message_by_name("google.rpc.Extendable").unwrap()); + extendable.set_field_by_name("keep", prost_reflect::Value::String("kept".into())); + extendable.set_extension( + &pool.get_extension_by_name("google.rpc.debug").unwrap(), + prost_reflect::Value::Message(debug), + ); + let status = status_with_raw_details(&[( + "type.googleapis.com/google.rpc.Extendable", + extendable.encode_to_vec(), + )]); + let body = error_body(&status, Some(&StatusDetails::new(&pool))); + assert_eq!( + body["details"], + json!([{"@type": "type.googleapis.com/google.rpc.Extendable", "keep": "kept"}]) + ); + assert!(!body.to_string().contains("hunter2"), "{body}"); +} + +#[test] +fn detail_with_required_debug_info_field_is_withheld() { + // Clearing a proto2 `required` DebugInfo field would forward a message + // that violates its own schema, so the whole detail is withheld instead, + // and it takes no opaque index, like a packed DebugInfo. + let pool = product_pool( + "google/rpc/debug_required.proto", + "syntax = \"proto2\"; package google.rpc; \ + message DebugInfo { repeated string stack_entries = 1; optional string detail = 2; } \ + message Wrapper { required DebugInfo info = 1; optional string keep = 2; } \ + message Outer { optional Wrapper wrapper = 1; optional string keep = 2; }", + ); + let debug_desc = pool.get_message_by_name("google.rpc.DebugInfo").unwrap(); + let wrapper_desc = pool.get_message_by_name("google.rpc.Wrapper").unwrap(); + let wrapper = || { + let mut debug = DynamicMessage::new(debug_desc.clone()); + debug.set_field_by_name( + "detail", + prost_reflect::Value::String("password=hunter2".into()), + ); + let mut wrapper = DynamicMessage::new(wrapper_desc.clone()); + wrapper.set_field_by_name("info", prost_reflect::Value::Message(debug)); + wrapper.set_field_by_name("keep", prost_reflect::Value::String("kept".into())); + wrapper + }; + let mut outer = DynamicMessage::new(pool.get_message_by_name("google.rpc.Outer").unwrap()); + outer.set_field_by_name("wrapper", prost_reflect::Value::Message(wrapper())); + outer.set_field_by_name("keep", prost_reflect::Value::String("outer".into())); + let status = status_with_raw_details(&[ + ( + "type.googleapis.com/google.rpc.Wrapper", + wrapper().encode_to_vec(), + ), + ( + "type.googleapis.com/google.rpc.Outer", + outer.encode_to_vec(), + ), + ("type.googleapis.com/acme.Unknown", vec![1, 2, 3]), + ]); + let renderer = StatusDetails::new(&pool).with_opaque_details(true); + let body = error_body(&status, Some(&renderer)); + // The top-level Wrapper is withheld; inside Outer only the optional field + // holding the Wrapper goes. + assert_eq!( + body["details"], + json!([{"@type": "type.googleapis.com/google.rpc.Outer", "keep": "outer"}]) + ); + assert_eq!(body["opaqueDetails"][0]["index"], json!(1)); + assert!(!body.to_string().contains("hunter2"), "{body}"); +} + +#[test] +fn detail_missing_a_required_field_is_malformed() { + // A proto2 detail whose payload lacks a `required` field decodes, but + // has no valid ProtoJSON form; directly or nested, it makes the status + // malformed instead of being forwarded without the field. + let pool = product_pool( + "acme/required.proto", + "syntax = \"proto2\"; package acme; \ + message Strict { required string id = 1; optional string note = 2; } \ + message Holder { optional Strict strict = 1; }", + ); + let mut strict = DynamicMessage::new(pool.get_message_by_name("acme.Strict").unwrap()); + strict.set_field_by_name("note", prost_reflect::Value::String("no id".into())); + let mut holder = DynamicMessage::new(pool.get_message_by_name("acme.Holder").unwrap()); + holder.set_field_by_name("strict", prost_reflect::Value::Message(strict.clone())); + for (type_url, bytes) in [ + ("type.googleapis.com/acme.Strict", strict.encode_to_vec()), + ("type.googleapis.com/acme.Holder", holder.encode_to_vec()), + ] { + let status = status_with_raw_details(&[(type_url, bytes)]); + let body = error_body(&status, Some(&StatusDetails::new(&pool))); + assert_eq!(body, malformed_upstream_status_body(), "{type_url}"); + } +} + +/// Unwraps the message out of a reflected value built by [`packed`]. +struct DynamicMessageValue(DynamicMessage); + +impl From for DynamicMessageValue { + fn from(value: prost_reflect::Value) -> Self { + match value { + prost_reflect::Value::Message(message) => Self(message), + other => panic!("expected a message, got {other:?}"), + } + } +} + +#[test] +fn opaque_details_key_is_absent_when_every_type_resolves() { + let body = error_body(&rich_status(), Some(&canonical_only())); + assert!(body.get("opaqueDetails").is_none(), "{body}"); +} + +/// The body a failed call gets when the upstream's error status itself cannot +/// be rendered faithfully: a generic INTERNAL, with no decoder diagnostics. +fn malformed_upstream_status_body() -> Value { + json!({ + "error": "INTERNAL", + "message": "upstream returned a malformed error status", + "code": 13, + "details": [] + }) +} + +#[test] +fn corrupt_known_detail_fails_the_error_safely() { + // The type resolves but its bytes do not decode (a truncated + // length-delimited field). That is a broken upstream response, not an + // unknown type: the whole error becomes a safe INTERNAL instead of the + // detail being passed on as opaque bytes, dropped, or half-rendered next + // to the valid one. + let valid = tonic_types::pb::ErrorInfo { + reason: "OK_ONE".into(), + ..Default::default() + }; + let status = status_with_raw_details(&[ + ( + "type.googleapis.com/google.rpc.ErrorInfo", + valid.encode_to_vec(), + ), + ( + "type.googleapis.com/google.rpc.ErrorInfo", + vec![0x0a, 0x05, b'a'], + ), + ]); + let details = canonical_only(); + assert_eq!( + error_body(&status, Some(&details)), + malformed_upstream_status_body() + ); + let resp = status_to_response_with_details(&status, Some(&details)); + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); +} + +#[test] +fn corrupt_well_known_detail_is_not_rendered_as_that_type() { + // A Duration whose bytes are truncated must not come out as a Duration + // (nor as base64 under the same `value` key a real Duration uses). + let status = + status_with_raw_details(&[("type.googleapis.com/google.protobuf.Duration", vec![0x08])]); + assert_eq!( + error_body(&status, Some(&canonical_only())), + malformed_upstream_status_body() + ); +} + +#[test] +fn out_of_range_well_known_value_fails_the_error_safely() { + // Well-formed bytes carrying a value the type forbids: Duration is limited + // to ±315,576,000,000 seconds, so this one has no valid JSON form. + let duration = prost_reflect::prost_types::Duration { + seconds: 400_000_000_000, + nanos: 0, + }; + let status = status_with_raw_details(&[( + "type.googleapis.com/google.protobuf.Duration", + duration.encode_to_vec(), + )]); + assert_eq!( + error_body(&status, Some(&canonical_only())), + malformed_upstream_status_body() + ); +} + +#[test] +fn well_known_type_detail_goes_under_value() { + // A well-known type with a special JSON representation (Duration is + // "1.500s") sits under `value`, which is ProtoJSON for `Any`; unlike the + // opaque extension, `value` here holds that JSON, not base64 bytes. + let duration = prost_reflect::prost_types::Duration { + seconds: 1, + nanos: 500_000_000, + }; + let status = status_with_raw_details(&[( + "type.googleapis.com/google.protobuf.Duration", + duration.encode_to_vec(), + )]); + assert_eq!( + canonical_only().render(&status).unwrap().details, + vec![json!({"@type": "type.googleapis.com/google.protobuf.Duration", "value": "1.500s"})] + ); +} + +#[test] +fn product_defined_detail_type_renders_its_fields() { + // A service's own detail message resolves through the product descriptors. + let pool = product_pool( + "acme.proto", + "syntax = \"proto3\"; package acme.v1; message QuotaTicket { string ticket = 1; int64 wait_ms = 2; }", + ); + let ticket = pool.get_message_by_name("acme.v1.QuotaTicket").unwrap(); + let mut msg = DynamicMessage::new(ticket); + msg.set_field_by_name("ticket", prost_reflect::Value::String("T-1".into())); + msg.set_field_by_name("wait_ms", prost_reflect::Value::I64(1500)); + let status = status_with_raw_details(&[( + "type.googleapis.com/acme.v1.QuotaTicket", + msg.encode_to_vec(), + )]); + assert_eq!( + StatusDetails::new(&pool).render(&status).unwrap().details, + vec![json!({ + "@type": "type.googleapis.com/acme.v1.QuotaTicket", + "ticket": "T-1", + "waitMs": "1500" + })] + ); +} + +#[test] +fn product_revision_of_a_canonical_type_wins() { + // When the product ships its own google.rpc revision, that definition is + // used: a field it adds (number 9 here) renders by its name instead of + // being lost as an unknown field of the canonical revision. + let pool = product_pool( + "google/rpc/error_details.proto", + "syntax = \"proto3\"; package google.rpc; message ErrorInfo { string reason = 1; string domain = 2; string tenant = 9; }", + ); + let desc = pool.get_message_by_name("google.rpc.ErrorInfo").unwrap(); + let mut msg = DynamicMessage::new(desc); + msg.set_field_by_name("reason", prost_reflect::Value::String("R".into())); + msg.set_field_by_name("tenant", prost_reflect::Value::String("t-7".into())); + let status = status_with_raw_details(&[( + "type.googleapis.com/google.rpc.ErrorInfo", + msg.encode_to_vec(), + )]); + assert_eq!( + StatusDetails::new(&pool).render(&status).unwrap().details, + vec![json!({ + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "R", + "tenant": "t-7" + })] + ); +} + +#[test] +fn malformed_trailer_fails_the_error_safely() { + // A trailer that is not a google.rpc.Status is a broken upstream + // response; answering with the original code and an empty `details` would + // claim the upstream sent no details. + let status = tonic::Status::with_details( + tonic::Code::NotFound, + "boom", + bytes::Bytes::from_static(&[0x1a, 0xff]), + ); + let details = canonical_only(); + assert_eq!( + error_body(&status, Some(&details)), + malformed_upstream_status_body() + ); + let resp = status_to_response_with_details(&status, Some(&details)); + assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR); +} + +#[test] +fn details_off_leaves_a_malformed_trailer_unread() { + // With details switched off the trailer is never decoded, so it cannot + // fail the error: the upstream's own code and message pass through. + let status = tonic::Status::with_details( + tonic::Code::NotFound, + "boom", + bytes::Bytes::from_static(&[0x1a, 0xff]), + ); + assert_eq!( + error_body(&status, None), + json!({"error": "NOT_FOUND", "message": "boom", "code": 5}) + ); + assert_eq!( + status_to_response_with_details(&status, None).status(), + StatusCode::NOT_FOUND + ); +} + +// --- policy ----------------------------------------------------------------- + +fn policy(enabled: bool, routes: &[(&str, bool)]) -> Result { + let base = if enabled { + ErrorDetailsPolicy::default() + } else { + ErrorDetailsPolicy::disabled() + }; + routes.iter().try_fold(base, |policy, (pattern, enabled)| { + policy.route(pattern, *enabled) + }) +} + +#[test] +fn policy_defaults_to_enabled_everywhere() { + let p = ErrorDetailsPolicy::default(); + assert!(p.enabled_for("/v1/users/{id}")); + assert!(p.enabled_for("/anything")); +} + +#[test] +fn policy_global_off_disables_every_route() { + let p = policy(false, &[]).unwrap(); + assert!(!p.enabled_for("/v1/users/{id}")); +} + +#[test] +fn policy_route_rule_overrides_global_in_both_directions() { + // Global off with a sub-route switched back on, and a nested route inside + // that sub-route switched off again by an earlier rule. + let p = policy( + false, + &[("/v1/public/internal/*", false), ("/v1/public/**", true)], + ) + .unwrap(); + assert!(p.enabled_for("/v1/public/items/{id}")); + assert!(!p.enabled_for("/v1/public/internal/{id}")); + assert!(!p.enabled_for("/v1/admin/items")); +} + +#[test] +fn policy_first_matching_rule_wins() { + // Order is the contract: a broad rule listed first shadows a narrower + // one listed after it. + let p = policy(true, &[("/v1/**", false), ("/v1/public/**", true)]).unwrap(); + assert!(!p.enabled_for("/v1/public/items")); +} + +#[test] +fn policy_opaque_is_off_by_default_and_decided_per_switch() { + // The opaque-detail extension is off unless switched on. Each switch is + // decided by the first matching rule that sets it, so an `opaque` rule + // listed first does not shadow a later `enabled` rule, and vice versa. + assert!(!ErrorDetailsPolicy::default().opaque_for("/v1/users/{id}")); + assert!(!ErrorDetailsPolicy::disabled().opaque_for("/v1/users/{id}")); + let p = ErrorDetailsPolicy::default() + .opaque_route("/v1/partner/**", true) + .unwrap() + .route("/v1/partner/internal/**", false) + .unwrap(); + assert!(p.opaque_for("/v1/partner/orders")); + assert!(!p.enabled_for("/v1/partner/internal/x")); + assert!(p.enabled_for("/v1/partner/orders")); + assert!(!p.opaque_for("/v1/users/{id}")); + let p = ErrorDetailsPolicy::default() + .with_opaque_details(true) + .opaque_route("/v1/public/**", false) + .unwrap(); + assert!(p.opaque_for("/v1/users/{id}")); + assert!(!p.opaque_for("/v1/public/items")); +} + +#[test] +fn policy_star_matches_one_parameter_segment_only() { + // A path parameter is one segment: `*` matches `{id}` but not a deeper + // route under it, while `**` also matches an axum catch-all. + let p = policy(true, &[("/v1/users/*", false), ("/v1/files/**", false)]).unwrap(); + assert!(!p.enabled_for("/v1/users/{id}")); + assert!(p.enabled_for("/v1/users/{id}/keys")); + assert!(!p.enabled_for("/v1/files/{*path}")); +} + +#[test] +fn policy_rejects_relative_pattern() { + // `v1/admin/**` can never match a route path; a silent no-op would leave + // details on where the operator meant to turn them off. + let err = policy(true, &[("v1/admin/**", false)]).unwrap_err(); + assert!(err.contains("must start with '/'"), "{err}"); +} + +#[test] +fn policy_rejects_invalid_glob() { + let err = policy(true, &[("/v1/[admin", false)]).unwrap_err(); + assert!(err.contains("invalid glob pattern"), "{err}"); +} diff --git a/src/transcode/mod.rs b/src/transcode/mod.rs index 314446a..af02c70 100644 --- a/src/transcode/mod.rs +++ b/src/transcode/mod.rs @@ -19,9 +19,11 @@ use axum::routing::{delete, get, patch, post, put, MethodRouter}; use axum::{Json, Router}; use futures::StreamExt; use prost_reflect::{DescriptorPool, DynamicMessage, MethodDescriptor, SerializeOptions}; +use std::sync::Arc; use tonic::client::Grpc; use crate::config::AliasConfig; +use error::{ErrorDetailsPolicy, StatusDetails}; /// Trait for state types that support REST→gRPC transcoding. /// @@ -64,6 +66,54 @@ struct RouteEntry { body: request::BodyMapping, /// Optional response subfield to return as the HTTP body (`response_body`). response_body: Option, + /// Renderer for the status details of this route's errors; `None` when the + /// error-details policy switches them off for the route. + error_details: Option>, + /// Wrap NDJSON stream lines in `{"result"}` / `{"error"}` envelopes. + ndjson_envelope: bool, + /// [`codec::has_required_fields`] of the response type, computed once so a + /// request does not walk the descriptor. + response_has_required: bool, +} + +/// How [`routes_with_options`] builds the transcoded routes. +/// +/// # Examples +/// +/// ``` +/// use structured_proxy::transcode::error::ErrorDetailsPolicy; +/// use structured_proxy::transcode::TranscodeOptions; +/// +/// let options = TranscodeOptions::default() +/// .with_error_details(ErrorDetailsPolicy::default().route("/v1/admin/**", false).unwrap()) +/// .with_ndjson_envelope(true); +/// # let _ = options; +/// ``` +#[derive(Debug, Clone, Default)] +pub struct TranscodeOptions { + pub(crate) error_details: ErrorDetailsPolicy, + pub(crate) ndjson_envelope: bool, +} + +impl TranscodeOptions { + /// Which routes return `google.rpc.Status` details in their error bodies + /// (all of them by default). + pub fn with_error_details(mut self, policy: ErrorDetailsPolicy) -> Self { + self.error_details = policy; + self + } + + /// Wrap every NDJSON line of a server-streaming response in an envelope: + /// `{"result": }` for data, `{"error": }` for the + /// terminal error (the grpc-gateway stream shape). Off by default, when a + /// data line is the bare message and the error line carries an + /// `@type: google.rpc.Status` marker instead. Only the envelope keeps the + /// two apart for RPCs that stream `google.protobuf.Any`, `Struct`, `Value` + /// or `ListValue`, whose messages can carry any keys. SSE is unaffected. + pub fn with_ndjson_envelope(mut self, enabled: bool) -> Self { + self.ndjson_envelope = enabled; + self + } } #[derive(Debug, Clone, Copy)] @@ -91,8 +141,19 @@ impl HttpMethod { /// Build transcoded REST→gRPC routes from a descriptor pool. /// /// Takes a `DescriptorPool` and optional path aliases from config. -/// Returns an axum Router that transcodes REST requests to gRPC calls. +/// Returns an axum Router that transcodes REST requests to gRPC calls. Error +/// bodies carry `google.rpc.Status` details on every route; use +/// [`routes_with_options`] to choose per route or to frame NDJSON streams. pub fn routes(pool: &DescriptorPool, aliases: &[AliasConfig]) -> Router { + routes_with_options(pool, aliases, &TranscodeOptions::default()) +} + +/// [`routes`], built as `options` describe. +pub fn routes_with_options( + pool: &DescriptorPool, + aliases: &[AliasConfig], + options: &TranscodeOptions, +) -> Router { let bindings = route_bindings(pool, aliases); if bindings.is_empty() { tracing::warn!("No HTTP-annotated RPCs found in proto descriptors"); @@ -101,13 +162,37 @@ pub fn routes(pool: &DescriptorPool, aliases: &[AliasConfig]) tracing::info!("Registering {} transcoded REST→gRPC routes", bindings.len()); + // At most two renderers, without and with the opaque-detail extension, + // each shared by every route that uses it and built only when one does. + // The second is a copy of the first: the descriptor pool inside is shared. + let mut status_details: [Option>; 2] = [None, None]; let mut router: Router = Router::new(); - for binding in bindings { + for mut binding in bindings { + let policy = &options.error_details; + if policy.enabled_for(&binding.axum_path) { + let opaque = policy.opaque_for(&binding.axum_path); + let slot = usize::from(opaque); + if status_details[slot].is_none() { + let base = status_details + .iter() + .flatten() + .next() + .map(|existing| existing.as_ref().clone()) + .unwrap_or_else(|| StatusDetails::new(pool)); + status_details[slot] = Some(Arc::new(base.with_opaque_details(opaque))); + } + binding.entry.error_details = status_details[slot].clone(); + } + binding.entry.ndjson_envelope = options.ndjson_envelope; let method = binding.entry.http_method; - let entry = std::sync::Arc::new(binding.entry); + let entry = Arc::new(binding.entry); let method_router: MethodRouter = if binding.streaming { - let handler = move |proxy_state: State, headers: HeaderMap| { - streaming_handler(proxy_state, headers, entry) + let handler = move |proxy_state: State, + headers: HeaderMap, + path_params: Path>, + raw_query: RawQuery, + body: axum::body::Bytes| { + streaming_handler(proxy_state, headers, path_params, raw_query, body, entry) }; match method { HttpMethod::Get => get(handler), @@ -213,16 +298,6 @@ fn message_to_json_string(msg: &DynamicMessage, opts: &SerializeOptions) -> Resu serde_json::to_string(&value).map_err(|e| e.to_string()) } -/// Terminal error frame for a stream that failed mid-flight. Shared by the -/// NDJSON and SSE paths so a client sees the same shape in either format. -fn stream_error_json(status: &tonic::Status) -> serde_json::Value { - serde_json::json!({ - "error": error::grpc_code_name(status.code()), - "message": status.message(), - "code": status.code() as i32, - }) -} - /// Whether the client negotiated a Server-Sent Events response via `Accept`. /// /// Considers every `Accept` header line (a client may send more than one) and @@ -268,12 +343,23 @@ fn accept_range_selects_sse(range: &str) -> bool { async fn streaming_handler( State(proxy_state): State, headers: HeaderMap, + Path(path_params): Path>, + RawQuery(raw_query): RawQuery, + body_bytes: axum::body::Bytes, entry: std::sync::Arc, ) -> Response { let channel = proxy_state.grpc_channel(); - let input_desc = entry.method.input(); - let request_msg = DynamicMessage::new(input_desc); + let request_msg = match decode_request( + &entry, + &headers, + &path_params, + raw_query.as_deref(), + &body_bytes, + ) { + Ok(msg) => msg, + Err(message) => return bad_request(&entry, message), + }; let grpc_metadata = metadata::http_headers_to_grpc_metadata(&headers, proxy_state.forwarded_headers()); @@ -282,19 +368,14 @@ async fn streaming_handler( metadata::apply_request_deadline(&mut grpc_request, &headers); let output_desc = entry.method.output(); - let grpc_codec = codec::DynamicCodec::new(output_desc.clone()); + let grpc_codec = + codec::DynamicCodec::with_required_check(output_desc.clone(), entry.response_has_required); let grpc_path = entry.grpc_path.clone(); let mut grpc_client = Grpc::new(channel); if let Err(e) = grpc_client.ready().await { - return ( - StatusCode::SERVICE_UNAVAILABLE, - Json(serde_json::json!({ - "error": "UNAVAILABLE", - "message": format!("gRPC upstream not ready: {e}"), - })), - ) - .into_response(); + let status = tonic::Status::unavailable(format!("gRPC upstream not ready: {e}")); + return error::status_to_response_with_details(&status, entry.error_details.as_deref()); } let use_sse = wants_sse(&headers); @@ -303,76 +384,115 @@ async fn streaming_handler( .server_streaming(grpc_request, grpc_path, grpc_codec) .await { + // Only a trailers-only rejection lands in `Err`. Once the upstream + // accepted the call, the response starts at once rather than waiting + // for the first item, so headers and SSE keep-alives are not held back; + // an error that comes before the first message is a terminal frame. Ok(response) => { let stream = response.into_inner(); + // The terminal frame renders like the unary error body. The + // closure takes over this request's route handle, so the stream + // keeps it alive without another refcount. + let envelope = entry.ndjson_envelope; + let render_error = move |status: &tonic::Status| { + error::error_body(status, entry.error_details.as_deref()) + }; if use_sse { - sse_response(stream, proxy_state.sse_keep_alive_secs()) + sse_response(stream, render_error, proxy_state.sse_keep_alive_secs()) } else { - ndjson_response(stream) + ndjson_response(stream, render_error, envelope) } } - Err(status) => error::status_to_response(status), + Err(status) => { + error::status_to_response_with_details(&status, entry.error_details.as_deref()) + } } } -/// One JSON frame of a streaming response, already serialized. +/// One frame of a streaming response: a serialized message, or the error body +/// (see [`error::error_body`]) that ends the stream. /// /// `Error` is terminal: [`json_frames`] stops the stream right after yielding /// it, so an error frame is always the last thing a client sees regardless of -/// whether it came from a gRPC status or a serialization failure. +/// whether it came from a gRPC status or a serialization failure. It stays a +/// JSON value so each format can add its own framing before serializing it. enum StreamFrame { Data(String), - Error(String), + Error(serde_json::Value), } +/// Type URL marking the terminal error line of an unenveloped NDJSON stream. A +/// data line is the ProtoJSON of a response message, which carries a top-level +/// `@type` only when the RPC streams `google.protobuf.Any`; `Struct`, `Value` +/// and `ListValue` can carry any key at all. For those RPCs no in-band marker +/// is collision-free, which is what the opt-in envelope +/// ([`TranscodeOptions::with_ndjson_envelope`]) is for; the unenveloped shape +/// stays the default so existing NDJSON readers keep working. +const STATUS_TYPE_URL: &str = "type.googleapis.com/google.rpc.Status"; + /// Turn a gRPC message stream into a stream of serialized JSON frames, stopping /// after the first error so error frames are unambiguously terminal. /// -/// Both a gRPC `Status` and a per-message serialization failure become a -/// terminal [`StreamFrame::Error`]; downstream messages the upstream might -/// still emit are dropped rather than streamed past the error. -fn json_frames(stream: St) -> impl futures::Stream + Send + 'static +/// Both a gRPC `Status` (rendered by `render_error`) and a per-message +/// serialization failure become a terminal [`StreamFrame::Error`]; downstream +/// messages the upstream might still emit are dropped rather than streamed past +/// the error. The terminal frame drops the upstream along with the stream +/// state, so the body ends at once instead of polling an upstream that may +/// stay open. +fn json_frames( + stream: St, + render_error: R, +) -> impl futures::Stream + Send + 'static where - St: futures::Stream> + Send + 'static, + St: futures::Stream> + Send + Unpin + 'static, + R: Fn(&tonic::Status) -> serde_json::Value + Send + 'static, { - let opts = response_serialize_options(); - stream.scan(false, move |stopped, result| { - if *stopped { - return futures::future::ready(None); - } - let frame = match result { + let state = Some((stream, render_error, response_serialize_options())); + futures::stream::unfold(state, |state| async move { + let (mut stream, render_error, opts) = state?; + let error = match stream.next().await? { Ok(msg) => match message_to_json_string(&msg, &opts) { - Ok(s) => StreamFrame::Data(s), - Err(e) => { - *stopped = true; - StreamFrame::Error( - serde_json::json!({ - "error": "INTERNAL", - "message": format!("serialization error: {e}"), - }) - .to_string(), - ) - } + Ok(s) => return Some((StreamFrame::Data(s), Some((stream, render_error, opts)))), + Err(e) => tonic::Status::internal(format!("serialization error: {e}")), }, - Err(status) => { - *stopped = true; - StreamFrame::Error(stream_error_json(&status).to_string()) - } + Err(status) => status, }; - futures::future::ready(Some(frame)) + Some((StreamFrame::Error(render_error(&error)), None)) }) } /// Build an NDJSON (`application/x-ndjson`) streaming response. -fn ndjson_response(stream: St) -> Response +/// +/// With `envelope`, every line is wrapped: `{"result": }` for data and +/// `{"error": }` for the terminal error. Without it, a data line is +/// the bare message and the error line is the error body plus an +/// `@type: google.rpc.Status` marker (see [`STATUS_TYPE_URL`]). +fn ndjson_response(stream: St, render_error: R, envelope: bool) -> Response where - St: futures::Stream> + Send + 'static, + St: futures::Stream> + Send + Unpin + 'static, + R: Fn(&tonic::Status) -> serde_json::Value + Send + 'static, { - // Data and error frames are both JSON lines; an error is distinguished by - // its `error` field and by being the final line (see `json_frames`). - let byte_stream = json_frames(stream).map(|frame| { + let byte_stream = json_frames(stream, render_error).map(move |frame| { let mut line = match frame { - StreamFrame::Data(s) | StreamFrame::Error(s) => s, + // The message is already serialized; wrap the text instead of + // parsing it back into a value. + StreamFrame::Data(s) if envelope => { + let mut wrapped = String::with_capacity(s.len() + 12); + wrapped.push_str("{\"result\":"); + wrapped.push_str(&s); + wrapped.push('}'); + wrapped + } + StreamFrame::Data(s) => s, + StreamFrame::Error(body) if envelope => { + serde_json::json!({ "error": body }).to_string() + } + StreamFrame::Error(mut body) => { + if let Some(fields) = body.as_object_mut() { + fields.insert("@type".into(), STATUS_TYPE_URL.into()); + } + body.to_string() + } }; line.push('\n'); Ok::(axum::body::Bytes::from(line)) @@ -390,17 +510,20 @@ where } /// Build a Server-Sent Events (`text/event-stream`) streaming response. -fn sse_response(stream: St, keep_alive_secs: u64) -> Response +fn sse_response(stream: St, render_error: R, keep_alive_secs: u64) -> Response where - St: futures::Stream> + Send + 'static, + St: futures::Stream> + Send + Unpin + 'static, + R: Fn(&tonic::Status) -> serde_json::Value + Send + 'static, { // Terminal errors use the `stream-error` event type, not the reserved // `error` type that the browser EventSource dispatches for transport // failures — clients listen for it via addEventListener("stream-error"). - let event_stream = json_frames(stream).map(|frame| { + let event_stream = json_frames(stream, render_error).map(|frame| { let event = match frame { StreamFrame::Data(s) => Event::default().data(s), - StreamFrame::Error(s) => Event::default().event("stream-error").data(s), + StreamFrame::Error(body) => Event::default() + .event("stream-error") + .data(body.to_string()), }; Ok::(event) }); @@ -410,88 +533,71 @@ where .into_response() } -/// Generic transcoding handler. -async fn transcode_handler( - State(proxy_state): State, - headers: HeaderMap, - Path(path_params): Path>, - RawQuery(raw_query): RawQuery, - body_bytes: axum::body::Bytes, - entry: std::sync::Arc, -) -> Response { - let channel = proxy_state.grpc_channel(); - +/// Map the HTTP request onto the RPC's input message: path parameters, query +/// parameters and the route's `body` rule. Unary and server-streaming routes +/// share it, so both bind a request the same way. The error is the message of +/// the 400 the caller answers with, before the upstream is called. +fn decode_request( + entry: &RouteEntry, + headers: &HeaderMap, + path_params: &std::collections::HashMap, + raw_query: Option<&str>, + body_bytes: &[u8], +) -> Result { // Only read the body when the rule maps it onto the message. let json_body = match entry.body { request::BodyMapping::None => serde_json::Value::Null, - _ => { - let ct = body::content_type(&headers); - match body::parse_body(ct, &body_bytes) { - Ok(v) => v, - Err(e) => { - return ( - StatusCode::BAD_REQUEST, - Json(serde_json::json!({ - "error": "INVALID_ARGUMENT", - "message": format!("failed to parse request body: {e}"), - })), - ) - .into_response(); - } - } - } + _ => body::parse_body(body::content_type(headers), body_bytes) + .map_err(|e| format!("failed to parse request body: {e}"))?, }; // Query string → field bindings (fields not bound by path or body). // A malformed query is a client error: reject it rather than silently // dropping every query-bound field. - let query_pairs = match request::parse_query(raw_query.as_deref()) { - Ok(pairs) => pairs, - Err(e) => { - return ( - StatusCode::BAD_REQUEST, - Json(serde_json::json!({ - "error": "INVALID_ARGUMENT", - "message": e, - })), - ) - .into_response(); - } - }; + let query_pairs = request::parse_query(raw_query)?; let input_desc = entry.method.input(); - let request_json = match request::build_request_json( + let request_json = request::build_request_json( &input_desc, &entry.body, json_body, - &path_params, + path_params, &query_pairs, - ) { - Ok(v) => v, - Err(e) => { - return ( - StatusCode::BAD_REQUEST, - Json(serde_json::json!({ - "error": "INVALID_ARGUMENT", - "message": e, - })), - ) - .into_response(); - } - }; + )?; + + DynamicMessage::deserialize(input_desc, request_json) + .map_err(|e| format!("failed to decode request: {e}")) +} + +/// The 400 answer to a request [`decode_request`] could not map, in the same +/// error body the upstream's own errors get on this route. +fn bad_request(entry: &RouteEntry, message: String) -> Response { + error::status_to_response_with_details( + &tonic::Status::invalid_argument(message), + entry.error_details.as_deref(), + ) +} + +/// Generic transcoding handler. +async fn transcode_handler( + State(proxy_state): State, + headers: HeaderMap, + Path(path_params): Path>, + RawQuery(raw_query): RawQuery, + body_bytes: axum::body::Bytes, + entry: std::sync::Arc, +) -> Response { + let channel = proxy_state.grpc_channel(); - let request_msg = match DynamicMessage::deserialize(input_desc, request_json) { + let request_msg = match decode_request( + &entry, + &headers, + &path_params, + raw_query.as_deref(), + &body_bytes, + ) { Ok(msg) => msg, - Err(e) => { - return ( - StatusCode::BAD_REQUEST, - Json(serde_json::json!({ - "error": "INVALID_ARGUMENT", - "message": format!("failed to decode request: {e}"), - })), - ) - .into_response(); - } + Err(message) => return bad_request(&entry, message), }; let grpc_metadata = @@ -501,19 +607,14 @@ async fn transcode_handler( metadata::apply_request_deadline(&mut grpc_request, &headers); let output_desc = entry.method.output(); - let grpc_codec = codec::DynamicCodec::new(output_desc.clone()); + let grpc_codec = + codec::DynamicCodec::with_required_check(output_desc.clone(), entry.response_has_required); let grpc_path = entry.grpc_path.clone(); let mut grpc_client = Grpc::new(channel); if let Err(e) = grpc_client.ready().await { - return ( - StatusCode::SERVICE_UNAVAILABLE, - Json(serde_json::json!({ - "error": "UNAVAILABLE", - "message": format!("gRPC upstream not ready: {e}"), - })), - ) - .into_response(); + let status = tonic::Status::unavailable(format!("gRPC upstream not ready: {e}")); + return error::status_to_response_with_details(&status, entry.error_details.as_deref()); } match grpc_client.unary(grpc_request, grpc_path, grpc_codec).await { @@ -541,18 +642,16 @@ async fn transcode_handler( } Err(e) => { tracing::error!("Failed to serialize gRPC response: {e}"); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": "INTERNAL", - "message": "failed to serialize response", - })), + error::status_to_response_with_details( + &tonic::Status::internal("failed to serialize response"), + entry.error_details.as_deref(), ) - .into_response() } } } - Err(status) => error::status_to_response(status), + Err(status) => { + error::status_to_response_with_details(&status, entry.error_details.as_deref()) + } } } @@ -583,6 +682,7 @@ fn extract_routes(pool: &DescriptorPool) -> Vec { } }; + let response_has_required = codec::has_required_fields(&method.output()); for binding in extract_http_bindings(&method, &http_ext) { entries.push(RouteEntry { http_path: binding.http_path, @@ -591,6 +691,10 @@ fn extract_routes(pool: &DescriptorPool) -> Vec { method: method.clone(), body: binding.body, response_body: binding.response_body, + // Decided per mounted path in `routes_with_options`. + error_details: None, + ndjson_envelope: false, + response_has_required, }); } } @@ -623,6 +727,7 @@ fn extract_streaming_routes(pool: &DescriptorPool) -> Vec { } }; + let response_has_required = codec::has_required_fields(&method.output()); for binding in extract_http_bindings(&method, &http_ext) { tracing::info!( "Registering streaming route: {} {} → {}", @@ -641,6 +746,10 @@ fn extract_streaming_routes(pool: &DescriptorPool) -> Vec { method: method.clone(), body: binding.body, response_body: binding.response_body, + // Decided per mounted path in `routes_with_options`. + error_details: None, + ndjson_envelope: false, + response_has_required, }); } } @@ -856,348 +965,4 @@ fn catch_all(name: &str, is_last: bool) -> String { } #[cfg(test)] -mod tests { - use super::*; - - /// Build a standalone `HttpRule`-shaped descriptor (self-referential - /// `additional_bindings`) so the binding parser can be tested without the - /// google.api extension wiring. - fn http_rule_descriptor() -> prost_reflect::MessageDescriptor { - use prost_reflect::prost::Message; - use prost_reflect::prost_types::{ - field_descriptor_proto::{Label, Type}, - DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet, - }; - - let str_field = |name: &str, num: i32| FieldDescriptorProto { - name: Some(name.to_string()), - number: Some(num), - label: Some(Label::Optional as i32), - r#type: Some(Type::String as i32), - ..Default::default() - }; - let rule = DescriptorProto { - name: Some("HttpRule".to_string()), - field: vec![ - str_field("get", 2), - str_field("put", 3), - str_field("post", 4), - str_field("delete", 5), - str_field("patch", 6), - str_field("body", 7), - str_field("response_body", 12), - FieldDescriptorProto { - name: Some("additional_bindings".to_string()), - number: Some(11), - label: Some(Label::Repeated as i32), - r#type: Some(Type::Message as i32), - type_name: Some(".gapi.HttpRule".to_string()), - ..Default::default() - }, - ], - ..Default::default() - }; - let file = FileDescriptorProto { - name: Some("http.proto".to_string()), - package: Some("gapi".to_string()), - message_type: vec![rule], - syntax: Some("proto3".to_string()), - ..Default::default() - }; - let fds = FileDescriptorSet { file: vec![file] }; - let pool = DescriptorPool::decode(fds.encode_to_vec().as_slice()).unwrap(); - pool.get_message_by_name("gapi.HttpRule").unwrap() - } - - #[test] - fn collect_bindings_reads_body_response_and_additional() { - let desc = http_rule_descriptor(); - - // additional_bindings entry: POST /v1/items with whole-body mapping. - let mut extra = DynamicMessage::new(desc.clone()); - extra.set_field_by_name("post", prost_reflect::Value::String("/v1/items".into())); - extra.set_field_by_name("body", prost_reflect::Value::String("*".into())); - - // primary rule: GET /v1/items/{id}, returns only the `result` subfield. - let mut rule = DynamicMessage::new(desc); - rule.set_field_by_name("get", prost_reflect::Value::String("/v1/items/{id}".into())); - rule.set_field_by_name( - "response_body", - prost_reflect::Value::String("result".into()), - ); - rule.set_field_by_name( - "additional_bindings", - prost_reflect::Value::List(vec![prost_reflect::Value::Message(extra)]), - ); - - let bindings = collect_bindings(&rule); - assert_eq!(bindings.len(), 2); - - // Primary: GET, no body, response_body = result. - assert!(matches!(bindings[0].http_method, HttpMethod::Get)); - assert_eq!(bindings[0].http_path, "/v1/items/{id}"); - assert_eq!(bindings[0].body, request::BodyMapping::None); - assert_eq!(bindings[0].response_body.as_deref(), Some("result")); - - // Additional: POST, whole-body mapping, no response_body. - assert!(matches!(bindings[1].http_method, HttpMethod::Post)); - assert_eq!(bindings[1].http_path, "/v1/items"); - assert_eq!(bindings[1].body, request::BodyMapping::Root); - assert_eq!(bindings[1].response_body, None); - } - - #[test] - fn test_proto_path_to_axum() { - // axum 0.8: proto `{param}` IS the native capture syntax, pass through verbatim. - assert_eq!(proto_path_to_axum("/v1/profiles/{id}"), "/v1/profiles/{id}"); - assert_eq!( - proto_path_to_axum("/v1/admin/profiles/{profile_id}/metadata/{key}"), - "/v1/admin/profiles/{profile_id}/metadata/{key}" - ); - assert_eq!(proto_path_to_axum("/v1/auth/login"), "/v1/auth/login"); - } - - #[test] - fn test_proto_path_to_axum_wildcards() { - // `{name=*}` single-segment field path collapses to a plain capture. - assert_eq!(proto_path_to_axum("/v1/{name=*}"), "/v1/{name}"); - // `{name=**}` multi-segment catch-all maps to axum's `{*name}`. - assert_eq!( - proto_path_to_axum("/v1/files/{path=**}"), - "/v1/files/{*path}" - ); - // Bare wildcards get position-named captures so they never collide. - // Index is the segment position after splitting on `/` (leading "" = 0). - assert_eq!(proto_path_to_axum("/v1/*/items"), "/v1/{wildcard2}/items"); - assert_eq!(proto_path_to_axum("/v1/files/**"), "/v1/files/{*wildcard3}"); - } - - #[test] - fn non_terminal_catch_all_degrades_to_single_capture() { - // A catch-all `{*name}` is only valid in axum's LAST path segment. - // An unsupported/multi-segment field template in a NON-terminal position - // (`/v1/{name=projects/*}/topics`) must NOT emit a mid-path catch-all — - // axum rejects `/v1/{*name}/topics` at `Router::route()`. It degrades to - // a single-segment capture instead. - assert_eq!( - proto_path_to_axum("/v1/{name=projects/*}/topics"), - "/v1/{name}/topics" - ); - let path = proto_path_to_axum("/v1/{name=projects/*}/topics"); - let _router: Router<()> = Router::new().route(&path, get(|| async { "ok" })); - - // The same guard applies to an explicit `**` template in non-terminal - // position and a terminal one still yields a real catch-all. - assert_eq!(proto_path_to_axum("/v1/{rest=**}/tail"), "/v1/{rest}/tail"); - assert_eq!( - proto_path_to_axum("/v1/files/{rest=**}"), - "/v1/files/{*rest}" - ); - } - - #[test] - fn multi_segment_field_template_does_not_fracture() { - // google.api.http resource-name templates (AIP-127) embed slashes - // inside a SINGLE brace span: `{name=shelves/*/books/*}`. Splitting on - // `/` before brace parsing fractured this into invalid fragments and - // produced a mangled axum path that panicked at `Router::route()`. - // It must collapse to a single catch-all capture instead. - assert_eq!( - proto_path_to_axum("/v1/{name=shelves/*/books/*}"), - "/v1/{*name}" - ); - // And the produced path must actually register on axum 0.8. - let path = proto_path_to_axum("/v1/{name=shelves/*/books/*}"); - let _router: Router<()> = Router::new().route(&path, get(|| async { "ok" })); - } - - /// Regression for the axum 0.7→0.8 migration bug: `proto_path_to_axum` - /// emitted `:id` syntax, which axum 0.8 rejects at `Router::route()` with - /// a startup panic ("Path segments must not start with `:`"). Building the - /// router over a brace-param path must NOT panic. Pre-fix this panicked. - #[test] - fn router_builds_with_brace_path_params_on_axum_0_8() { - let axum_path = proto_path_to_axum("/v1/profiles/{id}"); - let _router: Router<()> = Router::new().route(&axum_path, get(|| async { "ok" })); - - // Deeper nesting and a catch-all also route without panicking. - let nested = proto_path_to_axum("/v1/admin/profiles/{profile_id}/metadata/{key}"); - let catch_all = proto_path_to_axum("/v1/files/{path=**}"); - let _router: Router<()> = Router::new() - .route(&nested, get(|| async { "ok" })) - .route(&catch_all, get(|| async { "ok" })); - } - - /// `Item { name: "alice", count: 42 }` — default fixture for the - /// serialization helpers. - fn item_message() -> DynamicMessage { - item_message_named("alice", 42) - } - - /// Build an `Item { name, count }` message from a freshly-decoded - /// descriptor pool, used to exercise the streaming serialization helpers. - fn item_message_named(name: &str, count: i64) -> DynamicMessage { - use prost_reflect::prost::Message; - use prost_reflect::prost_types::{ - field_descriptor_proto::{Label, Type}, - DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet, - }; - - let item = DescriptorProto { - name: Some("Item".to_string()), - field: vec![ - FieldDescriptorProto { - name: Some("name".to_string()), - number: Some(1), - label: Some(Label::Optional as i32), - r#type: Some(Type::String as i32), - ..Default::default() - }, - FieldDescriptorProto { - name: Some("count".to_string()), - number: Some(2), - label: Some(Label::Optional as i32), - r#type: Some(Type::Int64 as i32), - ..Default::default() - }, - ], - ..Default::default() - }; - let file = FileDescriptorProto { - name: Some("item.proto".to_string()), - package: Some("test.v1".to_string()), - message_type: vec![item], - syntax: Some("proto3".to_string()), - ..Default::default() - }; - let mut bytes = Vec::new(); - FileDescriptorSet { file: vec![file] } - .encode(&mut bytes) - .unwrap(); - let pool = DescriptorPool::decode(bytes.as_slice()).unwrap(); - let desc = pool.get_message_by_name("test.v1.Item").unwrap(); - - let mut msg = DynamicMessage::new(desc); - msg.set_field_by_name("name", prost_reflect::Value::String(name.to_string())); - msg.set_field_by_name("count", prost_reflect::Value::I64(count)); - msg - } - - /// Collect a streaming response body into a single UTF-8 string. - async fn collect_body(resp: Response) -> String { - let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) - .await - .unwrap(); - String::from_utf8(bytes.to_vec()).unwrap() - } - - #[tokio::test] - async fn ndjson_error_frame_is_terminal() { - // A gRPC error mid-stream must be the LAST frame: messages the upstream - // would yield after the error are dropped, so the error line is an - // unambiguous end-of-stream signal rather than a mid-stream marker. - let items = vec![ - Ok(item_message_named("alice", 1)), - Err(tonic::Status::internal("boom")), - Ok(item_message_named("bob", 2)), - ]; - let body = collect_body(ndjson_response(futures::stream::iter(items))).await; - let lines: Vec<&str> = body.lines().collect(); - assert_eq!(lines.len(), 2, "stream must stop after the error frame"); - assert!(lines[0].contains("alice")); - assert!(lines[1].contains("INTERNAL") && lines[1].contains("boom")); - assert!(!body.contains("bob"), "post-error message must be dropped"); - } - - #[tokio::test] - async fn sse_error_uses_distinct_event_name() { - // The terminal error is sent as `event: stream-error`, not the reserved - // `error` type that collides with the browser EventSource onerror. - let items = vec![ - Ok(item_message_named("alice", 1)), - Err(tonic::Status::permission_denied("nope")), - Ok(item_message_named("bob", 2)), - ]; - let body = collect_body(sse_response(futures::stream::iter(items), 15)).await; - assert!(body.contains("stream-error")); - assert!(body.contains("PERMISSION_DENIED")); - assert!(!body.contains("bob"), "post-error message must be dropped"); - } - - #[test] - fn wants_sse_detects_event_stream_accept() { - let mut headers = HeaderMap::new(); - headers.insert("accept", "text/event-stream".parse().unwrap()); - assert!(wants_sse(&headers)); - } - - #[test] - fn wants_sse_matches_within_list_and_ignores_params() { - let mut headers = HeaderMap::new(); - headers.insert( - "accept", - "application/json, text/event-stream;q=0.9".parse().unwrap(), - ); - assert!(wants_sse(&headers)); - } - - #[test] - fn wants_sse_false_for_json_and_missing() { - let mut headers = HeaderMap::new(); - headers.insert("accept", "application/json".parse().unwrap()); - assert!(!wants_sse(&headers)); - assert!(!wants_sse(&HeaderMap::new())); - } - - #[test] - fn wants_sse_rejects_explicit_q_zero() { - // RFC 7231 §5.3.1: `q=0` means the media type is explicitly NOT - // acceptable, so it must not select the SSE path. - let mut headers = HeaderMap::new(); - headers.insert("accept", "text/event-stream;q=0".parse().unwrap()); - assert!(!wants_sse(&headers)); - } - - #[test] - fn wants_sse_honors_second_accept_header_line() { - // A client may send multiple `Accept` header lines; the negotiation - // must consider all of them, not just the first. - let mut headers = HeaderMap::new(); - headers.append("accept", "application/json".parse().unwrap()); - headers.append("accept", "text/event-stream".parse().unwrap()); - assert!(wants_sse(&headers)); - } - - #[test] - fn message_to_json_string_stringifies_64bit() { - let opts = response_serialize_options(); - let json = message_to_json_string(&item_message(), &opts).unwrap(); - let value: serde_json::Value = serde_json::from_str(&json).unwrap(); - assert_eq!(value["name"], "alice"); - // 64-bit integers are stringified to survive JS number precision limits. - assert_eq!(value["count"], "42"); - } - - #[test] - fn ndjson_response_omits_manual_transfer_encoding() { - // hyper picks the framing per protocol version; a hand-set - // transfer-encoding would be illegal on HTTP/2. - let resp = ndjson_response(futures::stream::empty::< - Result, - >()); - assert_eq!( - resp.headers().get("content-type").unwrap(), - "application/x-ndjson" - ); - assert!(resp.headers().get("transfer-encoding").is_none()); - } - - #[test] - fn stream_error_json_carries_grpc_code_name() { - let status = tonic::Status::permission_denied("nope"); - let value = stream_error_json(&status); - assert_eq!(value["error"], "PERMISSION_DENIED"); - assert_eq!(value["message"], "nope"); - assert_eq!(value["code"], tonic::Code::PermissionDenied as i32); - } -} +mod tests; diff --git a/src/transcode/tests.rs b/src/transcode/tests.rs new file mode 100644 index 0000000..4acafde --- /dev/null +++ b/src/transcode/tests.rs @@ -0,0 +1,531 @@ +use super::*; + +/// Build a standalone `HttpRule`-shaped descriptor (self-referential +/// `additional_bindings`) so the binding parser can be tested without the +/// google.api extension wiring. +fn http_rule_descriptor() -> prost_reflect::MessageDescriptor { + use prost_reflect::prost::Message; + use prost_reflect::prost_types::{ + field_descriptor_proto::{Label, Type}, + DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet, + }; + + let str_field = |name: &str, num: i32| FieldDescriptorProto { + name: Some(name.to_string()), + number: Some(num), + label: Some(Label::Optional as i32), + r#type: Some(Type::String as i32), + ..Default::default() + }; + let rule = DescriptorProto { + name: Some("HttpRule".to_string()), + field: vec![ + str_field("get", 2), + str_field("put", 3), + str_field("post", 4), + str_field("delete", 5), + str_field("patch", 6), + str_field("body", 7), + str_field("response_body", 12), + FieldDescriptorProto { + name: Some("additional_bindings".to_string()), + number: Some(11), + label: Some(Label::Repeated as i32), + r#type: Some(Type::Message as i32), + type_name: Some(".gapi.HttpRule".to_string()), + ..Default::default() + }, + ], + ..Default::default() + }; + let file = FileDescriptorProto { + name: Some("http.proto".to_string()), + package: Some("gapi".to_string()), + message_type: vec![rule], + syntax: Some("proto3".to_string()), + ..Default::default() + }; + let fds = FileDescriptorSet { file: vec![file] }; + let pool = DescriptorPool::decode(fds.encode_to_vec().as_slice()).unwrap(); + pool.get_message_by_name("gapi.HttpRule").unwrap() +} + +#[test] +fn collect_bindings_reads_body_response_and_additional() { + let desc = http_rule_descriptor(); + + // additional_bindings entry: POST /v1/items with whole-body mapping. + let mut extra = DynamicMessage::new(desc.clone()); + extra.set_field_by_name("post", prost_reflect::Value::String("/v1/items".into())); + extra.set_field_by_name("body", prost_reflect::Value::String("*".into())); + + // primary rule: GET /v1/items/{id}, returns only the `result` subfield. + let mut rule = DynamicMessage::new(desc); + rule.set_field_by_name("get", prost_reflect::Value::String("/v1/items/{id}".into())); + rule.set_field_by_name( + "response_body", + prost_reflect::Value::String("result".into()), + ); + rule.set_field_by_name( + "additional_bindings", + prost_reflect::Value::List(vec![prost_reflect::Value::Message(extra)]), + ); + + let bindings = collect_bindings(&rule); + assert_eq!(bindings.len(), 2); + + // Primary: GET, no body, response_body = result. + assert!(matches!(bindings[0].http_method, HttpMethod::Get)); + assert_eq!(bindings[0].http_path, "/v1/items/{id}"); + assert_eq!(bindings[0].body, request::BodyMapping::None); + assert_eq!(bindings[0].response_body.as_deref(), Some("result")); + + // Additional: POST, whole-body mapping, no response_body. + assert!(matches!(bindings[1].http_method, HttpMethod::Post)); + assert_eq!(bindings[1].http_path, "/v1/items"); + assert_eq!(bindings[1].body, request::BodyMapping::Root); + assert_eq!(bindings[1].response_body, None); +} + +#[test] +fn test_proto_path_to_axum() { + // axum 0.8: proto `{param}` IS the native capture syntax, pass through verbatim. + assert_eq!(proto_path_to_axum("/v1/profiles/{id}"), "/v1/profiles/{id}"); + assert_eq!( + proto_path_to_axum("/v1/admin/profiles/{profile_id}/metadata/{key}"), + "/v1/admin/profiles/{profile_id}/metadata/{key}" + ); + assert_eq!(proto_path_to_axum("/v1/auth/login"), "/v1/auth/login"); +} + +#[test] +fn test_proto_path_to_axum_wildcards() { + // `{name=*}` single-segment field path collapses to a plain capture. + assert_eq!(proto_path_to_axum("/v1/{name=*}"), "/v1/{name}"); + // `{name=**}` multi-segment catch-all maps to axum's `{*name}`. + assert_eq!( + proto_path_to_axum("/v1/files/{path=**}"), + "/v1/files/{*path}" + ); + // Bare wildcards get position-named captures so they never collide. + // Index is the segment position after splitting on `/` (leading "" = 0). + assert_eq!(proto_path_to_axum("/v1/*/items"), "/v1/{wildcard2}/items"); + assert_eq!(proto_path_to_axum("/v1/files/**"), "/v1/files/{*wildcard3}"); +} + +#[test] +fn non_terminal_catch_all_degrades_to_single_capture() { + // A catch-all `{*name}` is only valid in axum's LAST path segment. + // An unsupported/multi-segment field template in a NON-terminal position + // (`/v1/{name=projects/*}/topics`) must NOT emit a mid-path catch-all — + // axum rejects `/v1/{*name}/topics` at `Router::route()`. It degrades to + // a single-segment capture instead. + assert_eq!( + proto_path_to_axum("/v1/{name=projects/*}/topics"), + "/v1/{name}/topics" + ); + let path = proto_path_to_axum("/v1/{name=projects/*}/topics"); + let _router: Router<()> = Router::new().route(&path, get(|| async { "ok" })); + + // The same guard applies to an explicit `**` template in non-terminal + // position and a terminal one still yields a real catch-all. + assert_eq!(proto_path_to_axum("/v1/{rest=**}/tail"), "/v1/{rest}/tail"); + assert_eq!( + proto_path_to_axum("/v1/files/{rest=**}"), + "/v1/files/{*rest}" + ); +} + +#[test] +fn multi_segment_field_template_does_not_fracture() { + // google.api.http resource-name templates (AIP-127) embed slashes + // inside a SINGLE brace span: `{name=shelves/*/books/*}`. Splitting on + // `/` before brace parsing fractured this into invalid fragments and + // produced a mangled axum path that panicked at `Router::route()`. + // It must collapse to a single catch-all capture instead. + assert_eq!( + proto_path_to_axum("/v1/{name=shelves/*/books/*}"), + "/v1/{*name}" + ); + // And the produced path must actually register on axum 0.8. + let path = proto_path_to_axum("/v1/{name=shelves/*/books/*}"); + let _router: Router<()> = Router::new().route(&path, get(|| async { "ok" })); +} + +/// Regression for the axum 0.7→0.8 migration bug: `proto_path_to_axum` +/// emitted `:id` syntax, which axum 0.8 rejects at `Router::route()` with +/// a startup panic ("Path segments must not start with `:`"). Building the +/// router over a brace-param path must NOT panic. Pre-fix this panicked. +#[test] +fn router_builds_with_brace_path_params_on_axum_0_8() { + let axum_path = proto_path_to_axum("/v1/profiles/{id}"); + let _router: Router<()> = Router::new().route(&axum_path, get(|| async { "ok" })); + + // Deeper nesting and a catch-all also route without panicking. + let nested = proto_path_to_axum("/v1/admin/profiles/{profile_id}/metadata/{key}"); + let catch_all = proto_path_to_axum("/v1/files/{path=**}"); + let _router: Router<()> = Router::new() + .route(&nested, get(|| async { "ok" })) + .route(&catch_all, get(|| async { "ok" })); +} + +/// `Item { name: "alice", count: 42 }` — default fixture for the +/// serialization helpers. +fn item_message() -> DynamicMessage { + item_message_named("alice", 42) +} + +/// Build an `Item { name, count }` message from a freshly-decoded +/// descriptor pool, used to exercise the streaming serialization helpers. +fn item_message_named(name: &str, count: i64) -> DynamicMessage { + use prost_reflect::prost::Message; + use prost_reflect::prost_types::{ + field_descriptor_proto::{Label, Type}, + DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet, + }; + + let item = DescriptorProto { + name: Some("Item".to_string()), + field: vec![ + FieldDescriptorProto { + name: Some("name".to_string()), + number: Some(1), + label: Some(Label::Optional as i32), + r#type: Some(Type::String as i32), + ..Default::default() + }, + FieldDescriptorProto { + name: Some("count".to_string()), + number: Some(2), + label: Some(Label::Optional as i32), + r#type: Some(Type::Int64 as i32), + ..Default::default() + }, + ], + ..Default::default() + }; + let file = FileDescriptorProto { + name: Some("item.proto".to_string()), + package: Some("test.v1".to_string()), + message_type: vec![item], + syntax: Some("proto3".to_string()), + ..Default::default() + }; + let mut bytes = Vec::new(); + FileDescriptorSet { file: vec![file] } + .encode(&mut bytes) + .unwrap(); + let pool = DescriptorPool::decode(bytes.as_slice()).unwrap(); + let desc = pool.get_message_by_name("test.v1.Item").unwrap(); + + let mut msg = DynamicMessage::new(desc); + msg.set_field_by_name("name", prost_reflect::Value::String(name.to_string())); + msg.set_field_by_name("count", prost_reflect::Value::I64(count)); + msg +} + +/// Collect a streaming response body into a single UTF-8 string. +async fn collect_body(resp: Response) -> String { + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + String::from_utf8(bytes.to_vec()).unwrap() +} + +/// Terminal-frame renderer for a route with error details switched off. +fn no_details(status: &tonic::Status) -> serde_json::Value { + error::error_body(status, None) +} + +#[tokio::test] +async fn ndjson_error_frame_is_terminal() { + // A gRPC error mid-stream must be the LAST frame: messages the upstream + // would yield after the error are dropped, so the error line is an + // unambiguous end-of-stream signal rather than a mid-stream marker. + let items = vec![ + Ok(item_message_named("alice", 1)), + Err(tonic::Status::internal("boom")), + Ok(item_message_named("bob", 2)), + ]; + let body = collect_body(ndjson_response( + futures::stream::iter(items), + no_details, + false, + )) + .await; + let lines: Vec<&str> = body.lines().collect(); + assert_eq!(lines.len(), 2, "stream must stop after the error frame"); + assert!(lines[0].contains("alice")); + assert!(lines[1].contains("INTERNAL") && lines[1].contains("boom")); + assert!(!body.contains("bob"), "post-error message must be dropped"); +} + +#[tokio::test] +async fn sse_error_uses_distinct_event_name() { + // The terminal error is sent as `event: stream-error`, not the reserved + // `error` type that collides with the browser EventSource onerror. + let items = vec![ + Ok(item_message_named("alice", 1)), + Err(tonic::Status::permission_denied("nope")), + Ok(item_message_named("bob", 2)), + ]; + let body = collect_body(sse_response(futures::stream::iter(items), no_details, 15)).await; + assert!(body.contains("stream-error")); + assert!(body.contains("PERMISSION_DENIED")); + assert!(!body.contains("bob"), "post-error message must be dropped"); +} + +#[tokio::test] +async fn ndjson_terminal_frame_carries_status_details() { + // Once the stream has started the HTTP status (200) is already on the + // wire, so the only place a mid-stream error's details can travel is the + // terminal frame: it must be the same body the unary path renders. + use tonic_types::{ErrorDetail, ErrorInfo, StatusExt}; + let status = tonic::Status::with_error_details_vec( + tonic::Code::ResourceExhausted, + "quota", + [ErrorDetail::from(ErrorInfo::new( + "QUOTA", + "acme.example.com", + std::collections::HashMap::new(), + ))], + ); + let renderer = Arc::new(error::StatusDetails::new(&DescriptorPool::new())); + let mut expected = error::error_body(&status, Some(&renderer)); + let render = move |s: &tonic::Status| error::error_body(s, Some(&renderer)); + + let items = vec![Ok(item_message_named("alice", 1)), Err(status)]; + let body = collect_body(ndjson_response(futures::stream::iter(items), render, false)).await; + let lines: Vec<&str> = body.lines().collect(); + assert_eq!(lines.len(), 2); + let frame: serde_json::Value = serde_json::from_str(lines[1]).unwrap(); + // The line is the unary error body plus the NDJSON frame marker. + expected["@type"] = STATUS_TYPE_URL.into(); + assert_eq!(frame, expected); + assert_eq!( + frame["details"][0]["@type"], + "type.googleapis.com/google.rpc.ErrorInfo" + ); +} + +/// A `Wrapper { google.protobuf.Any payload = 1; }` whose payload names a type +/// no pool knows, so it cannot be serialized to JSON. +fn unserializable_message() -> DynamicMessage { + use prost_reflect::prost_types::{ + field_descriptor_proto::{Label, Type}, + DescriptorProto, FieldDescriptorProto, FileDescriptorProto, + }; + + let wrapper = DescriptorProto { + name: Some("Wrapper".to_string()), + field: vec![FieldDescriptorProto { + name: Some("payload".to_string()), + number: Some(1), + label: Some(Label::Optional as i32), + r#type: Some(Type::Message as i32), + type_name: Some(".google.protobuf.Any".to_string()), + ..Default::default() + }], + ..Default::default() + }; + let file = FileDescriptorProto { + name: Some("wrapper.proto".to_string()), + package: Some("test.v1".to_string()), + dependency: vec!["google/protobuf/any.proto".to_string()], + message_type: vec![wrapper], + syntax: Some("proto3".to_string()), + ..Default::default() + }; + let mut pool = DescriptorPool::global(); + pool.add_file_descriptor_proto(file).unwrap(); + let desc = pool.get_message_by_name("test.v1.Wrapper").unwrap(); + let any_desc = pool.get_message_by_name("google.protobuf.Any").unwrap(); + + let mut any = DynamicMessage::new(any_desc); + any.set_field_by_name( + "type_url", + prost_reflect::Value::String("type.googleapis.com/acme.v1.Unknown".into()), + ); + let mut msg = DynamicMessage::new(desc); + msg.set_field_by_name("payload", prost_reflect::Value::Message(any)); + // Sanity: the fixture really is unserializable. + assert!(message_to_json_string(&msg, &response_serialize_options()).is_err()); + msg +} + +#[tokio::test] +async fn serialization_failure_ends_the_stream_with_the_shared_error_body() { + // A message the proxy cannot turn into JSON ends the stream like an + // upstream error: one terminal INTERNAL frame in the route's error body + // (here with details on, so `details` is present and empty), then nothing. + let renderer = Arc::new(error::StatusDetails::new(&DescriptorPool::new())); + let render = move |s: &tonic::Status| error::error_body(s, Some(&renderer)); + let items = vec![ + Ok(item_message_named("alice", 1)), + Ok(unserializable_message()), + Ok(item_message_named("bob", 2)), + ]; + let body = collect_body(ndjson_response(futures::stream::iter(items), render, false)).await; + let lines: Vec<&str> = body.lines().collect(); + assert_eq!(lines.len(), 2, "{body}"); + let frame: serde_json::Value = serde_json::from_str(lines[1]).unwrap(); + assert_eq!(frame["@type"], STATUS_TYPE_URL); + assert_eq!(frame["error"], "INTERNAL"); + assert_eq!(frame["code"], 13); + assert_eq!(frame["details"], serde_json::json!([])); + assert!( + frame["message"] + .as_str() + .unwrap() + .starts_with("serialization error: "), + "{frame}" + ); +} + +#[tokio::test] +async fn terminal_frame_ends_the_body_while_the_upstream_stays_open() { + // After the terminal frame the body ends at once, without polling the + // upstream again: an upstream that stays open (neither a message nor a + // close) must not keep the response open, whichever failure ended it. + let serialization_failure = futures::stream::iter(vec![Ok(unserializable_message())]); + let upstream_error = futures::stream::iter(vec![Err(tonic::Status::internal("boom"))]); + for (name, items) in [ + ("serialization failure", serialization_failure), + ("upstream error", upstream_error), + ] { + let open_upstream = items.chain(futures::stream::pending()); + let body = tokio::time::timeout( + std::time::Duration::from_secs(1), + collect_body(ndjson_response(open_upstream, no_details, false)), + ) + .await + .unwrap_or_else(|_| panic!("{name}: body stayed open after the terminal frame")); + assert_eq!(body.lines().count(), 1, "{name}: {body}"); + } +} + +#[tokio::test] +async fn sse_error_payload_is_the_unary_body_without_the_ndjson_marker() { + // SSE frames the error by its event type, so the payload is exactly the + // body a unary error gets: no `@type` marker. + let status = tonic::Status::permission_denied("nope"); + let expected = error::error_body(&status, None); + let items = vec![Err(status)]; + let body = collect_body(sse_response(futures::stream::iter(items), no_details, 15)).await; + let payload = body + .lines() + .find_map(|line| line.strip_prefix("data: ")) + .unwrap(); + let frame: serde_json::Value = serde_json::from_str(payload).unwrap(); + assert_eq!(frame, expected); +} + +#[tokio::test] +async fn ndjson_envelope_wraps_data_and_error_lines() { + // With the envelope every line says what it is by its only key, so a data + // message can never be read as the terminal error, whatever it contains; + // the error line then needs no marker, and nothing follows it. + let status = tonic::Status::internal("boom"); + let expected_error = error::error_body(&status, None); + let items = vec![ + Ok(item_message_named("alice", 1)), + Err(status), + Ok(item_message_named("bob", 2)), + ]; + let body = collect_body(ndjson_response( + futures::stream::iter(items), + no_details, + true, + )) + .await; + let lines: Vec = body + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert_eq!( + lines, + vec![ + serde_json::json!({"result": {"name": "alice", "count": "1"}}), + serde_json::json!({"error": expected_error}), + ] + ); +} + +#[test] +fn wants_sse_detects_event_stream_accept() { + let mut headers = HeaderMap::new(); + headers.insert("accept", "text/event-stream".parse().unwrap()); + assert!(wants_sse(&headers)); +} + +#[test] +fn wants_sse_matches_within_list_and_ignores_params() { + let mut headers = HeaderMap::new(); + headers.insert( + "accept", + "application/json, text/event-stream;q=0.9".parse().unwrap(), + ); + assert!(wants_sse(&headers)); +} + +#[test] +fn wants_sse_false_for_json_and_missing() { + let mut headers = HeaderMap::new(); + headers.insert("accept", "application/json".parse().unwrap()); + assert!(!wants_sse(&headers)); + assert!(!wants_sse(&HeaderMap::new())); +} + +#[test] +fn wants_sse_rejects_explicit_q_zero() { + // RFC 7231 §5.3.1: `q=0` means the media type is explicitly NOT + // acceptable, so it must not select the SSE path. + let mut headers = HeaderMap::new(); + headers.insert("accept", "text/event-stream;q=0".parse().unwrap()); + assert!(!wants_sse(&headers)); +} + +#[test] +fn wants_sse_honors_second_accept_header_line() { + // A client may send multiple `Accept` header lines; the negotiation + // must consider all of them, not just the first. + let mut headers = HeaderMap::new(); + headers.append("accept", "application/json".parse().unwrap()); + headers.append("accept", "text/event-stream".parse().unwrap()); + assert!(wants_sse(&headers)); +} + +#[test] +fn message_to_json_string_stringifies_64bit() { + let opts = response_serialize_options(); + let json = message_to_json_string(&item_message(), &opts).unwrap(); + let value: serde_json::Value = serde_json::from_str(&json).unwrap(); + assert_eq!(value["name"], "alice"); + // 64-bit integers are stringified to survive JS number precision limits. + assert_eq!(value["count"], "42"); +} + +#[test] +fn ndjson_response_omits_manual_transfer_encoding() { + // hyper picks the framing per protocol version; a hand-set + // transfer-encoding would be illegal on HTTP/2. + let resp = ndjson_response( + futures::stream::empty::>(), + no_details, + false, + ); + assert_eq!( + resp.headers().get("content-type").unwrap(), + "application/x-ndjson" + ); + assert!(resp.headers().get("transfer-encoding").is_none()); +} + +#[test] +fn stream_error_frame_carries_grpc_code_name() { + let status = tonic::Status::permission_denied("nope"); + let value = no_details(&status); + assert_eq!(value["error"], "PERMISSION_DENIED"); + assert_eq!(value["message"], "nope"); + assert_eq!(value["code"], tonic::Code::PermissionDenied as i32); +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs new file mode 100644 index 0000000..5671ae8 --- /dev/null +++ b/tests/common/mod.rs @@ -0,0 +1,122 @@ +//! Harness for proxy tests against a real tonic upstream: compiles a test +//! `.proto` with `google.api.http` routes in memory, serves a gRPC service on a +//! random local port, and drives the proxy router built by `ProxyServer`. + +use axum::body::Body; +use http::StatusCode; +use prost_reflect::DescriptorPool; +use structured_proxy::config::ProxyConfig; +use structured_proxy::transcode::error::ErrorDetailsPolicy; +use structured_proxy::ProxyServer; +use tower::ServiceExt; + +/// Minimal `google/api/http.proto`: the fields the transcoder reads. +const HTTP_PROTO: &str = r#" +syntax = "proto3"; +package google.api; +message HttpRule { + string selector = 1; + oneof pattern { + string get = 2; + string put = 3; + string post = 4; + string delete = 5; + string patch = 6; + } + string body = 7; + string response_body = 12; + repeated HttpRule additional_bindings = 11; +} +"#; + +const ANNOTATIONS_PROTO: &str = r#" +syntax = "proto3"; +package google.api; +import "google/api/http.proto"; +import "google/protobuf/descriptor.proto"; +extend google.protobuf.MethodOptions { + HttpRule http = 72295728; +} +"#; + +/// Serves the google.api sources and one test file from memory, and +/// descriptor.proto from protox's bundled Google files. +struct TestProtos { + name: &'static str, + source: &'static str, +} + +impl protox::file::FileResolver for TestProtos { + fn open_file(&self, name: &str) -> Result { + let source = match name { + "google/api/http.proto" => HTTP_PROTO, + "google/api/annotations.proto" => ANNOTATIONS_PROTO, + _ if name == self.name => self.source, + _ => return protox::file::GoogleFileResolver::new().open_file(name), + }; + protox::file::File::from_source(name, source) + } +} + +/// Compile the test file `name` (which may import +/// `google/api/annotations.proto`) into a descriptor pool. +pub fn compile(name: &'static str, source: &'static str) -> DescriptorPool { + protox::Compiler::with_file_resolver(TestProtos { name, source }) + .open_file(name) + .expect("test protos compile") + .descriptor_pool() +} + +/// Serve `service` on a random local port; returns its `http://` URL. +pub async fn serve(service: S) -> String +where + S: tower::Service< + http::Request, + Response = http::Response, + Error = std::convert::Infallible, + > + tonic::server::NamedService + + Clone + + Send + + Sync + + 'static, + S::Future: Send + 'static, +{ + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let incoming = futures::stream::unfold(listener, |listener| async move { + let conn = listener.accept().await.map(|(stream, _)| stream); + Some((conn, listener)) + }); + tokio::spawn( + tonic::transport::Server::builder() + .add_service(service) + .serve_with_incoming(incoming), + ); + format!("http://{addr}") +} + +/// The proxy router for `pool` in front of `upstream`, returning error details +/// as `error_details` decides. +pub fn proxy( + upstream: &str, + pool: DescriptorPool, + error_details: ErrorDetailsPolicy, +) -> axum::Router { + let config = + ProxyConfig::from_yaml_str(&format!("upstream:\n default: \"{upstream}\"\n")).unwrap(); + ProxyServer::from_config(config) + .with_descriptors(pool) + .with_error_details(error_details) + .router() + .unwrap() +} + +/// Send `request` through `app`; returns the status and the body as text. +pub async fn send(app: &axum::Router, request: http::Request) -> (StatusCode, String) { + let resp = app.clone().oneshot(request).await.unwrap(); + let status = resp.status(); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + (status, String::from_utf8(bytes.to_vec()).unwrap()) +} diff --git a/tests/dynamic_codec.rs b/tests/dynamic_codec.rs new file mode 100644 index 0000000..698e1b5 --- /dev/null +++ b/tests/dynamic_codec.rs @@ -0,0 +1,183 @@ +//! `DynamicCodec` over a real tonic client and server. +//! +//! A message whose fields all hold their default values encodes to zero bytes +//! (`google.protobuf.Empty` always does). Those frames must decode to the +//! default message on both ends, like tonic's own prost codec does; a decoder +//! that reads an empty frame as "no message" makes the server answer +//! `Missing request message` and leaves the client without its response. + +use std::convert::Infallible; +use std::future::{ready, Ready}; +use std::pin::Pin; +use std::task::{Context, Poll}; + +use prost::Message as _; +use prost_reflect::prost_types::{ + field_descriptor_proto::{Label, Type}, + DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet, +}; +use prost_reflect::{DescriptorPool, DynamicMessage, MessageDescriptor, ReflectMessage}; +use structured_proxy::transcode::codec::DynamicCodec; + +/// `test.v1.Item { string name = 1; }`. +fn item_desc() -> MessageDescriptor { + item_desc_with("proto3", Label::Optional) +} + +/// The proto2 `test.v1.Item { required string name = 1; }`. +fn required_item_desc() -> MessageDescriptor { + item_desc_with("proto2", Label::Required) +} + +fn item_desc_with(syntax: &str, label: Label) -> MessageDescriptor { + let item = DescriptorProto { + name: Some("Item".to_string()), + field: vec![FieldDescriptorProto { + name: Some("name".to_string()), + number: Some(1), + label: Some(label as i32), + r#type: Some(Type::String as i32), + ..Default::default() + }], + ..Default::default() + }; + let file = FileDescriptorProto { + name: Some("item.proto".to_string()), + package: Some("test.v1".to_string()), + message_type: vec![item], + syntax: Some(syntax.to_string()), + ..Default::default() + }; + let fds = FileDescriptorSet { file: vec![file] }; + DescriptorPool::decode(fds.encode_to_vec().as_slice()) + .unwrap() + .get_message_by_name("test.v1.Item") + .unwrap() +} + +fn item(desc: &MessageDescriptor, name: &str) -> DynamicMessage { + let mut msg = DynamicMessage::new(desc.clone()); + msg.set_field_by_name("name", prost_reflect::Value::String(name.to_string())); + msg +} + +/// Answers `Item { name: "echo:" + request.name }`, or the default (empty) +/// Item when the request name is `"reply-empty"`. +#[derive(Clone)] +struct Echo { + desc: MessageDescriptor, +} + +impl tonic::server::UnaryService for Echo { + type Response = DynamicMessage; + type Future = Ready, tonic::Status>>; + + fn call(&mut self, request: tonic::Request) -> Self::Future { + let name = match request.get_ref().get_field_by_name("name").as_deref() { + Some(prost_reflect::Value::String(name)) => name.clone(), + _ => String::new(), + }; + let reply = if name == "reply-empty" { + DynamicMessage::new(self.desc.clone()) + } else { + item(&self.desc, &format!("echo:{name}")) + }; + ready(Ok(tonic::Response::new(reply))) + } +} + +#[derive(Clone)] +struct EchoService { + desc: MessageDescriptor, +} + +impl tonic::server::NamedService for EchoService { + const NAME: &'static str = "test.v1.Echo"; +} + +impl tower::Service> for EchoService { + type Response = http::Response; + type Error = Infallible; + type Future = + Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, req: http::Request) -> Self::Future { + let desc = self.desc.clone(); + Box::pin(async move { + let mut grpc = tonic::server::Grpc::new(DynamicCodec::new(desc.clone())); + Ok(grpc.unary(Echo { desc }, req).await) + }) + } +} + +/// Call `test.v1.Echo/Call` with `request` on a fresh server. +async fn call(request: DynamicMessage) -> Result { + let desc = request.descriptor(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let incoming = futures::stream::unfold(listener, |listener| async move { + let conn = listener.accept().await.map(|(stream, _)| stream); + Some((conn, listener)) + }); + tokio::spawn( + tonic::transport::Server::builder() + .add_service(EchoService { desc: desc.clone() }) + .serve_with_incoming(incoming), + ); + + let channel = tonic::transport::Channel::from_shared(format!("http://{addr}")) + .unwrap() + .connect() + .await + .unwrap(); + let mut client = tonic::client::Grpc::new(channel); + client.ready().await.unwrap(); + client + .unary( + tonic::Request::new(request), + "/test.v1.Echo/Call".parse().unwrap(), + DynamicCodec::new(desc), + ) + .await + .map(tonic::Response::into_inner) +} + +#[tokio::test] +async fn server_decodes_an_all_default_request() { + // The request encodes to zero bytes; the server must still see a request + // (with the default, empty name) rather than fail with `INTERNAL`. + let desc = item_desc(); + let reply = call(DynamicMessage::new(desc.clone())).await.unwrap(); + assert_eq!(reply, item(&desc, "echo:")); +} + +#[tokio::test] +async fn client_decodes_an_all_default_response() { + // The upstream answers with the default message (zero bytes on the wire); + // the client must receive it instead of an error about a missing message. + let desc = item_desc(); + let reply = call(item(&desc, "reply-empty")).await.unwrap(); + assert_eq!(reply, DynamicMessage::new(desc)); +} + +#[tokio::test] +async fn empty_response_missing_a_required_field_is_rejected() { + // A zero-byte frame is a valid message only when no field is required: for + // a proto2 message with a `required` field it is an uninitialized one, and + // the client must fail with INTERNAL rather than hand it on as a reply. + let desc = required_item_desc(); + let err = call(item(&desc, "reply-empty")).await.unwrap_err(); + assert_eq!(err.code(), tonic::Code::Internal, "{err:?}"); + assert!(err.message().contains("required"), "{err:?}"); +} + +#[tokio::test] +async fn non_empty_messages_round_trip() { + let desc = item_desc(); + let reply = call(item(&desc, "alice")).await.unwrap(); + assert_eq!(reply, item(&desc, "echo:alice")); +} diff --git a/tests/error_details.rs b/tests/error_details.rs new file mode 100644 index 0000000..d6c2215 --- /dev/null +++ b/tests/error_details.rs @@ -0,0 +1,713 @@ +//! REST error bodies carry the upstream's `google.rpc.Status` details. +//! +//! Runs the proxy (through its public `ProxyServer`) in front of a real tonic +//! gRPC server that fails with `tonic_types` details, so every case below goes +//! over the actual `grpc-status-details-bin` trailer: unary errors, a stream +//! refused before any response header, and a stream that fails after its first +//! message, in both NDJSON and SSE. + +mod common; + +use std::convert::Infallible; +use std::future::{ready, Ready}; +use std::pin::Pin; +use std::task::{Context, Poll}; + +use axum::body::Body; +use futures::stream::BoxStream; +use http::StatusCode; +use prost::Message as _; +use prost_reflect::{DescriptorPool, DynamicMessage, MessageDescriptor}; +use serde_json::{json, Value}; +use structured_proxy::transcode::codec::DynamicCodec; +use structured_proxy::transcode::error::ErrorDetailsPolicy; +use tonic_types::{BadRequest, DebugInfo, ErrorDetail, ErrorInfo, FieldViolation, StatusExt}; + +// --- descriptors ------------------------------------------------------------ + +const THINGS_PROTO: &str = r#" +syntax = "proto3"; +package test.v1; +import "google/api/annotations.proto"; + +message Item { + string name = 1; + int64 count = 2; +} + +// A detail type only this product defines. +message QuotaTicket { + string ticket = 1; +} + +service Things { + rpc Get(Item) returns (Item) { + option (google.api.http) = { get: "/v1/things/{name}" }; + } + rpc GetQuiet(Item) returns (Item) { + option (google.api.http) = { get: "/v1/quiet/{name}" }; + } + rpc Watch(Item) returns (stream Item) { + option (google.api.http) = { get: "/v1/things/{name}/watch" }; + } + rpc WatchDenied(Item) returns (stream Item) { + option (google.api.http) = { get: "/v1/things/{name}/denied" }; + } +} +"#; + +fn pool() -> DescriptorPool { + common::compile("test/v1/things.proto", THINGS_PROTO) +} + +fn item_desc(pool: &DescriptorPool) -> MessageDescriptor { + pool.get_message_by_name("test.v1.Item").unwrap() +} + +// --- upstream --------------------------------------------------------------- + +/// `INVALID_ARGUMENT` with ErrorInfo + BadRequest, and a DebugInfo that must +/// not reach the HTTP client. +fn rich_status() -> tonic::Status { + tonic::Status::with_error_details_vec( + tonic::Code::InvalidArgument, + "invalid email", + [ + ErrorDetail::from(ErrorInfo::new( + "EMAIL_TAKEN", + "identity.example.com", + [("email".to_string(), "a@b.c".to_string())] + .into_iter() + .collect::>(), + )), + ErrorDetail::from(BadRequest::new(vec![FieldViolation::new( + "email", + "already registered", + )])), + ErrorDetail::from(DebugInfo::new( + vec!["at identity::register (register.rs:42)".to_string()], + "unique violation on users_email_key", + )), + ], + ) +} + +/// The `details` a client must see for [`rich_status`]. +fn rich_details() -> Value { + json!([ + { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "EMAIL_TAKEN", + "domain": "identity.example.com", + "metadata": {"email": "a@b.c"} + }, + { + "@type": "type.googleapis.com/google.rpc.BadRequest", + "fieldViolations": [{"field": "email", "description": "already registered"}] + } + ]) +} + +/// A product-defined detail, an unknown one and a well-known type, packed by +/// hand since tonic-types only builds the google.rpc ones. +fn mixed_status(pool: &DescriptorPool) -> tonic::Status { + let mut ticket = DynamicMessage::new(pool.get_message_by_name("test.v1.QuotaTicket").unwrap()); + ticket.set_field_by_name("ticket", prost_reflect::Value::String("T-1".into())); + let duration = prost_reflect::prost_types::Duration { + seconds: 1, + nanos: 500_000_000, + }; + let details = [ + ( + "type.googleapis.com/test.v1.QuotaTicket", + ticket.encode_to_vec(), + ), + ( + "type.googleapis.com/acme.v1.Missing", + vec![0x08, 0x96, 0x01], + ), + ( + "type.googleapis.com/google.protobuf.Duration", + duration.encode_to_vec(), + ), + ]; + let mut rpc = tonic_types::pb::Status { + code: tonic::Code::FailedPrecondition as i32, + message: "not yet".into(), + ..Default::default() + }; + for (type_url, value) in details { + rpc.details.push(Default::default()); + let any = rpc.details.last_mut().expect("just pushed"); + any.type_url = type_url.to_string(); + any.value = value; + } + tonic::Status::with_details( + tonic::Code::FailedPrecondition, + "not yet", + bytes::Bytes::from(rpc.encode_to_vec()), + ) +} + +/// `NOT_FOUND` whose `ErrorInfo` detail is truncated: a known type with bytes +/// that do not decode, i.e. a broken upstream response. +fn corrupt_status() -> tonic::Status { + let mut rpc = tonic_types::pb::Status { + code: tonic::Code::NotFound as i32, + message: "gone".into(), + ..Default::default() + }; + rpc.details.push(Default::default()); + let any = rpc.details.last_mut().expect("just pushed"); + any.type_url = "type.googleapis.com/google.rpc.ErrorInfo".to_string(); + any.value = vec![0x0a, 0x05, b'a']; + tonic::Status::with_details( + tonic::Code::NotFound, + "gone", + bytes::Bytes::from(rpc.encode_to_vec()), + ) +} + +/// The body a client gets instead of a broken upstream error status. +fn malformed_upstream_status_body() -> Value { + json!({ + "error": "INTERNAL", + "message": "upstream returned a malformed error status", + "code": 13, + "details": [] + }) +} + +/// Unary RPCs fail according to the requested `name`. +#[derive(Clone)] +struct Failing { + pool: DescriptorPool, +} + +impl tonic::server::UnaryService for Failing { + type Response = DynamicMessage; + type Future = Ready, tonic::Status>>; + + fn call(&mut self, request: tonic::Request) -> Self::Future { + let name = match request.get_ref().get_field_by_name("name").as_deref() { + Some(prost_reflect::Value::String(name)) => name.clone(), + _ => String::new(), + }; + ready(Err(match name.as_str() { + "rich" => rich_status(), + "mixed" => mixed_status(&self.pool), + "corrupt" => corrupt_status(), + _ => tonic::Status::not_found("no such thing"), + })) + } +} + +/// `Watch`: one message, then an error after the response has started: the +/// corrupt one for `name == "corrupt"`, the rich one otherwise. +#[derive(Clone)] +struct FailsMidStream { + item: MessageDescriptor, +} + +impl tonic::server::ServerStreamingService for FailsMidStream { + type Response = DynamicMessage; + type ResponseStream = BoxStream<'static, Result>; + type Future = Ready, tonic::Status>>; + + fn call(&mut self, request: tonic::Request) -> Self::Future { + let failure = match request.get_ref().get_field_by_name("name").as_deref() { + Some(prost_reflect::Value::String(name)) if name == "corrupt" => corrupt_status(), + _ => rich_status(), + }; + let mut first = DynamicMessage::new(self.item.clone()); + first.set_field_by_name("name", prost_reflect::Value::String("first".into())); + first.set_field_by_name("count", prost_reflect::Value::I64(1)); + let items: Vec> = vec![Ok(first), Err(failure)]; + ready(Ok(tonic::Response::new(Box::pin(futures::stream::iter( + items, + ))))) + } +} + +/// `WatchDenied`: refuses before sending any message or response header. +#[derive(Clone)] +struct RefusesStream; + +impl tonic::server::ServerStreamingService for RefusesStream { + type Response = DynamicMessage; + type ResponseStream = BoxStream<'static, Result>; + type Future = Ready, tonic::Status>>; + + fn call(&mut self, _request: tonic::Request) -> Self::Future { + ready(Err(tonic::Status::with_error_details_vec( + tonic::Code::PermissionDenied, + "not your thing", + [ErrorDetail::from(ErrorInfo::new( + "NOT_OWNER", + "things.example.com", + std::collections::HashMap::new(), + ))], + ))) + } +} + +/// The `test.v1.Things` gRPC service, dispatching by method path. +#[derive(Clone)] +struct Things { + pool: DescriptorPool, +} + +impl tonic::server::NamedService for Things { + const NAME: &'static str = "test.v1.Things"; +} + +impl tower::Service> for Things { + type Response = http::Response; + type Error = Infallible; + type Future = + Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, req: http::Request) -> Self::Future { + let pool = self.pool.clone(); + Box::pin(async move { + let item = item_desc(&pool); + let mut grpc = tonic::server::Grpc::new(DynamicCodec::new(item.clone())); + let resp = match req.uri().path() { + "/test.v1.Things/Get" | "/test.v1.Things/GetQuiet" => { + grpc.unary(Failing { pool }, req).await + } + "/test.v1.Things/Watch" => { + grpc.server_streaming(FailsMidStream { item }, req).await + } + "/test.v1.Things/WatchDenied" => grpc.server_streaming(RefusesStream, req).await, + other => panic!("unexpected gRPC path {other}"), + }; + Ok(resp) + }) + } +} + +// --- proxy harness ---------------------------------------------------------- + +/// A proxy router in front of a fresh upstream, returning error details as +/// `error_details` decides. +async fn proxy(error_details: ErrorDetailsPolicy) -> axum::Router { + let pool = pool(); + let upstream = common::serve(Things { pool: pool.clone() }).await; + common::proxy(&upstream, pool, error_details) +} + +async fn get(app: &axum::Router, path: &str, accept: Option<&str>) -> (StatusCode, String) { + let mut req = http::Request::get(path); + if let Some(accept) = accept { + req = req.header("accept", accept); + } + common::send(app, req.body(Body::empty()).unwrap()).await +} + +async fn get_json(app: &axum::Router, path: &str) -> (StatusCode, Value) { + let (status, body) = get(app, path, None).await; + (status, serde_json::from_str(&body).unwrap()) +} + +// --- unary ------------------------------------------------------------------ + +#[tokio::test] +async fn unary_error_carries_error_info_and_bad_request() { + // The acceptance case: typed details arrive as ProtoJSON `Any`s next to + // the existing fields, with the HTTP status of the gRPC → HTTP mapping, + // and DebugInfo stays behind. + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get_json(&app, "/v1/things/rich").await; + assert_eq!(status, StatusCode::BAD_REQUEST); + assert_eq!( + body, + json!({ + "error": "INVALID_ARGUMENT", + "message": "invalid email", + "code": 3, + "details": rich_details() + }) + ); + let text = body.to_string(); + assert!(!text.contains("register.rs") && !text.contains("users_email_key")); +} + +#[tokio::test] +async fn unary_error_without_trailer_has_empty_details() { + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get_json(&app, "/v1/things/missing").await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!( + body, + json!({"error": "NOT_FOUND", "message": "no such thing", "code": 5, "details": []}) + ); +} + +#[tokio::test] +async fn product_unknown_and_well_known_details_are_told_apart() { + // Three different renderings side by side: a product message expands to + // its fields and a well-known type with a special JSON form sits under + // `value` as that JSON, both in `details`; the type no descriptor + // describes goes to `opaqueDetails` (its position, original type URL and + // base64 of the original bytes), never into `details`, on the route that + // switches the extension on. + let app = proxy( + ErrorDetailsPolicy::default() + .opaque_route("/v1/things/*", true) + .unwrap(), + ) + .await; + let (status, body) = get_json(&app, "/v1/things/mixed").await; + assert_eq!(status, StatusCode::BAD_REQUEST); + assert_eq!( + body["details"], + json!([ + {"@type": "type.googleapis.com/test.v1.QuotaTicket", "ticket": "T-1"}, + {"@type": "type.googleapis.com/google.protobuf.Duration", "value": "1.500s"} + ]) + ); + assert_eq!( + body["opaqueDetails"], + json!([{"index": 1, "typeUrl": "type.googleapis.com/acme.v1.Missing", "bytes": "CJYB"}]) + ); + // A route the rule does not match keeps the default: the unknown detail is + // withheld, since its bytes cannot be checked for a nested DebugInfo. + let (status, quiet) = get_json(&app, "/v1/quiet/mixed").await; + assert_eq!(status, StatusCode::BAD_REQUEST); + assert_eq!(quiet["details"], body["details"]); + assert!(quiet.get("opaqueDetails").is_none(), "{quiet}"); +} + +// --- per-route switch ------------------------------------------------------- + +#[tokio::test] +async fn route_rule_switches_details_off_for_one_route() { + // Only the matched route loses `details` (the key is absent, not empty); + // its HTTP status and the other routes are unaffected. + let app = proxy( + ErrorDetailsPolicy::default() + .route("/v1/quiet/*", false) + .unwrap(), + ) + .await; + let (status, quiet) = get_json(&app, "/v1/quiet/rich").await; + assert_eq!(status, StatusCode::BAD_REQUEST); + assert_eq!( + quiet, + json!({"error": "INVALID_ARGUMENT", "message": "invalid email", "code": 3}) + ); + let (_, loud) = get_json(&app, "/v1/things/rich").await; + assert_eq!(loud["details"], rich_details()); +} + +#[tokio::test] +async fn global_switch_off_with_a_sub_route_back_on() { + // Global off, `/v1/things/**` back on: the sub-route (including its + // streaming routes) keeps details, everything else drops them. + let app = proxy( + ErrorDetailsPolicy::disabled() + .route("/v1/things/**", true) + .unwrap(), + ) + .await; + let (_, quiet) = get_json(&app, "/v1/quiet/rich").await; + assert!(quiet.get("details").is_none(), "{quiet}"); + let (_, things) = get_json(&app, "/v1/things/rich").await; + assert_eq!(things["details"], rich_details()); + let (_, denied) = get_json(&app, "/v1/things/x/denied").await; + assert_eq!(denied["details"][0]["reason"], "NOT_OWNER"); +} + +#[tokio::test] +async fn global_switch_off_removes_details_from_stream_frames_too() { + // The switch covers the in-stream terminal frame as well: a route with + // details off ends its stream with the bare error body. + let app = proxy(ErrorDetailsPolicy::disabled()).await; + let (status, body) = get(&app, "/v1/things/x/watch", None).await; + assert_eq!(status, StatusCode::OK, "{body}"); + let last: Value = serde_json::from_str(body.lines().last().unwrap()).unwrap(); + assert_eq!( + last, + json!({ + "@type": "type.googleapis.com/google.rpc.Status", + "error": "INVALID_ARGUMENT", + "message": "invalid email", + "code": 3 + }) + ); +} + +// --- broken upstream status --------------------------------------------------- + +#[tokio::test] +async fn unary_error_with_a_corrupt_known_detail_becomes_a_safe_internal() { + // The type resolves but its bytes do not decode: a broken upstream + // response. Before headers the proxy still owns the status, so the client + // gets a generic 500 INTERNAL, not the upstream's NOT_FOUND with the detail + // dropped, passed on as base64, or otherwise reinterpreted. + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get(&app, "/v1/things/corrupt", None).await; + assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR); + assert_eq!( + serde_json::from_str::(&body).unwrap(), + malformed_upstream_status_body() + ); + assert!(!body.contains("CgVh") && !body.contains("gone"), "{body}"); +} + +#[tokio::test] +async fn stream_error_with_a_corrupt_known_detail_ends_with_a_safe_internal_frame() { + // After the first message the 200 is sent, so the same failure becomes the + // terminal frame instead, in both formats. + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get(&app, "/v1/things/corrupt/watch", None).await; + assert_eq!(status, StatusCode::OK); + let mut expected = malformed_upstream_status_body(); + expected["@type"] = "type.googleapis.com/google.rpc.Status".into(); + let lines: Vec = body + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert_eq!( + lines, + vec![json!({"name": "first", "count": "1"}), expected] + ); + + let (status, body) = get(&app, "/v1/things/corrupt/watch", Some("text/event-stream")).await; + assert_eq!(status, StatusCode::OK); + let error_payload = body + .split("\n\n") + .find(|event| event.contains("event: stream-error")) + .and_then(|event| event.lines().find_map(|line| line.strip_prefix("data: "))) + .expect("a stream-error event"); + assert_eq!( + serde_json::from_str::(error_payload).unwrap(), + malformed_upstream_status_body() + ); +} + +// --- YAML settings ------------------------------------------------------------ + +/// A proxy created from a YAML document (upstream plus `extra_yaml`), the way +/// the standalone binary reads its config file. +async fn proxy_from_yaml(extra_yaml: &str) -> axum::Router { + let pool = pool(); + let upstream = common::serve(Things { pool: pool.clone() }).await; + structured_proxy::ProxyServer::from_yaml_str(&format!( + "upstream:\n default: \"{upstream}\"\n{extra_yaml}" + )) + .unwrap() + .with_descriptors(pool) + .router() + .unwrap() +} + +#[tokio::test] +async fn yaml_switches_route_details_off_and_envelopes_ndjson() { + // Both settings come from the config file: the quiet route loses its + // details, the others keep them, and NDJSON lines are enveloped. + let app = proxy_from_yaml( + "error_details:\n routes:\n - pattern: \"/v1/quiet/*\"\n enabled: false\nstreaming:\n ndjson_envelope: true\n", + ) + .await; + let (_, quiet) = get_json(&app, "/v1/quiet/rich").await; + assert!(quiet.get("details").is_none(), "{quiet}"); + let (_, loud) = get_json(&app, "/v1/things/rich").await; + assert_eq!(loud["details"], rich_details()); + + let (status, body) = get(&app, "/v1/things/x/watch", None).await; + assert_eq!(status, StatusCode::OK); + let lines: Vec = body + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert_eq!( + lines, + vec![ + json!({"result": {"name": "first", "count": "1"}}), + json!({"error": { + "error": "INVALID_ARGUMENT", + "message": "invalid email", + "code": 3, + "details": rich_details() + }}), + ] + ); +} + +#[tokio::test] +async fn yaml_switches_opaque_details_globally_and_per_route() { + // `opaque: true` turns the extension on everywhere and a rule setting only + // `opaque: false` turns it back off for one route, leaving its details on. + let app = proxy_from_yaml( + "error_details:\n opaque: true\n routes:\n - pattern: \"/v1/quiet/*\"\n opaque: false\n", + ) + .await; + let (_, things) = get_json(&app, "/v1/things/mixed").await; + assert_eq!(things["opaqueDetails"][0]["index"], 1, "{things}"); + let (_, quiet) = get_json(&app, "/v1/quiet/mixed").await; + assert_eq!(quiet["details"], things["details"]); + assert!(quiet.get("opaqueDetails").is_none(), "{quiet}"); +} + +#[test] +fn yaml_route_rule_without_a_switch_is_rejected() { + // A rule that sets neither `enabled` nor `opaque` would match routes and + // change nothing, which is a mistake rather than an intent. + let err = structured_proxy::ProxyServer::from_yaml_str( + "upstream:\n default: \"http://127.0.0.1:1\"\nerror_details:\n routes:\n - pattern: \"/v1/**\"\n", + ) + .err() + .expect("a rule without a switch must be rejected"); + assert!( + err.to_string().contains("neither enabled nor opaque"), + "{err}" + ); +} + +#[test] +fn yaml_with_an_invalid_error_details_pattern_is_rejected() { + let err = structured_proxy::ProxyServer::from_yaml_str( + "upstream:\n default: \"http://127.0.0.1:1\"\nerror_details:\n routes:\n - pattern: \"v1/**\"\n enabled: false\n", + ) + .err() + .expect("a relative pattern must be rejected"); + assert!(err.to_string().contains("must start with '/'"), "{err}"); +} + +// --- errors the proxy raises itself ------------------------------------------ + +#[tokio::test] +async fn unmappable_request_gets_the_shared_error_body() { + // A request the proxy rejects before calling the upstream answers in the + // same body as an upstream error on that route, so a client parses one + // shape: here INVALID_ARGUMENT with empty details. + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get_json(&app, "/v1/things/rich?count=many").await; + assert_eq!(status, StatusCode::BAD_REQUEST); + assert_eq!(body["error"], "INVALID_ARGUMENT"); + assert_eq!(body["code"], 3); + assert_eq!(body["details"], json!([])); + assert!(body["message"].is_string()); +} + +#[tokio::test] +async fn unreachable_upstream_gets_the_shared_error_body() { + // Nothing listens on the upstream port: 503 UNAVAILABLE in the shared + // body. With details switched off for the route, the key is absent here + // too. + let app = common::proxy( + "http://127.0.0.1:1", + pool(), + ErrorDetailsPolicy::default() + .route("/v1/quiet/*", false) + .unwrap(), + ); + let (status, body) = get_json(&app, "/v1/things/rich").await; + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(body["error"], "UNAVAILABLE"); + assert_eq!(body["code"], 14); + assert_eq!(body["details"], json!([])); + + let (status, quiet) = get_json(&app, "/v1/quiet/rich").await; + assert_eq!(status, StatusCode::SERVICE_UNAVAILABLE); + assert_eq!(quiet["code"], 14); + assert!(quiet.get("details").is_none(), "{quiet}"); +} + +// --- streaming -------------------------------------------------------------- + +#[tokio::test] +async fn stream_refused_before_headers_maps_like_a_unary_error() { + // No message was sent yet, so the proxy still owns the HTTP status: it is + // mapped (PERMISSION_DENIED → 403) and the body is the unary error body. + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get_json(&app, "/v1/things/x/denied").await; + assert_eq!(status, StatusCode::FORBIDDEN); + assert_eq!( + body, + json!({ + "error": "PERMISSION_DENIED", + "message": "not your thing", + "code": 7, + "details": [{ + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "NOT_OWNER", + "domain": "things.example.com" + }] + }) + ); +} + +#[tokio::test] +async fn ndjson_stream_failing_after_first_message_ends_with_detailed_error_line() { + // The 200 and the first message are already on the wire when the upstream + // fails, so the status cannot change: the error arrives as exactly one + // final NDJSON line holding the same body a unary error would have, marked + // by `@type: google.rpc.Status` so it is not mistaken for a data line. + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get(&app, "/v1/things/x/watch", None).await; + assert_eq!(status, StatusCode::OK); + let lines: Vec = body + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert_eq!( + lines, + vec![ + json!({"name": "first", "count": "1"}), + json!({ + "@type": "type.googleapis.com/google.rpc.Status", + "error": "INVALID_ARGUMENT", + "message": "invalid email", + "code": 3, + "details": rich_details() + }), + ] + ); +} + +#[tokio::test] +async fn sse_stream_failing_after_first_message_ends_with_detailed_stream_error_event() { + // Same failure over SSE: one data event, then exactly one `stream-error` + // event with the full error body, and nothing after it. + let app = proxy(ErrorDetailsPolicy::default()).await; + let (status, body) = get(&app, "/v1/things/x/watch", Some("text/event-stream")).await; + assert_eq!(status, StatusCode::OK); + let events: Vec<(Option<&str>, Value)> = body + .split("\n\n") + .filter(|block| !block.trim().is_empty()) + .map(|block| { + let mut event = None; + let mut data = None; + for line in block.lines() { + if let Some(name) = line.strip_prefix("event: ") { + event = Some(name); + } else if let Some(payload) = line.strip_prefix("data: ") { + data = Some(serde_json::from_str(payload).unwrap()); + } + } + (event, data.expect("every event carries data")) + }) + .collect(); + assert_eq!( + events, + vec![ + (None, json!({"name": "first", "count": "1"})), + ( + Some("stream-error"), + json!({ + "error": "INVALID_ARGUMENT", + "message": "invalid email", + "code": 3, + "details": rich_details() + }) + ), + ] + ); +} diff --git a/tests/streaming_request.rs b/tests/streaming_request.rs new file mode 100644 index 0000000..befbbca --- /dev/null +++ b/tests/streaming_request.rs @@ -0,0 +1,189 @@ +//! Server-streaming routes map the HTTP request onto the gRPC request exactly +//! like unary routes: path parameters, query parameters and the `body` rule. +//! +//! The upstream echoes the request it received as the only stream message, so +//! each test sees what actually reached the service. + +mod common; + +use std::convert::Infallible; +use std::future::{ready, Ready}; +use std::pin::Pin; +use std::task::{Context, Poll}; + +use axum::body::Body; +use futures::stream::BoxStream; +use http::StatusCode; +use prost_reflect::{DescriptorPool, DynamicMessage, MessageDescriptor}; +use serde_json::{json, Value}; +use structured_proxy::transcode::codec::DynamicCodec; + +const THINGS_PROTO: &str = r#" +syntax = "proto3"; +package test.v1; +import "google/api/annotations.proto"; + +message Item { + string name = 1; + int64 count = 2; +} + +service Things { + rpc Watch(Item) returns (stream Item) { + option (google.api.http) = { + get: "/v1/things/{name}/watch" + additional_bindings { post: "/v1/things:watch" body: "*" } + }; + } +} +"#; + +/// Streams back the request it was called with. +#[derive(Clone)] +struct EchoStream; + +impl tonic::server::ServerStreamingService for EchoStream { + type Response = DynamicMessage; + type ResponseStream = BoxStream<'static, Result>; + type Future = Ready, tonic::Status>>; + + fn call(&mut self, request: tonic::Request) -> Self::Future { + let echoed: Vec> = vec![Ok(request.into_inner())]; + ready(Ok(tonic::Response::new(Box::pin(futures::stream::iter( + echoed, + ))))) + } +} + +#[derive(Clone)] +struct Things { + item: MessageDescriptor, +} + +impl tonic::server::NamedService for Things { + const NAME: &'static str = "test.v1.Things"; +} + +impl tower::Service> for Things { + type Response = http::Response; + type Error = Infallible; + type Future = + Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, req: http::Request) -> Self::Future { + let item = self.item.clone(); + Box::pin(async move { + assert_eq!(req.uri().path(), "/test.v1.Things/Watch"); + let mut grpc = tonic::server::Grpc::new(DynamicCodec::new(item)); + Ok(grpc.server_streaming(EchoStream, req).await) + }) + } +} + +async fn proxy() -> axum::Router { + let pool: DescriptorPool = common::compile("test/v1/things.proto", THINGS_PROTO); + let item = pool.get_message_by_name("test.v1.Item").unwrap(); + let upstream = common::serve(Things { item }).await; + common::proxy(&upstream, pool, Default::default()) +} + +/// The NDJSON lines of a streaming response, parsed. +fn ndjson(body: &str) -> Vec { + body.lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect() +} + +#[tokio::test] +async fn get_stream_binds_path_and_query_parameters() { + // `{name}` comes from the path and `count` from the query string; before + // the fix the upstream received an empty request. + let app = proxy().await; + let (status, body) = common::send( + &app, + http::Request::get("/v1/things/alpha/watch?count=7") + .body(Body::empty()) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(ndjson(&body), vec![json!({"name": "alpha", "count": "7"})]); +} + +#[tokio::test] +async fn sse_stream_binds_the_request_too() { + // The request mapping does not depend on the negotiated stream format. + let app = proxy().await; + let (status, body) = common::send( + &app, + http::Request::get("/v1/things/alpha/watch?count=7") + .header("accept", "text/event-stream") + .body(Body::empty()) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + let data: Vec = body + .lines() + .filter_map(|line| line.strip_prefix("data: ")) + .map(|payload| serde_json::from_str(payload).unwrap()) + .collect(); + assert_eq!(data, vec![json!({"name": "alpha", "count": "7"})]); +} + +#[tokio::test] +async fn post_stream_maps_the_whole_body() { + let app = proxy().await; + let (status, body) = common::send( + &app, + http::Request::post("/v1/things:watch") + .header("content-type", "application/json") + .body(Body::from(r#"{"name": "beta", "count": "3"}"#)) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(ndjson(&body), vec![json!({"name": "beta", "count": "3"})]); +} + +#[tokio::test] +async fn post_stream_with_malformed_body_is_rejected_before_the_upstream() { + // A body that is not JSON is the client's error, answered with 400 like on + // a unary route, instead of opening a stream with an empty request. + let app = proxy().await; + let (status, body) = common::send( + &app, + http::Request::post("/v1/things:watch") + .header("content-type", "application/json") + .body(Body::from("{not json")) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{body}"); + let error: Value = serde_json::from_str(&body).unwrap(); + assert_eq!(error["error"], "INVALID_ARGUMENT"); + assert_eq!(error["code"], 3); + assert_eq!(error["details"], json!([])); +} + +#[tokio::test] +async fn get_stream_with_ill_typed_query_is_rejected_before_the_upstream() { + // `count` is an int64: a non-numeric value cannot build the request. + let app = proxy().await; + let (status, body) = common::send( + &app, + http::Request::get("/v1/things/alpha/watch?count=many") + .body(Body::empty()) + .unwrap(), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{body}"); + let error: Value = serde_json::from_str(&body).unwrap(); + assert_eq!(error["error"], "INVALID_ARGUMENT"); + assert_eq!(error["code"], 3); + assert_eq!(error["details"], json!([])); +}