From 46209eb2533329d7dfe7b946befe919fbe60d2d5 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 19 Oct 2026 10:16:20 +0300 Subject: [PATCH 1/5] fix(openapi): describe the JSON the proxy writes - google.protobuf.NullValue fields (singular, oneof member, list item, map value) are documented as JSON null ({nullable: true, enum: [null]}; OpenAPI 3.0.3 has no null type) instead of the string NULL_VALUE; path and query parameters keep the enum name the parser reads - object properties are keyed by the field's JSON name, which responses use; requests accept both spellings - google.protobuf.UInt64Value is format uint64, not int64 - a response_body path naming no field is described as only null - a test serializes sample messages through the unary response path and validates them against the generated schema Closes #148 --- src/openapi.rs | 47 +++++-- src/openapi/tests.rs | 301 +++++++++++++++++++++++++++++++++++++++++-- src/transcode/mod.rs | 2 +- 3 files changed, 329 insertions(+), 21 deletions(-) diff --git a/src/openapi.rs b/src/openapi.rs index a9343fd..217b117 100644 --- a/src/openapi.rs +++ b/src/openapi.rs @@ -16,6 +16,15 @@ use crate::transcode::rule::{self, HttpBinding, RouteMethod}; use crate::transcode::RpcSelection; use crate::transcode::{self, httpbody, path}; +/// How a field's value travels, which decides its schema where the two differ. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Encoding { + /// ProtoJSON: a request body or a response. + Json, + /// The string of a path or query parameter. + Param, +} + /// The operations an OpenAPI 3.0 path item can hold, in the order a `*` rule /// lists them. const OPERATIONS: [&str; 8] = [ @@ -248,7 +257,7 @@ fn build_operation( }); // Try to get type from input message field. if let Some(field) = input.get_field_by_name(name) { - param["schema"] = field_to_schema(&field); + param["schema"] = field_to_schema(&field, Encoding::Param); } param }) @@ -284,7 +293,7 @@ fn build_operation( raw_content() } else { register_nested(&field, schemas); - json!({ "application/json": { "schema": field_to_schema(&field) } }) + json!({ "application/json": { "schema": field_to_schema(&field, Encoding::Json) } }) }; op["requestBody"] = json!({ "required": true, "content": content }); } @@ -309,7 +318,7 @@ fn build_operation( "name": field.name(), "in": "query", "required": false, - "schema": field_to_schema(&field), + "schema": field_to_schema(&field, Encoding::Param), }) }), ); @@ -346,10 +355,10 @@ fn build_operation( let schema = match response_field(&output, path) { Some(field) => { register_nested(&field, schemas); - field_to_schema(&field) + field_to_schema(&field, Encoding::Json) } // The runtime answers JSON `null` for a path that names no field. - None => json!({ "nullable": true }), + None => null_schema(), }; json!({ "200": { @@ -446,8 +455,9 @@ fn message_to_schema( continue; } - let schema = field_to_schema(&field); - properties.insert(name, schema); + // ProtoJSON writes the JSON name; a request may use either name. + let schema = field_to_schema(&field, Encoding::Json); + properties.insert(field.json_name().to_owned(), schema); } let mut schema = json!({ @@ -479,8 +489,22 @@ fn message_ref_or_inline(msg: &MessageDescriptor, schemas: &mut Map Value { +/// Only JSON `null`. OpenAPI 3.0.3 has no `null` type, and `nullable` without +/// a `type` constrains nothing, so the enum does (OpenAPI 3.0.3, Schema +/// Object, `nullable`). +fn null_schema() -> Value { + json!({ "nullable": true, "enum": [null] }) +} + +fn field_to_schema(field: &FieldDescriptor, encoding: Encoding) -> Value { let base = match field.kind() { + // ProtoJSON writes a `NullValue` as `null` (protobuf JSON mapping, + // "NullValue"); a parameter names it like any enum value. + Kind::Enum(e) + if e.full_name() == "google.protobuf.NullValue" && encoding == Encoding::Json => + { + null_schema() + } Kind::Double | Kind::Float => json!({ "type": "number", "format": "double" }), Kind::Int32 | Kind::Sint32 | Kind::Sfixed32 => { json!({ "type": "integer", "format": "int32" }) @@ -517,7 +541,7 @@ fn field_to_schema(field: &FieldDescriptor) -> Value { if let Kind::Message(entry) = field.kind() { let value_field = entry.get_field_by_name("value"); let value_schema = value_field - .map(|f| field_to_schema(&f)) + .map(|f| field_to_schema(&f, encoding)) .unwrap_or_else(|| json!({})); json!({ "type": "object", "additionalProperties": value_schema }) } else { @@ -551,9 +575,8 @@ fn well_known_schema(msg: &MessageDescriptor) -> Value { "google.protobuf.Int32Value" | "google.protobuf.UInt32Value" => { json!({ "type": "integer" }) } - "google.protobuf.Int64Value" | "google.protobuf.UInt64Value" => { - json!({ "type": "string", "format": "int64" }) - } + "google.protobuf.Int64Value" => json!({ "type": "string", "format": "int64" }), + "google.protobuf.UInt64Value" => json!({ "type": "string", "format": "uint64" }), "google.protobuf.FloatValue" | "google.protobuf.DoubleValue" => { json!({ "type": "number" }) } diff --git a/src/openapi/tests.rs b/src/openapi/tests.rs index 96b83e5..60bfd84 100644 --- a/src/openapi/tests.rs +++ b/src/openapi/tests.rs @@ -1,3 +1,5 @@ +use prost_reflect::{DynamicMessage, MapKey, Value as ProtoValue}; + use super::*; #[test] @@ -126,6 +128,12 @@ syntax = "proto3"; package test.v1; import "google/api/annotations.proto"; import "google/api/httpbody.proto"; +import "google/protobuf/duration.proto"; +import "google/protobuf/empty.proto"; +import "google/protobuf/field_mask.proto"; +import "google/protobuf/struct.proto"; +import "google/protobuf/timestamp.proto"; +import "google/protobuf/wrappers.proto"; message Item { string name = 1; @@ -149,6 +157,63 @@ message Tree { string name = 1; Tree child = 2; } +enum Shade { + SHADE_UNSPECIFIED = 0; + SHADE_DARK = 1; +} +// Every kind of field a response can carry, for the schema/wire comparison. +message Wire { + double ratio = 1; + float scale = 2; + int32 small_count = 3; + sint32 delta = 4; + sfixed32 fixed_delta = 5; + int64 big_count = 6; + sint64 big_delta = 7; + sfixed64 fixed_big = 8; + uint32 unsigned_small = 9; + fixed32 fixed_small = 10; + uint64 unsigned_big = 11; + fixed64 fixed_unsigned_big = 12; + bool flag = 13; + string display_name = 14; + bytes raw_data = 15; + Shade shade = 16; + Note note = 17; + google.protobuf.NullValue nothing = 18; + oneof choice { + google.protobuf.NullValue unset = 19; + string label = 20; + Note memo = 21; + } + repeated google.protobuf.NullValue nulls = 22; + map null_map = 23; + repeated int64 big_list = 24; + repeated Shade shades = 25; + repeated Note notes = 26; + map by_number = 27; + map notes_by_key = 28; + google.protobuf.Timestamp seen_at = 29; + google.protobuf.Duration ttl = 30; + google.protobuf.Struct attributes = 31; + google.protobuf.Value any_value = 32; + google.protobuf.ListValue values = 33; + google.protobuf.FieldMask mask = 34; + google.protobuf.Empty nothing_else = 35; + google.protobuf.StringValue maybe_name = 36; + google.protobuf.BytesValue maybe_bytes = 37; + google.protobuf.BoolValue maybe_flag = 38; + google.protobuf.Int32Value maybe_int = 39; + google.protobuf.UInt32Value maybe_uint = 40; + google.protobuf.Int64Value maybe_long = 41; + google.protobuf.UInt64Value maybe_ulong = 42; + google.protobuf.FloatValue maybe_float = 43; + google.protobuf.DoubleValue maybe_double = 44; +} +message NullQuery { + google.protobuf.NullValue none = 1; + repeated google.protobuf.NullValue nones = 2; +} service Api { rpc Head(Item) returns (Item) { @@ -196,6 +261,12 @@ service Api { rpc Shadowed(Item) returns (Item) { option (google.api.http) = { get: "/v1/shelf/{name=**}" }; } + rpc Echo(Wire) returns (Wire) { + option (google.api.http) = { post: "/v1/wire" body: "*" }; + } + rpc Ping(NullQuery) returns (NullQuery) { + option (google.api.http) = { get: "/v1/ping" }; + } } "#; @@ -214,16 +285,20 @@ impl protox::file::FileResolver for TestProtos { } } +fn test_pool() -> DescriptorPool { + protox::Compiler::with_file_resolver(TestProtos) + .open_file("test/v1/api.proto") + .unwrap() + .descriptor_pool() +} + fn spec(aliases: &[AliasConfig]) -> Value { spec_of(aliases, &[]) } /// [`spec`] documenting only the RPCs `only` selects. fn spec_of(aliases: &[AliasConfig], only: &[&str]) -> Value { - let pool = protox::Compiler::with_file_resolver(TestProtos) - .open_file("test/v1/api.proto") - .unwrap() - .descriptor_pool(); + let pool = test_pool(); let selection = RpcSelection::new(&pool, only).unwrap(); generate(&pool, &config(), aliases, &selection) } @@ -447,12 +522,222 @@ fn a_selection_documents_only_the_rpcs_it_transcodes() { #[test] fn a_selection_naming_nothing_in_the_descriptors_is_an_error() { - let pool = protox::Compiler::with_file_resolver(TestProtos) - .open_file("test/v1/api.proto") - .unwrap() - .descriptor_pool(); + let pool = test_pool(); for name in ["test.v1.Nope", "test.v1.Api/Nope", "Api"] { let err = RpcSelection::new(&pool, [name]).unwrap_err(); assert!(err.contains(name), "{err}"); } } + +// --- schema/wire comparison ---------------------------------------------------- + +/// The value a field of `kind` holds in a sample: never its default, so the +/// wire shows its encoding. +fn sample_value(kind: &Kind, variant: usize, depth: usize) -> ProtoValue { + match kind { + Kind::Double => ProtoValue::F64(1.5), + Kind::Float => ProtoValue::F32(0.5), + // Positive and below a second in nanos, so it is also a valid + // `Timestamp.nanos` / `Duration.nanos`. + Kind::Int32 | Kind::Sint32 | Kind::Sfixed32 => ProtoValue::I32(500_000_000), + // Within the `Timestamp` range. + Kind::Int64 | Kind::Sint64 | Kind::Sfixed64 => ProtoValue::I64(1_700_000_000), + Kind::Uint32 | Kind::Fixed32 => ProtoValue::U32(u32::MAX), + Kind::Uint64 | Kind::Fixed64 => ProtoValue::U64(u64::MAX), + Kind::Bool => ProtoValue::Bool(true), + Kind::String => ProtoValue::String("sample".into()), + Kind::Bytes => ProtoValue::Bytes(vec![0, 0xff].into()), + Kind::Enum(e) => ProtoValue::EnumNumber(e.values().last().unwrap().number()), + Kind::Message(m) => ProtoValue::Message(sample_message(m, variant, depth - 1)), + } +} + +fn sample_key(kind: &Kind) -> MapKey { + match kind { + Kind::String => MapKey::String("k".into()), + Kind::Bool => MapKey::Bool(true), + Kind::Int32 | Kind::Sint32 | Kind::Sfixed32 => MapKey::I32(-7), + Kind::Int64 | Kind::Sint64 | Kind::Sfixed64 => MapKey::I64(-7), + Kind::Uint32 | Kind::Fixed32 => MapKey::U32(7), + Kind::Uint64 | Kind::Fixed64 => MapKey::U64(7), + other => unreachable!("not a map key kind: {other:?}"), + } +} + +/// A message with every field set: lists with two elements, maps with one +/// entry, and member `variant` (modulo their size) of each oneof. Below +/// `depth` levels a message field is left unset, and a oneof takes its first +/// member, so a recursive type (`Value` in `ListValue`) ends. +fn sample_message(desc: &MessageDescriptor, variant: usize, depth: usize) -> DynamicMessage { + let mut msg = DynamicMessage::new(desc.clone()); + for field in desc.fields() { + if let Some(oneof) = field.containing_oneof() { + let members: Vec<_> = oneof.fields().collect(); + let pick = if depth == 0 { + 0 + } else { + variant % members.len() + }; + if members[pick].number() != field.number() { + continue; + } + } + let message_typed = matches!(field.kind(), Kind::Message(_)); + if depth == 0 && message_typed { + continue; + } + let value = if field.is_map() { + let Kind::Message(entry) = field.kind() else { + unreachable!("a map field is a message of entries"); + }; + let key = sample_key(&entry.map_entry_key_field().kind()); + let value = sample_value(&entry.map_entry_value_field().kind(), variant, depth); + ProtoValue::Map([(key, value)].into_iter().collect()) + } else if field.is_list() { + let item = sample_value(&field.kind(), variant, depth); + ProtoValue::List(vec![item.clone(), item]) + } else { + sample_value(&field.kind(), variant, depth) + }; + msg.set_field(&field, value); + } + msg +} + +/// Check that `value` is valid against the OpenAPI 3.0.3 `schema`, resolving +/// `$ref` in `components`, and that every object key it carries is a declared +/// property or allowed by `additionalProperties`. `at` names the place for the +/// failure message. +fn check_wire(value: &Value, schema: &Value, components: &Value, at: &str) { + if let Some(reference) = schema["$ref"].as_str() { + let name = reference.strip_prefix("#/components/schemas/").unwrap(); + let target = &components[name]; + assert!(target.is_object(), "{at}: unresolved {reference}"); + return check_wire(value, target, components, at); + } + if let Some(allowed) = schema["enum"].as_array() { + assert!(allowed.contains(value), "{at}: {value} not in {schema}"); + } + let Some(ty) = schema["type"].as_str() else { + // No type: any value, `null` included (OpenAPI 3.0.3 `nullable`). + return; + }; + if value.is_null() { + assert_eq!(schema["nullable"], true, "{at}: null against {schema}"); + return; + } + match ty { + "string" => { + let s = value + .as_str() + .unwrap_or_else(|| panic!("{at}: {value} is not a string")); + match schema["format"].as_str() { + Some("int64") => assert!(s.parse::().is_ok(), "{at}: {s} is not int64"), + Some("uint64") => assert!(s.parse::().is_ok(), "{at}: {s} is not uint64"), + _ => {} + } + } + "integer" => { + let fits = match schema["format"].as_str() { + Some("int32") => value.as_i64().is_some_and(|n| i32::try_from(n).is_ok()), + Some("uint32") => value.as_u64().is_some_and(|n| u32::try_from(n).is_ok()), + _ => value.is_i64() || value.is_u64(), + }; + assert!(fits, "{at}: {value} does not fit {schema}"); + } + "number" => assert!(value.is_number(), "{at}: {value} is not a number"), + "boolean" => assert!(value.is_boolean(), "{at}: {value} is not a boolean"), + "array" => { + let items = value + .as_array() + .unwrap_or_else(|| panic!("{at}: {value} is not an array")); + for (i, item) in items.iter().enumerate() { + check_wire(item, &schema["items"], components, &format!("{at}[{i}]")); + } + } + "object" => { + let fields = value + .as_object() + .unwrap_or_else(|| panic!("{at}: {value} is not an object")); + let declared = schema["properties"].as_object(); + for (key, field) in fields { + let at = format!("{at}.{key}"); + if let Some(property) = declared.and_then(|p| p.get(key)) { + check_wire(field, property, components, &at); + continue; + } + match &schema["additionalProperties"] { + Value::Bool(true) => {} + extra @ Value::Object(_) => check_wire(field, extra, components, &at), + // A bare object (`Struct`) holds any keys. + Value::Null if declared.is_none() => {} + _ => panic!("{at}: not a declared property of {schema}"), + } + } + } + other => panic!("{at}: unknown type {other}"), + } +} + +#[test] +fn every_response_validates_against_its_schema() { + // The spec has to describe what the proxy writes: each sample `Wire` + // (every scalar, enum, well-known type, a `NullValue` alone, in a oneof, + // a list and a map) goes through the unary response serializer and must + // validate against the documented response schema. + let pool = test_pool(); + let spec = spec(&[]); + let components = &spec["components"]["schemas"]; + let schema = &spec["paths"]["/v1/wire"]["post"]["responses"]["200"]["content"] + ["application/json"]["schema"]; + let wire = pool.get_message_by_name("test.v1.Wire").unwrap(); + let choices = wire.oneofs().map(|o| o.fields().len()).max().unwrap(); + // `Value.kind` has six members: every one of them is sampled too. + for variant in 0..choices.max(6) { + let msg = sample_message(&wire, variant, 6); + let body = transcode::json_body(&msg, None).unwrap(); + let value: Value = serde_json::from_slice(&body).unwrap(); + check_wire(&value, schema, components, &format!("Wire#{variant}")); + } +} + +#[test] +fn null_value_is_json_null_in_bodies_and_a_name_in_parameters() { + // ProtoJSON writes a `NullValue` as `null`, alone, as a oneof member, in + // a list and as a map value; the query parser reads it by its enum name. + let spec = spec(&[]); + let wire = &spec["components"]["schemas"]["Wire"]["properties"]; + let only_null = json!({ "nullable": true, "enum": [null] }); + assert_eq!(wire["nothing"], only_null); + assert_eq!(wire["unset"], only_null); + assert_eq!( + wire["nulls"], + json!({ "type": "array", "items": only_null }) + ); + assert_eq!( + wire["nullMap"], + json!({ "type": "object", "additionalProperties": only_null }) + ); + assert!(spec["components"]["schemas"].get("NullValue").is_none()); + + let params = spec["paths"]["/v1/ping"]["get"]["parameters"] + .as_array() + .unwrap(); + let name = json!({ "type": "string", "enum": ["NULL_VALUE"] }); + assert_eq!(params[0]["schema"], name); + assert_eq!( + params[1]["schema"], + json!({ "type": "array", "items": name }) + ); +} + +#[test] +fn properties_are_keyed_by_the_json_name() { + // Responses carry `displayName`, not `display_name`. + let spec = spec(&[]); + let wire = spec["components"]["schemas"]["Wire"]["properties"] + .as_object() + .unwrap(); + assert!(wire.contains_key("displayName"), "{wire:?}"); + assert!(!wire.contains_key("display_name"), "{wire:?}"); +} diff --git a/src/transcode/mod.rs b/src/transcode/mod.rs index 3471c11..dc550d9 100644 --- a/src/transcode/mod.rs +++ b/src/transcode/mod.rs @@ -1193,7 +1193,7 @@ fn message_to_json_string(msg: &DynamicMessage, opts: &SerializeOptions) -> Resu /// The unary response body as JSON: the whole message, or the subfield /// `response_body` names (JSON `null` when the path does not exist). -fn json_body( +pub(crate) fn json_body( msg: &DynamicMessage, response_body: Option<&str>, ) -> Result, serde_json::Error> { From 8c6ec61165e0d400b1fc01324e483e2e826e58be Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 19 Oct 2026 11:02:44 +0300 Subject: [PATCH 2/5] fix(openapi): document query fields by their dotted keys - a field below a message-typed query field is its own parameter named by the dotted path the query parser reads (filter.q), typed as the leaf, instead of an object parameter whose $ref pointed at the JSON body schema; a nested NullValue is now NULL_VALUE there, not null - maps and repeated messages, which a query string cannot set, are no longer listed; a message already on the way is not entered again - the schema/wire test also checks a message with every field default Regression tests: message_typed_query_field_is_one_parameter_per_leaf, query_parameters_are_the_keys_the_parser_reads, and the nested case in null_value_is_json_null_in_bodies_and_a_name_in_parameters. --- src/openapi.rs | 57 +++++++++++++++++++++++---------- src/openapi/tests.rs | 75 +++++++++++++++++++++++++++++++------------- 2 files changed, 93 insertions(+), 39 deletions(-) diff --git a/src/openapi.rs b/src/openapi.rs index 217b117..0103a70 100644 --- a/src/openapi.rs +++ b/src/openapi.rs @@ -305,23 +305,9 @@ fn build_operation( }; // With `body: "*"` the whole message is the body: no query parameters. if binding.body != BodyMapping::Root { - params.extend( - input - .fields() - .filter(|f| { - !path_params.contains(&f.name().to_string()) && body_field != Some(f.name()) - }) - .map(|field| { - // A message-typed parameter refers to its schema by `$ref`. - register_nested(&field, schemas); - json!({ - "name": field.name(), - "in": "query", - "required": false, - "schema": field_to_schema(&field, Encoding::Param), - }) - }), - ); + let bound = |key: &str| path_params.iter().any(|p| p == key) || body_field == Some(key); + let mut on_the_way = vec![input.full_name().to_owned()]; + query_params(&input, "", &bound, &mut on_the_way, &mut params); } if !params.is_empty() { op["parameters"] = json!(params); @@ -408,6 +394,43 @@ fn build_operation( op } +/// Push a query parameter for every field below `msg` that `bound` (the path +/// and the body) leaves to the query, keyed by its dotted proto path as the +/// query parser reads it: a field of a message field is `filter.q`, not an +/// object `filter`. A map or a repeated message cannot be set from a query +/// string, and a message in `on_the_way` is not entered again, so a message +/// that contains itself ends. +fn query_params( + msg: &MessageDescriptor, + prefix: &str, + bound: &dyn Fn(&str) -> bool, + on_the_way: &mut Vec, + params: &mut Vec, +) { + for field in msg.fields() { + let key = format!("{prefix}{}", field.name()); + if bound(&key) || field.is_map() { + continue; + } + match field.kind() { + Kind::Message(inner) if !is_well_known(&inner) => { + if field.is_list() || on_the_way.iter().any(|m| m == inner.full_name()) { + continue; + } + on_the_way.push(inner.full_name().to_owned()); + query_params(&inner, &format!("{key}."), bound, on_the_way, params); + on_the_way.pop(); + } + _ => params.push(json!({ + "name": key, + "in": "query", + "required": false, + "schema": field_to_schema(&field, Encoding::Param), + })), + } + } +} + /// The field a (possibly dotted) `response_body` path names in `output`, /// walking singular message fields. fn response_field(output: &MessageDescriptor, path: &str) -> Option { diff --git a/src/openapi/tests.rs b/src/openapi/tests.rs index 60bfd84..42a6f10 100644 --- a/src/openapi/tests.rs +++ b/src/openapi/tests.rs @@ -213,6 +213,13 @@ message Wire { message NullQuery { google.protobuf.NullValue none = 1; repeated google.protobuf.NullValue nones = 2; + NullFilter filter = 3; + map tags = 4; + repeated Note notes = 5; + google.protobuf.Timestamp since = 6; +} +message NullFilter { + google.protobuf.NullValue none = 1; } service Api { @@ -424,36 +431,40 @@ fn body_rule_decides_body_and_query_fields() { assert_eq!(touch["parameters"].as_array().unwrap().len(), 3); } +/// The names of the query parameters of `operation`, in order. +fn query_names(operation: &Value) -> Vec<&str> { + operation["parameters"] + .as_array() + .unwrap() + .iter() + .filter(|p| p["in"] == "query") + .map(|p| p["name"].as_str().unwrap()) + .collect() +} + #[test] -fn message_typed_query_parameter_schema_is_registered() { - // `filter` is bound only through the query; its `$ref` must resolve to a - // schema in `components`. +fn message_typed_query_field_is_one_parameter_per_leaf() { + // The query parser reads a field below `filter` from the dotted key + // `filter.q`, so that is the parameter, typed as the leaf. let spec = spec(&[]); - let param = &spec["paths"]["/v1/find"]["get"]["parameters"][0]; - assert_eq!(param["in"], "query"); - assert_eq!(param["schema"]["$ref"], "#/components/schemas/Filter"); - assert!( - spec["components"]["schemas"].get("Filter").is_some(), - "{}", - spec["components"]["schemas"] - ); + let find = &spec["paths"]["/v1/find"]["get"]; + assert_eq!(query_names(find), ["filter.q"]); + assert_eq!(find["parameters"][0]["schema"], json!({ "type": "string" })); } #[test] fn self_referencing_message_does_not_recurse_forever() { - // `Tree.child` is a `Tree`: in the query (GET) and in the body (POST) its - // schema is registered once and referenced, instead of being expanded - // until the stack overflows. + // `Tree.child` is a `Tree`: in the body (POST) its schema is registered + // once and referenced, and in the query (GET) a message already on the + // way is not entered again, instead of being expanded until the stack + // overflows. let spec = spec(&[]); let tree = &spec["components"]["schemas"]["Tree"]; assert_eq!( tree["properties"]["child"]["$ref"], "#/components/schemas/Tree" ); - assert_eq!( - spec["paths"]["/v1/tree"]["get"]["parameters"][1]["schema"]["$ref"], - "#/components/schemas/Tree" - ); + assert_eq!(query_names(&spec["paths"]["/v1/tree"]["get"]), ["name"]); assert!(spec["paths"]["/v1/tree"]["post"]["requestBody"].is_object()); } @@ -699,12 +710,18 @@ fn every_response_validates_against_its_schema() { let value: Value = serde_json::from_slice(&body).unwrap(); check_wire(&value, schema, components, &format!("Wire#{variant}")); } + // Nothing set: every scalar and enum goes out with its default value. + let empty = DynamicMessage::new(wire); + let body = transcode::json_body(&empty, None).unwrap(); + let value: Value = serde_json::from_slice(&body).unwrap(); + check_wire(&value, schema, components, "Wire#default"); } #[test] fn null_value_is_json_null_in_bodies_and_a_name_in_parameters() { // ProtoJSON writes a `NullValue` as `null`, alone, as a oneof member, in - // a list and as a map value; the query parser reads it by its enum name. + // a list and as a map value; the query parser reads it by its enum name, + // below a message field too. let spec = spec(&[]); let wire = &spec["components"]["schemas"]["Wire"]["properties"]; let only_null = json!({ "nullable": true, "enum": [null] }); @@ -720,15 +737,29 @@ fn null_value_is_json_null_in_bodies_and_a_name_in_parameters() { ); assert!(spec["components"]["schemas"].get("NullValue").is_none()); - let params = spec["paths"]["/v1/ping"]["get"]["parameters"] - .as_array() - .unwrap(); + let ping = &spec["paths"]["/v1/ping"]["get"]; + let params = ping["parameters"].as_array().unwrap(); let name = json!({ "type": "string", "enum": ["NULL_VALUE"] }); assert_eq!(params[0]["schema"], name); assert_eq!( params[1]["schema"], json!({ "type": "array", "items": name }) ); + assert_eq!(params[2]["name"], "filter.none"); + assert_eq!(params[2]["schema"], name); +} + +#[test] +fn query_parameters_are_the_keys_the_parser_reads() { + // A map or a repeated message cannot be set from a query string, so it + // is no parameter; a well-known type is one, read from its string form. + let spec = spec(&[]); + let ping = &spec["paths"]["/v1/ping"]["get"]; + assert_eq!(query_names(ping), ["none", "nones", "filter.none", "since"]); + assert_eq!( + ping["parameters"][3]["schema"], + json!({ "type": "string", "format": "date-time" }) + ); } #[test] From e3769e0164e7d0270f1667bdbb8cce00ba2f1fb9 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 19 Oct 2026 11:23:12 +0300 Subject: [PATCH 3/5] fix(openapi): bind well-known types without a string form by field - a query parameter is a whole well-known type only when the parser can read it from one string (Timestamp, Duration, FieldMask, wrappers, Value); Any, Struct, ListValue and Empty are walked like any message, so Any shows payload.type_url and payload.value and ListValue shows its repeated values - a Value read from a query string is documented as a string Regression test: query_parameters_are_the_keys_the_parser_reads. --- src/openapi.rs | 23 ++++++++++++++++++++++- src/openapi/tests.rs | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/openapi.rs b/src/openapi.rs index 0103a70..163b39d 100644 --- a/src/openapi.rs +++ b/src/openapi.rs @@ -413,7 +413,7 @@ fn query_params( continue; } match field.kind() { - Kind::Message(inner) if !is_well_known(&inner) => { + Kind::Message(inner) if !has_string_form(&inner) => { if field.is_list() || on_the_way.iter().any(|m| m == inner.full_name()) { continue; } @@ -431,6 +431,21 @@ fn query_params( } } +/// Whether a query value can be a whole `msg`: a well-known type the parser +/// reads from one string (`Value` as its string value). `Any`, `Struct`, +/// `ListValue` and `Empty` have no string form, so their fields are bound +/// one by one, as any other message's. +fn has_string_form(msg: &MessageDescriptor) -> bool { + is_well_known(msg) + && !matches!( + msg.full_name(), + "google.protobuf.Any" + | "google.protobuf.Struct" + | "google.protobuf.ListValue" + | "google.protobuf.Empty" + ) +} + /// The field a (possibly dotted) `response_body` path names in `output`, /// walking singular message fields. fn response_field(output: &MessageDescriptor, path: &str) -> Option { @@ -548,6 +563,12 @@ fn field_to_schema(field: &FieldDescriptor, encoding: Encoding) -> Value { let values: Vec = e.values().map(|v| json!(v.name())).collect(); json!({ "type": "string", "enum": values }) } + // A parameter string becomes a `Value`'s string value. + Kind::Message(msg) + if encoding == Encoding::Param && msg.full_name() == "google.protobuf.Value" => + { + json!({ "type": "string" }) + } Kind::Message(msg) => { if is_well_known(&msg) { well_known_schema(&msg) diff --git a/src/openapi/tests.rs b/src/openapi/tests.rs index 42a6f10..01d7085 100644 --- a/src/openapi/tests.rs +++ b/src/openapi/tests.rs @@ -128,6 +128,7 @@ syntax = "proto3"; package test.v1; import "google/api/annotations.proto"; import "google/api/httpbody.proto"; +import "google/protobuf/any.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/empty.proto"; import "google/protobuf/field_mask.proto"; @@ -217,6 +218,11 @@ message NullQuery { map tags = 4; repeated Note notes = 5; google.protobuf.Timestamp since = 6; + google.protobuf.Any payload = 7; + google.protobuf.Struct attrs = 8; + google.protobuf.ListValue items = 9; + google.protobuf.Empty nothing = 10; + google.protobuf.Value loose = 11; } message NullFilter { google.protobuf.NullValue none = 1; @@ -752,14 +758,37 @@ fn null_value_is_json_null_in_bodies_and_a_name_in_parameters() { #[test] fn query_parameters_are_the_keys_the_parser_reads() { // A map or a repeated message cannot be set from a query string, so it - // is no parameter; a well-known type is one, read from its string form. + // is no parameter. A well-known type with a string form is one, read from + // that string; one without (`Any`, `Struct`, `ListValue`, `Empty`) is + // bound through its own fields, like any message. let spec = spec(&[]); let ping = &spec["paths"]["/v1/ping"]["get"]; - assert_eq!(query_names(ping), ["none", "nones", "filter.none", "since"]); assert_eq!( - ping["parameters"][3]["schema"], + query_names(ping), + [ + "none", + "nones", + "filter.none", + "since", + "payload.type_url", + "payload.value", + "items.values", + "loose" + ] + ); + let params = ping["parameters"].as_array().unwrap(); + assert_eq!( + params[3]["schema"], json!({ "type": "string", "format": "date-time" }) ); + // A `Value` read from a query string is its string value, each repeated + // key one more element of `ListValue.values`. + let string = json!({ "type": "string" }); + assert_eq!( + params[6]["schema"], + json!({ "type": "array", "items": string }) + ); + assert_eq!(params[7]["schema"], string); } #[test] From c34becef99fee6b41c6b15c9a6a242cf741843df Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 19 Oct 2026 12:13:02 +0300 Subject: [PATCH 4/5] fix(openapi): treat only special-JSON types as well-known Every google.protobuf message was handled as a well-known type by its package prefix, so an ordinary one such as SourceContext was an empty object in bodies and a single query parameter. Well-known now means the types ProtoJSON writes in their own form, the same list the request parser reads by; a query parameter is a whole message only for the explicit list of types with a string form. Regression tests: an_ordinary_google_protobuf_message_has_its_own_schema and the SourceContext case in query_parameters_are_the_keys_the_parser_reads. --- src/openapi.rs | 37 ++++++++++++++++++++----------- src/openapi/tests.rs | 24 +++++++++++++++++++- src/transcode/request.rs | 3 ++- src/transcode/request/presence.rs | 2 +- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/openapi.rs b/src/openapi.rs index 163b39d..1e9877f 100644 --- a/src/openapi.rs +++ b/src/openapi.rs @@ -11,7 +11,7 @@ use prost_reflect::{DescriptorPool, FieldDescriptor, Kind, MessageDescriptor, Me use serde_json::{json, Map, Value}; use crate::config::{AliasConfig, OpenApiConfig}; -use crate::transcode::request::BodyMapping; +use crate::transcode::request::{self, BodyMapping}; use crate::transcode::rule::{self, HttpBinding, RouteMethod}; use crate::transcode::RpcSelection; use crate::transcode::{self, httpbody, path}; @@ -432,18 +432,26 @@ fn query_params( } /// Whether a query value can be a whole `msg`: a well-known type the parser -/// reads from one string (`Value` as its string value). `Any`, `Struct`, -/// `ListValue` and `Empty` have no string form, so their fields are bound -/// one by one, as any other message's. +/// reads from one string (`Value` as its string value). Every other message, +/// `Any`, `Struct`, `ListValue` and `Empty` included, has its fields bound +/// one by one. fn has_string_form(msg: &MessageDescriptor) -> bool { - is_well_known(msg) - && !matches!( - msg.full_name(), - "google.protobuf.Any" - | "google.protobuf.Struct" - | "google.protobuf.ListValue" - | "google.protobuf.Empty" - ) + matches!( + msg.full_name(), + "google.protobuf.Timestamp" + | "google.protobuf.Duration" + | "google.protobuf.FieldMask" + | "google.protobuf.Value" + | "google.protobuf.DoubleValue" + | "google.protobuf.FloatValue" + | "google.protobuf.Int64Value" + | "google.protobuf.UInt64Value" + | "google.protobuf.Int32Value" + | "google.protobuf.UInt32Value" + | "google.protobuf.BoolValue" + | "google.protobuf.StringValue" + | "google.protobuf.BytesValue" + ) } /// The field a (possibly dotted) `response_body` path names in `output`, @@ -596,8 +604,11 @@ fn field_to_schema(field: &FieldDescriptor, encoding: Encoding) -> Value { } } +/// Whether ProtoJSON writes `msg` in its own form rather than as an object of +/// its fields: the list the request parser reads by. Any other message of the +/// `google.protobuf` package (`SourceContext`) is an ordinary message. fn is_well_known(msg: &MessageDescriptor) -> bool { - msg.full_name().starts_with("google.protobuf.") + request::has_special_json(msg) } fn well_known_schema(msg: &MessageDescriptor) -> Value { diff --git a/src/openapi/tests.rs b/src/openapi/tests.rs index 01d7085..10530ff 100644 --- a/src/openapi/tests.rs +++ b/src/openapi/tests.rs @@ -132,6 +132,7 @@ import "google/protobuf/any.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/empty.proto"; import "google/protobuf/field_mask.proto"; +import "google/protobuf/source_context.proto"; import "google/protobuf/struct.proto"; import "google/protobuf/timestamp.proto"; import "google/protobuf/wrappers.proto"; @@ -210,6 +211,8 @@ message Wire { google.protobuf.UInt64Value maybe_ulong = 42; google.protobuf.FloatValue maybe_float = 43; google.protobuf.DoubleValue maybe_double = 44; + // In the `google.protobuf` package, but an ordinary message on the wire. + google.protobuf.SourceContext origin = 45; } message NullQuery { google.protobuf.NullValue none = 1; @@ -223,6 +226,7 @@ message NullQuery { google.protobuf.ListValue items = 9; google.protobuf.Empty nothing = 10; google.protobuf.Value loose = 11; + google.protobuf.SourceContext source = 12; } message NullFilter { google.protobuf.NullValue none = 1; @@ -773,7 +777,8 @@ fn query_parameters_are_the_keys_the_parser_reads() { "payload.type_url", "payload.value", "items.values", - "loose" + "loose", + "source.file_name" ] ); let params = ping["parameters"].as_array().unwrap(); @@ -801,3 +806,20 @@ fn properties_are_keyed_by_the_json_name() { assert!(wire.contains_key("displayName"), "{wire:?}"); assert!(!wire.contains_key("display_name"), "{wire:?}"); } + +#[test] +fn an_ordinary_google_protobuf_message_has_its_own_schema() { + // Only the types with their own JSON form are described inline; any + // other message of the package is an object of its fields, as ProtoJSON + // writes it. + let spec = spec(&[]); + let schemas = &spec["components"]["schemas"]; + assert_eq!( + schemas["Wire"]["properties"]["origin"]["$ref"], + "#/components/schemas/SourceContext" + ); + assert_eq!( + schemas["SourceContext"]["properties"]["fileName"], + json!({ "type": "string" }) + ); +} diff --git a/src/transcode/request.rs b/src/transcode/request.rs index a66e1ca..2ddc6ca 100644 --- a/src/transcode/request.rs +++ b/src/transcode/request.rs @@ -18,7 +18,8 @@ use serde::de::value::StrDeserializer; use serde::Deserializer; use serde_json::Value as JsonValue; -use presence::{has_special_json, OneEntry, Presence, Recording}; +pub(crate) use presence::has_special_json; +use presence::{OneEntry, Presence, Recording}; /// The path parameters of a request: each dotted field path the path binds, /// with its value. diff --git a/src/transcode/request/presence.rs b/src/transcode/request/presence.rs index b1690c1..602d5b6 100644 --- a/src/transcode/request/presence.rs +++ b/src/transcode/request/presence.rs @@ -154,7 +154,7 @@ impl Presence { /// Whether prost-reflect reads `message` through its own JSON form rather than /// as a map of fields (its list of well-known types). -pub(super) fn has_special_json(message: &MessageDescriptor) -> bool { +pub(crate) fn has_special_json(message: &MessageDescriptor) -> bool { matches!( message.full_name(), "google.protobuf.Any" From 6ed39c3bc699075a69462ac57648a3b89de9b60d Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 19 Oct 2026 13:34:15 +0300 Subject: [PATCH 5/5] test(openapi): pin repeated string-form query parameters A repeated well-known type with a string form (repeated Timestamp, ListValue.values) is set from the query one value per repeated key, so it stays a documented parameter. A test now checks that the spec describes it as an array and that the parser binds every value, and the comments say which repeated messages a query string cannot set. --- src/openapi.rs | 9 ++++++--- src/openapi/tests.rs | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/openapi.rs b/src/openapi.rs index 1e9877f..bfe1d91 100644 --- a/src/openapi.rs +++ b/src/openapi.rs @@ -397,9 +397,9 @@ fn build_operation( /// Push a query parameter for every field below `msg` that `bound` (the path /// and the body) leaves to the query, keyed by its dotted proto path as the /// query parser reads it: a field of a message field is `filter.q`, not an -/// object `filter`. A map or a repeated message cannot be set from a query -/// string, and a message in `on_the_way` is not entered again, so a message -/// that contains itself ends. +/// object `filter`. A map, or a repeated message without a string form, cannot +/// be set from a query string, and a message in `on_the_way` is not entered +/// again, so a message that contains itself ends. fn query_params( msg: &MessageDescriptor, prefix: &str, @@ -421,6 +421,9 @@ fn query_params( query_params(&inner, &format!("{key}."), bound, on_the_way, params); on_the_way.pop(); } + // Scalars, enums and string-form messages, repeated ones included: + // each repeated key is one more element (`seen=…&seen=…`), the + // parser reading every value as the element's string form. _ => params.push(json!({ "name": key, "in": "query", diff --git a/src/openapi/tests.rs b/src/openapi/tests.rs index 10530ff..ae83a24 100644 --- a/src/openapi/tests.rs +++ b/src/openapi/tests.rs @@ -227,6 +227,7 @@ message NullQuery { google.protobuf.Empty nothing = 10; google.protobuf.Value loose = 11; google.protobuf.SourceContext source = 12; + repeated google.protobuf.Timestamp seen = 13; } message NullFilter { google.protobuf.NullValue none = 1; @@ -778,7 +779,8 @@ fn query_parameters_are_the_keys_the_parser_reads() { "payload.value", "items.values", "loose", - "source.file_name" + "source.file_name", + "seen" ] ); let params = ping["parameters"].as_array().unwrap(); @@ -807,6 +809,40 @@ fn properties_are_keyed_by_the_json_name() { assert!(!wire.contains_key("display_name"), "{wire:?}"); } +#[test] +fn a_repeated_string_form_type_is_one_parameter_per_value() { + // Unlike a repeated ordinary message, a repeated well-known type with a + // string form is set from the query, one value per repeated key; the + // parser and the spec have to agree on that. + let pool = test_pool(); + let spec = spec(&[]); + let params = spec["paths"]["/v1/ping"]["get"]["parameters"] + .as_array() + .unwrap(); + let seen = params.iter().find(|p| p["name"] == "seen").unwrap(); + assert_eq!( + seen["schema"], + json!({ "type": "array", "items": { "type": "string", "format": "date-time" } }) + ); + + let input = pool.get_message_by_name("test.v1.NullQuery").unwrap(); + let query = "seen=2020-01-01T00:00:00Z&seen=2021-01-01T00:00:00Z&items.values=a&items.values=b"; + let message = transcode::request::build_request_message( + &input, + &BodyMapping::None, + transcode::request::Body::Absent, + &std::collections::HashMap::new(), + Some(query), + ) + .unwrap(); + let value = serde_json::to_value(&message).unwrap(); + assert_eq!( + value["seen"], + json!(["2020-01-01T00:00:00Z", "2021-01-01T00:00:00Z"]) + ); + assert_eq!(value["items"], json!(["a", "b"])); +} + #[test] fn an_ordinary_google_protobuf_message_has_its_own_schema() { // Only the types with their own JSON form are described inline; any