From 7951fef44012ecc3bba1d997f66978f907c6ddac Mon Sep 17 00:00:00 2001 From: Peter Marsh Date: Fri, 5 Jun 2026 13:30:34 +0200 Subject: [PATCH 1/4] Add tests for non-spec-compliant scalar bool string parsing The Proto JSON specification (https://protobuf.dev/programming-guides/json/) requires that bool fields accept only the JSON literals true and false. The transcoder also accepts string values for bool fields, performing case-insensitive matching: "true", "yes", "TRUE", "TrUe" all parse as true, and "false", "no", "FALSE", "NO" all parse as false. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/bookstore.proto | 3 ++ test/json_request_translator_test.cc | 58 ++++++++++++++++++++++++++ test/testdata/bookstore_service.pb.txt | 12 ++++++ 3 files changed, 73 insertions(+) diff --git a/test/bookstore.proto b/test/bookstore.proto index ce73ff1..6de7153 100644 --- a/test/bookstore.proto +++ b/test/bookstore.proto @@ -73,3 +73,6 @@ message DeleteBookRequest { int64 shelf = 1; int64 book = 2; } +message ScalarBoolMessage { + bool b = 1; +} diff --git a/test/json_request_translator_test.cc b/test/json_request_translator_test.cc index f3d9a91..e9f0edd 100644 --- a/test/json_request_translator_test.cc +++ b/test/json_request_translator_test.cc @@ -794,6 +794,64 @@ TEST_F(JsonRequestTranslatorTest, StreamingErrorNotAnArray) { absl::StatusCode::kInvalidArgument)); } +// Scalar bool fields accept string values. +// The proto JSON spec requires bool fields to only accept JSON true/false +// literals, but the transcoder also accepts string values like "true", "yes", +// "false", "no" (case-insensitive). +TEST_F(JsonRequestTranslatorTest, ScalarBoolAcceptsStringTrue) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("ScalarBoolMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": "true"})", "b: true"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, ScalarBoolAcceptsStringFalse) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("ScalarBoolMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": "false"})", "b: false"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, ScalarBoolAcceptsStringYes) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("ScalarBoolMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": "yes"})", "b: true"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, ScalarBoolAcceptsStringNo) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("ScalarBoolMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": "no"})", "b: false"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, ScalarBoolAcceptsStringCaseInsensitive) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("ScalarBoolMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": "TrUe"})", "b: true"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, ScalarBoolAcceptsStringNOUpperCase) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("ScalarBoolMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": "NO"})", "b: false"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + } // namespace } // namespace testing } // namespace transcoding diff --git a/test/testdata/bookstore_service.pb.txt b/test/testdata/bookstore_service.pb.txt index dd00e94..5c07c93 100644 --- a/test/testdata/bookstore_service.pb.txt +++ b/test/testdata/bookstore_service.pb.txt @@ -455,6 +455,18 @@ types { file_name: "wrappers.proto" } } +types { + name: "ScalarBoolMessage" + fields { + kind: TYPE_BOOL + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "b" + json_name: "b" + } + source_context { + } +} enums { name: "google.protobuf.NullValue" enumvalue { From 4bc746b1495aebacc1127f5c06f9d92d4a576479 Mon Sep 17 00:00:00 2001 From: Peter Marsh Date: Fri, 5 Jun 2026 14:07:54 +0200 Subject: [PATCH 2/4] Add tests for non-spec-compliant wrapper type object form parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Proto JSON specification requires that well-known wrapper types (google.protobuf.BoolValue, google.protobuf.StringValue, etc.) be represented as their underlying JSON scalar. For example, a BoolValue field should be true or false directly, not {"value": true}. The transcoder accepts the object form {"value": } for wrapper types, treating it as a valid representation. It also applies the non-spec-compliant string-to-bool parsing within the wrapper, so {"b": {"value": "yes"}} is accepted for a BoolValue field. Extra keys alongside "value" are silently ignored. Tests cover all nine well-known wrapper types: BoolValue, StringValue, BytesValue, Int32Value, Int64Value, UInt32Value, UInt64Value, FloatValue, and DoubleValue. When a wrapper type field receives an object with NO "value" key, the transcoder correctly rejects it with INVALID_ARGUMENT — tests for this correct behaviour are also included. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/BUILD | 1 + test/bookstore.proto | 28 ++++ test/json_request_translator_test.cc | 149 +++++++++++++++++ test/testdata/bookstore_service.pb.txt | 221 +++++++++++++++++++++++++ 4 files changed, 399 insertions(+) diff --git a/test/BUILD b/test/BUILD index 657b77f..3c32892 100644 --- a/test/BUILD +++ b/test/BUILD @@ -114,6 +114,7 @@ proto_library( name = "bookstore_proto", testonly = 1, srcs = ["bookstore.proto"], + deps = ["@com_google_protobuf//:wrappers_proto"], ) cc_proto_library( diff --git a/test/bookstore.proto b/test/bookstore.proto index 6de7153..0abf7c5 100644 --- a/test/bookstore.proto +++ b/test/bookstore.proto @@ -18,6 +18,7 @@ // Test proto for transcoding syntax = "proto3"; package google.grpc.transcoding; +import "google/protobuf/wrappers.proto"; message Biography { int64 year_born = 1; int64 year_died = 2; @@ -76,3 +77,30 @@ message DeleteBookRequest { message ScalarBoolMessage { bool b = 1; } +message BoolValueMessage { + google.protobuf.BoolValue b = 1; +} +message StringValueMessage { + google.protobuf.StringValue s = 1; +} +message BytesValueMessage { + google.protobuf.BytesValue b = 1; +} +message Int32ValueMessage { + google.protobuf.Int32Value i = 1; +} +message Int64ValueMessage { + google.protobuf.Int64Value i = 1; +} +message UInt32ValueMessage { + google.protobuf.UInt32Value u = 1; +} +message UInt64ValueMessage { + google.protobuf.UInt64Value u = 1; +} +message FloatValueMessage { + google.protobuf.FloatValue f = 1; +} +message DoubleValueMessage { + google.protobuf.DoubleValue d = 1; +} diff --git a/test/json_request_translator_test.cc b/test/json_request_translator_test.cc index e9f0edd..724e504 100644 --- a/test/json_request_translator_test.cc +++ b/test/json_request_translator_test.cc @@ -852,6 +852,155 @@ TEST_F(JsonRequestTranslatorTest, ScalarBoolAcceptsStringNOUpperCase) { EXPECT_TRUE((RunTest(1, 1.0, &tc))); } +// Wrapper types accept the object/message form {"value": ...}. +// The proto JSON spec requires wrapper types (e.g. google.protobuf.BoolValue) +// to be represented as their underlying scalar, not as {"value": }. + +TEST_F(JsonRequestTranslatorTest, BoolValueAcceptsObjectFormTrue) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("BoolValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": {"value": true}})", "b { value: true }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, BoolValueAcceptsObjectFormFalse) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("BoolValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": {"value": false}})", "b { value: false }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, StringValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("StringValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"s": {"value": "oh no"}})", R"(s { value: "oh no" })"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, BytesValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("BytesValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": {"value": "dGVzdA=="}})", R"(b { value: "test" })"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, Int32ValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("Int32ValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"i": {"value": 42}})", "i { value: 42 }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, Int64ValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("Int64ValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"i": {"value": "42"}})", "i { value: 42 }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, UInt32ValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("UInt32ValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"u": {"value": 42}})", "u { value: 42 }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, UInt64ValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("UInt64ValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"u": {"value": "42"}})", "u { value: 42 }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, FloatValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("FloatValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"f": {"value": 3.14}})", "f { value: 3.14 }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, DoubleValueAcceptsObjectForm) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("DoubleValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"d": {"value": 2.718}})", "d { value: 2.718 }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +// Behaviors 1+2 combined: wrapper types in object form also accept string +// bool values. +TEST_F(JsonRequestTranslatorTest, BoolValueObjectFormAcceptsStringYes) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("BoolValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": {"value": "yes"}})", "b { value: true }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +TEST_F(JsonRequestTranslatorTest, BoolValueObjectFormAcceptsStringNo) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("BoolValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": {"value": "no"}})", "b { value: false }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +// Extra keys alongside "value" are silently ignored. +TEST_F(JsonRequestTranslatorTest, BoolValueObjectFormExtraKeysIgnored) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("BoolValueMessage"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"b": {"value": true, "extra": "ignored"}})", + "b { value: true }"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +// When a wrapper type field receives an object with NO "value" key, the +// transcoder correctly rejects it with INVALID_ARGUMENT. +TEST_F(JsonRequestTranslatorTest, BoolValueRejectsArbitraryObject) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("BoolValueMessage"); + Build(); + AddChunk(R"({"b": {"foo": "bar", "baz": 123}})"); + Finish(); + EXPECT_TRUE(Tester().ExpectNone()); + EXPECT_TRUE( + Tester().ExpectStatusEq(absl::StatusCode::kInvalidArgument)); +} + +TEST_F(JsonRequestTranslatorTest, StringValueRejectsArbitraryObject) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("StringValueMessage"); + Build(); + AddChunk(R"({"s": {"name": "test", "count": 42}})"); + Finish(); + EXPECT_TRUE(Tester().ExpectNone()); + EXPECT_TRUE( + Tester().ExpectStatusEq(absl::StatusCode::kInvalidArgument)); +} + } // namespace } // namespace testing } // namespace transcoding diff --git a/test/testdata/bookstore_service.pb.txt b/test/testdata/bookstore_service.pb.txt index 5c07c93..16bc397 100644 --- a/test/testdata/bookstore_service.pb.txt +++ b/test/testdata/bookstore_service.pb.txt @@ -455,6 +455,110 @@ types { file_name: "wrappers.proto" } } +types { + name: "google.protobuf.StringValue" + fields { + kind: TYPE_STRING + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} +types { + name: "google.protobuf.BytesValue" + fields { + kind: TYPE_BYTES + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} +types { + name: "google.protobuf.Int32Value" + fields { + kind: TYPE_INT32 + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} +types { + name: "google.protobuf.Int64Value" + fields { + kind: TYPE_INT64 + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} +types { + name: "google.protobuf.UInt32Value" + fields { + kind: TYPE_UINT32 + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} +types { + name: "google.protobuf.UInt64Value" + fields { + kind: TYPE_UINT64 + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} +types { + name: "google.protobuf.FloatValue" + fields { + kind: TYPE_FLOAT + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} +types { + name: "google.protobuf.DoubleValue" + fields { + kind: TYPE_DOUBLE + cardinality: CARDINALITY_REQUIRED + number: 1 + name: "value" + json_name: "value" + } + source_context { + file_name: "wrappers.proto" + } +} types { name: "ScalarBoolMessage" fields { @@ -467,6 +571,123 @@ types { source_context { } } +types { + name: "BoolValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "b" + type_url: "type.googleapis.com/google.protobuf.BoolValue" + json_name: "b" + } + source_context { + } +} +types { + name: "StringValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "s" + type_url: "type.googleapis.com/google.protobuf.StringValue" + json_name: "s" + } + source_context { + } +} +types { + name: "BytesValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "b" + type_url: "type.googleapis.com/google.protobuf.BytesValue" + json_name: "b" + } + source_context { + } +} +types { + name: "Int32ValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "i" + type_url: "type.googleapis.com/google.protobuf.Int32Value" + json_name: "i" + } + source_context { + } +} +types { + name: "Int64ValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "i" + type_url: "type.googleapis.com/google.protobuf.Int64Value" + json_name: "i" + } + source_context { + } +} +types { + name: "UInt32ValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "u" + type_url: "type.googleapis.com/google.protobuf.UInt32Value" + json_name: "u" + } + source_context { + } +} +types { + name: "UInt64ValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "u" + type_url: "type.googleapis.com/google.protobuf.UInt64Value" + json_name: "u" + } + source_context { + } +} +types { + name: "FloatValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "f" + type_url: "type.googleapis.com/google.protobuf.FloatValue" + json_name: "f" + } + source_context { + } +} +types { + name: "DoubleValueMessage" + fields { + kind: TYPE_MESSAGE + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "d" + type_url: "type.googleapis.com/google.protobuf.DoubleValue" + json_name: "d" + } + source_context { + } +} enums { name: "google.protobuf.NullValue" enumvalue { From eaa76e33ea9b320492dcd4b84dca859008e75c45 Mon Sep 17 00:00:00 2001 From: Peter Marsh Date: Fri, 5 Jun 2026 14:16:39 +0200 Subject: [PATCH 3/4] Add tests for body/binding collision detection using proto field name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a request has both a JSON body and query parameters (variable bindings) targeting the same field, the RequestWeaver's CollisionCheck determines whether the body value takes precedence. It compares the raw JSON key name against the binding's proto field name (Field::name()). Because the JSON stream parser passes the original JSON key through the ObjectWriter chain (not the resolved proto field name), collision is only detected when the body uses the proto field name. When the body uses the json_name (auto-generated camelCase or custom), the collision is not detected and the query parameter silently overrides the body value. Additionally, RenderNull does not call CollisionCheck, so null body values never suppress bindings — the query parameter always wins. Given a message: message Example { string snake_case_field = 1; // json_name: snakeCaseField string custom_field = 2 [json_name = "myCustomName"]; // json_name: myCustomName } And a POST to /example?snake_case_field=query: | Body JSON | Result | Why | |----------------------------------|---------------------------|--------------------------------------| | {"snake_case_field": "body"} | snake_case_field = "body" | Proto name matches, body wins | | {"snakeCaseField": "body"} | snake_case_field = "query"| json_name doesn't match, query wins | | {"snake_case_field": ""} | snake_case_field = "" | Empty string still present, body wins| | {"snake_case_field": null} | snake_case_field = "query"| RenderNull skips CollisionCheck | And a POST to /example?custom_field=query: | Body JSON | Result | Why | |----------------------------------|---------------------------|--------------------------------------| | {"myCustomName": "body"} | custom_field = "query" | Custom json_name doesn't match | Co-Authored-By: Claude Opus 4.6 (1M context) --- test/bookstore.proto | 4 ++ test/json_request_translator_test.cc | 70 ++++++++++++++++++++++++++ test/testdata/bookstore_service.pb.txt | 19 +++++++ 3 files changed, 93 insertions(+) diff --git a/test/bookstore.proto b/test/bookstore.proto index 0abf7c5..fd778bf 100644 --- a/test/bookstore.proto +++ b/test/bookstore.proto @@ -104,3 +104,7 @@ message FloatValueMessage { message DoubleValueMessage { google.protobuf.DoubleValue d = 1; } +message FieldNamingMessage { + string snake_case_field = 1; + string custom_field = 2 [json_name = "myCustomName"]; +} diff --git a/test/json_request_translator_test.cc b/test/json_request_translator_test.cc index 724e504..4bb43db 100644 --- a/test/json_request_translator_test.cc +++ b/test/json_request_translator_test.cc @@ -1001,6 +1001,76 @@ TEST_F(JsonRequestTranslatorTest, StringValueRejectsArbitraryObject) { Tester().ExpectStatusEq(absl::StatusCode::kInvalidArgument)); } +// Body presence detection with variable bindings (query parameters). +// When a JSON body and a query parameter target the same field, the weaver's +// CollisionCheck determines which value wins. CollisionCheck compares the raw +// JSON key name against the binding's proto field name (Field::name()). This +// means collision is only detected when the body uses the proto field name, not +// the json_name. + +// Proto field name in body → collision detected → body wins. +TEST_F(JsonRequestTranslatorTest, BodyProtoNameWinsOverBinding) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("FieldNamingMessage"); + AddVariableBinding("snake_case_field", "query"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"snake_case_field": "body"})", + R"(snake_case_field: "body")"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +// json_name (camelCase) in body → collision NOT detected → binding is weaved +// after body → query param silently overrides body value. +TEST_F(JsonRequestTranslatorTest, BodyJsonNameOverriddenByBinding) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("FieldNamingMessage"); + AddVariableBinding("snake_case_field", "query"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"snakeCaseField": "body"})", + R"(snake_case_field: "query")"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +// Custom json_name in body → collision NOT detected → binding overrides. +TEST_F(JsonRequestTranslatorTest, BodyCustomJsonNameOverriddenByBinding) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("FieldNamingMessage"); + AddVariableBinding("custom_field", "query"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"myCustomName": "body"})", + R"(custom_field: "query")"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +// Empty string body value using proto name → collision detected → body wins +// (default value is still considered present). +TEST_F(JsonRequestTranslatorTest, BodyEmptyStringWinsOverBinding) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("FieldNamingMessage"); + AddVariableBinding("snake_case_field", "query"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"snake_case_field": ""})", + R"(snake_case_field: "")"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + +// Null body value → RenderNull does not call CollisionCheck → binding survives +// and is weaved → query param wins. +TEST_F(JsonRequestTranslatorTest, BodyNullAllowsBindingToWin) { + LoadService("bookstore_service.pb.txt"); + SetMessageType("FieldNamingMessage"); + AddVariableBinding("snake_case_field", "query"); + TranslationTestCase tc(false); + tc.AddMessage(R"({"snake_case_field": null})", + R"(snake_case_field: "query")"); + tc.Build(); + EXPECT_TRUE((RunTest(1, 1.0, &tc))); +} + } // namespace } // namespace testing } // namespace transcoding diff --git a/test/testdata/bookstore_service.pb.txt b/test/testdata/bookstore_service.pb.txt index 16bc397..1ef6564 100644 --- a/test/testdata/bookstore_service.pb.txt +++ b/test/testdata/bookstore_service.pb.txt @@ -688,6 +688,25 @@ types { source_context { } } +types { + name: "FieldNamingMessage" + fields { + kind: TYPE_STRING + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "snake_case_field" + json_name: "snakeCaseField" + } + fields { + kind: TYPE_STRING + cardinality: CARDINALITY_OPTIONAL + number: 2 + name: "custom_field" + json_name: "myCustomName" + } + source_context { + } +} enums { name: "google.protobuf.NullValue" enumvalue { From d3fbbde38e15271cd96277719e5e3587b02f5ecd Mon Sep 17 00:00:00 2001 From: Peter Marsh Date: Fri, 5 Jun 2026 14:18:00 +0200 Subject: [PATCH 4/4] Add tests for float/double integer values rendered without decimal point MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a float or double proto field holds a value that is exactly an integer (e.g. 100.0, 1000.0, 42.0), the transcoder serializes it to JSON as the bare integer (100, 1000, 42) with no trailing ".0". This is valid JSON per the Proto JSON specification, but may surprise clients in languages where 100 (integer) and 100.0 (float) are distinct types — a client deserializing the response may interpret the field as an integer rather than a floating-point number. Co-Authored-By: Claude Opus 4.6 (1M context) --- test/bookstore.proto | 4 ++ test/response_to_json_translator_test.cc | 67 ++++++++++++++++++++++++ test/testdata/bookstore_service.pb.txt | 19 +++++++ 3 files changed, 90 insertions(+) diff --git a/test/bookstore.proto b/test/bookstore.proto index fd778bf..b7dd8be 100644 --- a/test/bookstore.proto +++ b/test/bookstore.proto @@ -108,3 +108,7 @@ message FieldNamingMessage { string snake_case_field = 1; string custom_field = 2 [json_name = "myCustomName"]; } +message FloatDoubleMessage { + float f = 1; + double d = 2; +} diff --git a/test/response_to_json_translator_test.cc b/test/response_to_json_translator_test.cc index 2b58f1f..74f8454 100644 --- a/test/response_to_json_translator_test.cc +++ b/test/response_to_json_translator_test.cc @@ -1110,6 +1110,73 @@ TEST_F(ResponseToJsonTranslatorTest, IncompleteFrame) { "Incomplete gRPC frame expected size: 5 actual size: 1"); } +// Float and double fields set to integer values are serialized as integers in +// JSON (e.g. 100, not 100.0). This uses direct string matching because +// ExpectJsonObjectEq normalizes numbers and would treat 100 and 100.0 as equal. +TEST_F(ResponseToJsonTranslatorTest, FloatIntegerValueNoDecimalPoint) { + ::google::api::Service service; + ASSERT_TRUE( + transcoding::testing::LoadService("bookstore_service.pb.txt", &service)); + TypeHelper type_helper(service.types(), service.enums()); + + TestZeroCopyInputStream input_stream; + ResponseToJsonTranslator translator( + type_helper.Resolver(), "type.googleapis.com/FloatDoubleMessage", false, + &input_stream); + + input_stream.AddChunk( + GenerateGrpcMessage(R"(f: 100 d: 1000)")); + input_stream.Finish(); + + std::string message; + EXPECT_TRUE(translator.NextMessage(&message)); + EXPECT_NE(std::string::npos, message.find("100")); + EXPECT_NE(std::string::npos, message.find("1000")); + EXPECT_EQ(std::string::npos, message.find("100.0")); + EXPECT_EQ(std::string::npos, message.find("1000.0")); +} + +TEST_F(ResponseToJsonTranslatorTest, DoubleIntegerValueNoDecimalPoint) { + ::google::api::Service service; + ASSERT_TRUE( + transcoding::testing::LoadService("bookstore_service.pb.txt", &service)); + TypeHelper type_helper(service.types(), service.enums()); + + TestZeroCopyInputStream input_stream; + ResponseToJsonTranslator translator( + type_helper.Resolver(), "type.googleapis.com/FloatDoubleMessage", false, + &input_stream); + + input_stream.AddChunk( + GenerateGrpcMessage(R"(d: 42)")); + input_stream.Finish(); + + std::string message; + EXPECT_TRUE(translator.NextMessage(&message)); + EXPECT_NE(std::string::npos, message.find("42")); + EXPECT_EQ(std::string::npos, message.find("42.0")); +} + +TEST_F(ResponseToJsonTranslatorTest, FloatDoubleNonIntegerValueHasDecimal) { + ::google::api::Service service; + ASSERT_TRUE( + transcoding::testing::LoadService("bookstore_service.pb.txt", &service)); + TypeHelper type_helper(service.types(), service.enums()); + + TestZeroCopyInputStream input_stream; + ResponseToJsonTranslator translator( + type_helper.Resolver(), "type.googleapis.com/FloatDoubleMessage", false, + &input_stream); + + input_stream.AddChunk( + GenerateGrpcMessage(R"(f: 3.14 d: 2.718)")); + input_stream.Finish(); + + std::string message; + EXPECT_TRUE(translator.NextMessage(&message)); + EXPECT_NE(std::string::npos, message.find(".")); +} + } // namespace } // namespace testing } // namespace transcoding diff --git a/test/testdata/bookstore_service.pb.txt b/test/testdata/bookstore_service.pb.txt index 1ef6564..d253ec1 100644 --- a/test/testdata/bookstore_service.pb.txt +++ b/test/testdata/bookstore_service.pb.txt @@ -707,6 +707,25 @@ types { source_context { } } +types { + name: "FloatDoubleMessage" + fields { + kind: TYPE_FLOAT + cardinality: CARDINALITY_OPTIONAL + number: 1 + name: "f" + json_name: "f" + } + fields { + kind: TYPE_DOUBLE + cardinality: CARDINALITY_OPTIONAL + number: 2 + name: "d" + json_name: "d" + } + source_context { + } +} enums { name: "google.protobuf.NullValue" enumvalue {