From b6e4e78f4fa7fb239197101990dc895b4ef0e6ef Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:35:30 +0000 Subject: [PATCH 1/9] Add FloatFromDouble: checked double-to-float narrowing (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A finite double beyond float range is UB to static_cast ([conv.double]); generated deserializers need the check in one tested place. NaN and ±Infinity pass through — they are legal Smithy float values on every wire. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- runtime/include/smithy/core/document_serde.h | 5 +++ runtime/src/core/document_serde.cc | 11 ++++++ runtime/tests/core/document_serde_test.cc | 36 ++++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/runtime/include/smithy/core/document_serde.h b/runtime/include/smithy/core/document_serde.h index 0282e0e7..5afa1395 100644 --- a/runtime/include/smithy/core/document_serde.h +++ b/runtime/include/smithy/core/document_serde.h @@ -24,6 +24,11 @@ Outcome BlobFromDocument(const Document& doc); // "Infinity", "-Infinity". Outcome DoubleFromDocument(const Document& doc); +// Checked double→float narrowing for generated float members: a finite value +// beyond float range is rejected (the raw cast would be undefined behavior), +// NaN and ±Infinity pass through with their float spellings. +Outcome FloatFromDouble(double value); + // Shortest round-trip decimal text (std::to_chars); non-finite values render // as the Smithy wire spellings "NaN" / "Infinity" / "-Infinity". Used for // HTTP label/query/header bindings and JSON bodies. diff --git a/runtime/src/core/document_serde.cc b/runtime/src/core/document_serde.cc index bb6ea350..a535c5a5 100644 --- a/runtime/src/core/document_serde.cc +++ b/runtime/src/core/document_serde.cc @@ -53,6 +53,17 @@ Outcome DoubleFromDocument(const Document& doc) { return Error::Serialization("number: expected a number or NaN/Infinity/-Infinity"); } +Outcome FloatFromDouble(double value) { + // Only finite out-of-range values are rejected: casting those is undefined + // behavior ([conv.double]), while NaN and ±Infinity narrow losslessly and + // are legal Smithy float values on every wire. + if (std::isfinite(value) && + (value < -std::numeric_limits::max() || value > std::numeric_limits::max())) { + return Error::Serialization("number: value out of float range"); + } + return static_cast(value); +} + namespace { template std::string FormatFloating(T value) { diff --git a/runtime/tests/core/document_serde_test.cc b/runtime/tests/core/document_serde_test.cc index a9d7b68e..aa23bc94 100644 --- a/runtime/tests/core/document_serde_test.cc +++ b/runtime/tests/core/document_serde_test.cc @@ -83,6 +83,42 @@ TEST(DocumentSerdeTest, DoubleFromDocumentAcceptsNonFiniteSpellings) { EXPECT_FALSE(DoubleFromDocument(Document(true)).ok()); } +TEST(DocumentSerdeTest, FloatFromDoubleNarrowsInRangeValues) { + EXPECT_EQ(*FloatFromDouble(1.5), 1.5F); + EXPECT_EQ(*FloatFromDouble(-2.25), -2.25F); + EXPECT_EQ(*FloatFromDouble(0.0), 0.0F); + // The exact float extremes are in range, not rejected. + EXPECT_EQ(*FloatFromDouble(std::numeric_limits::max()), std::numeric_limits::max()); + EXPECT_EQ(*FloatFromDouble(-std::numeric_limits::max()), + -std::numeric_limits::max()); + // Values below float precision round (here: to zero) — rounding is not an + // error, only magnitude overflow is. + EXPECT_EQ(*FloatFromDouble(1e-300), 0.0F); +} + +TEST(DocumentSerdeTest, FloatFromDoubleRejectsFiniteOverflow) { + // The raw static_cast would be UB for these ([conv.double]); a hostile + // request body carrying 1e300 for a float member must fail the parse. + EXPECT_FALSE(FloatFromDouble(1e300).ok()); + EXPECT_FALSE(FloatFromDouble(-1e300).ok()); + EXPECT_FALSE(FloatFromDouble(std::numeric_limits::max()).ok()); + // Just past the float edge in double precision is already overflow. + EXPECT_FALSE( + FloatFromDouble(std::nextafter(static_cast(std::numeric_limits::max()), + std::numeric_limits::infinity())) + .ok()); +} + +TEST(DocumentSerdeTest, FloatFromDoublePassesNonFiniteThrough) { + // Smithy float carries NaN/±Infinity on every wire; narrowing them is + // well-defined and must not be caught in the overflow net. + EXPECT_TRUE(std::isnan(*FloatFromDouble(std::numeric_limits::quiet_NaN()))); + EXPECT_EQ(*FloatFromDouble(std::numeric_limits::infinity()), + std::numeric_limits::infinity()); + EXPECT_EQ(*FloatFromDouble(-std::numeric_limits::infinity()), + -std::numeric_limits::infinity()); +} + TEST(DocumentSerdeTest, FormatFloatingPoint) { EXPECT_EQ(FormatDouble(4.1), "4.1"); EXPECT_EQ(FormatFloat(4.1F), "4.1"); From 16cb7fb803758f12cbd2b345ef353e6717cfc544 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:35:40 +0000 Subject: [PATCH 2/9] Range-check intEnum and float wire values instead of truncating (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit intEnum merges into the bounded-integer serde case: its underlying type is int32, so a wire int64 of 2^32+2 silently aliased onto a valid enumerator. float rejects finite doubles beyond float range via smithy::FloatFromDouble on both the document-body path and the query/label/header text path — the raw cast was UB that UBSan's float-cast-overflow aborts on. Unknown in-range intEnum values still parse, matching string enums' tolerant reads. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- .../smithycpp/codegen/HttpBindingCodeGen.java | 6 ++- .../io/smithycpp/codegen/SerdeCodeGen.java | 26 +++++++++---- .../codegen/HttpJsonBindingProtocolTest.java | 38 +++++++++++++++++++ .../smithycpp/codegen/SerdeGeneratorTest.java | 37 ++++++++++++++++++ examples/cafe/generated/src/serde.cc | 4 +- .../roundtrip/jsonrpc/generated/src/serde.cc | 5 ++- .../roundtrip/rest/generated/src/serde.cc | 5 ++- examples/roundtrip/rpc/generated/src/serde.cc | 5 ++- .../simplerestjson/generated/src/serde.cc | 4 +- examples/weather/generated/src/serde.cc | 12 ++++-- .../rpcv2cbor/generated/src/serde.cc | 27 ++++++++++--- .../simplerestjson/generated/src/serde.cc | 6 ++- 12 files changed, 151 insertions(+), 24 deletions(-) diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/HttpBindingCodeGen.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/HttpBindingCodeGen.java index a8f28739..e8ed676f 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/HttpBindingCodeGen.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/HttpBindingCodeGen.java @@ -582,9 +582,13 @@ private static boolean writeSimpleTextParse( "$Lstatic_cast<$L>(*parsed_num)$L", open, context.cppSymbols().typeRef(target), close); } case FLOAT -> { + // Checked narrowing: a finite text value beyond float range would be + // UB to cast (UBSan float-cast-overflow), so it fails the parse. w.write("auto parsed_num = helpers::ParseDoubleText($L);", valueExpr); w.write("if (!parsed_num) return std::move(parsed_num).error();"); - w.write("$Lstatic_cast(*parsed_num)$L", open, close); + w.write("auto narrowed_num = smithy::FloatFromDouble(*parsed_num);"); + w.write("if (!narrowed_num) return std::move(narrowed_num).error();"); + w.write("$L*narrowed_num$L", open, close); } case DOUBLE -> { w.write("auto parsed_num = helpers::ParseDoubleText($L);", valueExpr); diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/SerdeCodeGen.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/SerdeCodeGen.java index 75c75a8d..e669b9f5 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/SerdeCodeGen.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/SerdeCodeGen.java @@ -195,12 +195,13 @@ void writeDeserializeInto( w.write("if (!$L->is_bool()) $L", docExpr, wrong); w.write("$L = $L->as_bool();", outExpr, docExpr); } - case BYTE, SHORT, INTEGER, LONG -> { + case BYTE, SHORT, INTEGER, LONG, INT_ENUM -> { String type = context.cppSymbols().typeRef(shape); w.write("if (!$L->is_int()) $L", docExpr, wrong); if (shape.getType() != software.amazon.smithy.model.shapes.ShapeType.LONG) { - // Narrower integers reject out-of-range wire values instead of - // truncating (the malformed-request suite pins this). + // Narrower integers — intEnum included, its underlying type is + // int32 — reject out-of-range wire values instead of truncating + // (the malformed-request suite pins this). String bounds = switch (shape.getType()) { case BYTE -> "-128 || " + docExpr + "->as_int() > 127"; @@ -216,12 +217,21 @@ void writeDeserializeInto( } w.write("$L = static_cast<$L>($L->as_int());", outExpr, type, docExpr); } - case INT_ENUM -> { - String type = context.cppSymbols().typeRef(shape); - w.write("if (!$L->is_int()) $L", docExpr, wrong); - w.write("$L = static_cast<$L>($L->as_int());", outExpr, type, docExpr); + case FLOAT -> { + // The double→float narrowing is checked: a finite wire value beyond + // float range would be UB to cast (UBSan float-cast-overflow). + w.openBlock("{"); + w.write("auto parsed = smithy::DoubleFromDocument(*$L);", docExpr); + w.write( + "if (!parsed) return smithy::Error::Serialization($S);", path + ": expected a number"); + w.write("auto narrowed = smithy::FloatFromDouble(*parsed);"); + w.write( + "if (!narrowed) return smithy::Error::Serialization($S);", + path + ": value out of range"); + w.write("$L = *narrowed;", outExpr); + w.closeBlock("}"); } - case FLOAT, DOUBLE -> { + case DOUBLE -> { String type = context.cppSymbols().typeRef(shape); w.openBlock("{"); w.write("auto parsed = smithy::DoubleFromDocument(*$L);", docExpr); diff --git a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/HttpJsonBindingProtocolTest.java b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/HttpJsonBindingProtocolTest.java index 12acf01e..883f88e9 100644 --- a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/HttpJsonBindingProtocolTest.java +++ b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/HttpJsonBindingProtocolTest.java @@ -133,6 +133,44 @@ void serverParsesTheInverseOfTheClientRequest() { assertTrue(server.contains("body_doc->Find(\"body\")"), server); } + @Test + void floatTextBindingsNarrowThroughTheCheckedHelper() { + // No checked-in fixture binds a float to a text position, so the golden + // trees can't pin this: a finite query/label/header value beyond float + // range must fail the parse via smithy::FloatFromDouble instead of + // hitting the UB static_cast (issue #109). Doubles stay unnarrowed. + String server = + PluginTestHarness.generate( + """ + $version: "2.0" + namespace test.rest + + use alloy#simpleRestJson + + @simpleRestJson + service Nums { version: "1", operations: [GetStats] } + + @http(method: "GET", uri: "/stats") + operation GetStats { + input := { + @httpQuery("ratio") + ratio: Float + + @httpQuery("precise") + precise: Double + } + output := { info: String } + } + """, + "test.rest#Nums", + "test::rest") + .expectFileString("/src/server.cc"); + assertTrue(server.contains("smithy::FloatFromDouble(*parsed_num)"), server); + assertTrue( + server.contains("if (!narrowed_num) return std::move(narrowed_num).error();"), server); + assertEquals(1, count(server, "FloatFromDouble"), server); + } + @Test void serverRoutesCarryContentNegotiationAndErrorIdentity() { String server = generateFiles().expectFileString("/src/server.cc"); diff --git a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/SerdeGeneratorTest.java b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/SerdeGeneratorTest.java index afe3d533..4db17667 100644 --- a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/SerdeGeneratorTest.java +++ b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/SerdeGeneratorTest.java @@ -77,6 +77,43 @@ void unionsEnforceExactlyOneMemberToleratingTheTypeKey() { assertTrue(serde.contains("unknown or missing union member"), serde); } + @Test + void narrowedNumericsRejectOutOfRangeWireValuesInsteadOfTruncating() { + // intEnum shares the int32 bounds check with Integer members (its + // underlying type is int32 — an unchecked cast would alias 2^32+2 onto a + // valid enumerator), and float rejects finite doubles beyond float range + // (the raw cast is UB; UBSan float-cast-overflow). Double stays + // uncheckable by construction. Issue #109. + String serde = + generateSerde( + """ + $version: "2.0" + namespace test.serde + + service Svc { version: "1", operations: [Op] } + operation Op { input := { payload: Payload } } + + structure Payload { + weight: Weight + ratio: Float + precise: Double + } + + intEnum Weight { + LIGHT = 1 + HEAVY = 2 + } + """); + int intEnumCheck = serde.indexOf("Payload.weight: value out of range"); + int intEnumCast = serde.indexOf("static_cast(member->as_int())"); + assertTrue(intEnumCheck >= 0, serde); + assertTrue(intEnumCast >= 0, serde); + assertTrue(intEnumCheck < intEnumCast, "range check must precede the narrowing cast"); + assertTrue(serde.contains("smithy::FloatFromDouble"), serde); + assertTrue(serde.contains("Payload.ratio: value out of range"), serde); + assertFalse(serde.contains("Payload.precise: value out of range"), serde); + } + @Test void timestampFormatTraitOverridesTheProtocolDefault() { String serde = generateSerde(KITCHEN_MODEL); diff --git a/examples/cafe/generated/src/serde.cc b/examples/cafe/generated/src/serde.cc index bd7ddad0..74dfef5b 100644 --- a/examples/cafe/generated/src/serde.cc +++ b/examples/cafe/generated/src/serde.cc @@ -242,7 +242,9 @@ smithy::Outcome DeserializeDairyMilk(const smithy::Document& doc) { { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("DairyMilk.percentFat: expected a number"); - out.percentFat = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("DairyMilk.percentFat: value out of range"); + out.percentFat = *narrowed; } } return out; diff --git a/examples/roundtrip/jsonrpc/generated/src/serde.cc b/examples/roundtrip/jsonrpc/generated/src/serde.cc index d5c1147d..3cedb185 100644 --- a/examples/roundtrip/jsonrpc/generated/src/serde.cc +++ b/examples/roundtrip/jsonrpc/generated/src/serde.cc @@ -317,7 +317,9 @@ smithy::Outcome DeserializeKitchenSink(const smithy::Document& doc) { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("KitchenSink.ratio: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("KitchenSink.ratio: value out of range"); + parsed_member = *narrowed; } out.ratio = std::move(parsed_member); } @@ -360,6 +362,7 @@ smithy::Outcome DeserializeKitchenSink(const smithy::Document& doc) if (member != nullptr && !member->is_null()) { types::Weight parsed_member{}; if (!member->is_int()) return smithy::Error::Serialization("KitchenSink.weight: unexpected type on the wire"); + if (member->as_int() < -2147483648LL || member->as_int() > 2147483647LL) return smithy::Error::Serialization("KitchenSink.weight: value out of range"); parsed_member = static_cast(member->as_int()); out.weight = std::move(parsed_member); } diff --git a/examples/roundtrip/rest/generated/src/serde.cc b/examples/roundtrip/rest/generated/src/serde.cc index b7acaa4c..b5e6c258 100644 --- a/examples/roundtrip/rest/generated/src/serde.cc +++ b/examples/roundtrip/rest/generated/src/serde.cc @@ -357,7 +357,9 @@ smithy::Outcome DeserializeKitchenSink(const smithy::Document& doc) { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("KitchenSink.ratio: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("KitchenSink.ratio: value out of range"); + parsed_member = *narrowed; } out.ratio = std::move(parsed_member); } @@ -400,6 +402,7 @@ smithy::Outcome DeserializeKitchenSink(const smithy::Document& doc) if (member != nullptr && !member->is_null()) { types::Weight parsed_member{}; if (!member->is_int()) return smithy::Error::Serialization("KitchenSink.weight: unexpected type on the wire"); + if (member->as_int() < -2147483648LL || member->as_int() > 2147483647LL) return smithy::Error::Serialization("KitchenSink.weight: value out of range"); parsed_member = static_cast(member->as_int()); out.weight = std::move(parsed_member); } diff --git a/examples/roundtrip/rpc/generated/src/serde.cc b/examples/roundtrip/rpc/generated/src/serde.cc index 697a7386..307fa106 100644 --- a/examples/roundtrip/rpc/generated/src/serde.cc +++ b/examples/roundtrip/rpc/generated/src/serde.cc @@ -317,7 +317,9 @@ smithy::Outcome DeserializeKitchenSink(const smithy::Document& doc) { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("KitchenSink.ratio: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("KitchenSink.ratio: value out of range"); + parsed_member = *narrowed; } out.ratio = std::move(parsed_member); } @@ -360,6 +362,7 @@ smithy::Outcome DeserializeKitchenSink(const smithy::Document& doc) if (member != nullptr && !member->is_null()) { types::Weight parsed_member{}; if (!member->is_int()) return smithy::Error::Serialization("KitchenSink.weight: unexpected type on the wire"); + if (member->as_int() < -2147483648LL || member->as_int() > 2147483647LL) return smithy::Error::Serialization("KitchenSink.weight: value out of range"); parsed_member = static_cast(member->as_int()); out.weight = std::move(parsed_member); } diff --git a/examples/simplerestjson/generated/src/serde.cc b/examples/simplerestjson/generated/src/serde.cc index acee2cf8..28432927 100644 --- a/examples/simplerestjson/generated/src/serde.cc +++ b/examples/simplerestjson/generated/src/serde.cc @@ -164,7 +164,9 @@ smithy::Outcome DeserializeGetBookOutput(const smithy::Document& { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("GetBookOutput.price: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("GetBookOutput.price: value out of range"); + parsed_member = *narrowed; } out.price = std::move(parsed_member); } diff --git a/examples/weather/generated/src/serde.cc b/examples/weather/generated/src/serde.cc index cad52d16..83e027b0 100644 --- a/examples/weather/generated/src/serde.cc +++ b/examples/weather/generated/src/serde.cc @@ -97,7 +97,9 @@ smithy::Outcome DeserializeGetForecastOutput(const smithy::Do { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("GetForecastOutput.chanceOfRain: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("GetForecastOutput.chanceOfRain: value out of range"); + parsed_member = *narrowed; } out.chanceOfRain = std::move(parsed_member); } @@ -143,7 +145,9 @@ smithy::Outcome DeserializeCityCoordinates(const smithy::Docume { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("CityCoordinates.latitude: expected a number"); - out.latitude = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("CityCoordinates.latitude: value out of range"); + out.latitude = *narrowed; } } { @@ -154,7 +158,9 @@ smithy::Outcome DeserializeCityCoordinates(const smithy::Docume { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("CityCoordinates.longitude: expected a number"); - out.longitude = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("CityCoordinates.longitude: value out of range"); + out.longitude = *narrowed; } } return out; diff --git a/protocol-tests/rpcv2cbor/generated/src/serde.cc b/protocol-tests/rpcv2cbor/generated/src/serde.cc index 58f9e3ed..f53a3b7a 100644 --- a/protocol-tests/rpcv2cbor/generated/src/serde.cc +++ b/protocol-tests/rpcv2cbor/generated/src/serde.cc @@ -364,7 +364,9 @@ smithy::Outcome DeserializeDefaults(const smithy::Document& doc) { { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("Defaults.defaultFloat: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("Defaults.defaultFloat: value out of range"); + parsed_member = *narrowed; } out.defaultFloat = std::move(parsed_member); } @@ -407,6 +409,7 @@ smithy::Outcome DeserializeDefaults(const smithy::Document& doc) { if (member != nullptr && !member->is_null()) { types::TestIntEnum parsed_member{}; if (!member->is_int()) return smithy::Error::Serialization("Defaults.defaultIntEnum: unexpected type on the wire"); + if (member->as_int() < -2147483648LL || member->as_int() > 2147483647LL) return smithy::Error::Serialization("Defaults.defaultIntEnum: value out of range"); parsed_member = static_cast(member->as_int()); out.defaultIntEnum = std::move(parsed_member); } @@ -487,7 +490,9 @@ smithy::Outcome DeserializeDefaults(const smithy::Document& doc) { { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("Defaults.zeroFloat: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("Defaults.zeroFloat: value out of range"); + parsed_member = *narrowed; } out.zeroFloat = std::move(parsed_member); } @@ -1052,7 +1057,9 @@ smithy::Outcome DeserializeOperationWithDefaultsOut { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("OperationWithDefaultsOutput.defaultFloat: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("OperationWithDefaultsOutput.defaultFloat: value out of range"); + parsed_member = *narrowed; } out.defaultFloat = std::move(parsed_member); } @@ -1095,6 +1102,7 @@ smithy::Outcome DeserializeOperationWithDefaultsOut if (member != nullptr && !member->is_null()) { types::TestIntEnum parsed_member{}; if (!member->is_int()) return smithy::Error::Serialization("OperationWithDefaultsOutput.defaultIntEnum: unexpected type on the wire"); + if (member->as_int() < -2147483648LL || member->as_int() > 2147483647LL) return smithy::Error::Serialization("OperationWithDefaultsOutput.defaultIntEnum: value out of range"); parsed_member = static_cast(member->as_int()); out.defaultIntEnum = std::move(parsed_member); } @@ -1175,7 +1183,9 @@ smithy::Outcome DeserializeOperationWithDefaultsOut { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("OperationWithDefaultsOutput.zeroFloat: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("OperationWithDefaultsOutput.zeroFloat: value out of range"); + parsed_member = *narrowed; } out.zeroFloat = std::move(parsed_member); } @@ -1531,6 +1541,7 @@ smithy::Outcome> DeserializeIntegerEnumList(const smith if (item->is_null()) return smithy::Error::Serialization("std::vector: null element in a dense list"); IntegerEnum parsed_item{}; if (!item->is_int()) return smithy::Error::Serialization("std::vector[]: unexpected type on the wire"); + if (item->as_int() < -2147483648LL || item->as_int() > 2147483647LL) return smithy::Error::Serialization("std::vector[]: value out of range"); parsed_item = static_cast(item->as_int()); out.push_back(std::move(parsed_item)); } @@ -2536,7 +2547,9 @@ smithy::Outcome DeserializeSimpleScalarPropertiesIn { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("SimpleScalarPropertiesInput.floatValue: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("SimpleScalarPropertiesInput.floatValue: value out of range"); + parsed_member = *narrowed; } out.floatValue = std::move(parsed_member); } @@ -2679,7 +2692,9 @@ smithy::Outcome DeserializeSimpleScalarPropertiesO { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("SimpleScalarPropertiesOutput.floatValue: expected a number"); - parsed_member = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("SimpleScalarPropertiesOutput.floatValue: value out of range"); + parsed_member = *narrowed; } out.floatValue = std::move(parsed_member); } diff --git a/protocol-tests/simplerestjson/generated/src/serde.cc b/protocol-tests/simplerestjson/generated/src/serde.cc index f08c35ef..05415c72 100644 --- a/protocol-tests/simplerestjson/generated/src/serde.cc +++ b/protocol-tests/simplerestjson/generated/src/serde.cc @@ -171,7 +171,9 @@ smithy::Outcome DeserializeMenuItem(const smithy::Document& doc) { { auto parsed = smithy::DoubleFromDocument(*member); if (!parsed) return smithy::Error::Serialization("MenuItem.price: expected a number"); - out.price = static_cast(*parsed); + auto narrowed = smithy::FloatFromDouble(*parsed); + if (!narrowed) return smithy::Error::Serialization("MenuItem.price: value out of range"); + out.price = *narrowed; } } return out; @@ -478,6 +480,7 @@ smithy::Outcome DeserializeGetIntEnumInput(const smithy::Docume return smithy::Error::Serialization("GetIntEnumInput: missing required member: aa"); } if (!member->is_int()) return smithy::Error::Serialization("GetIntEnumInput.aa: unexpected type on the wire"); + if (member->as_int() < -2147483648LL || member->as_int() > 2147483647LL) return smithy::Error::Serialization("GetIntEnumInput.aa: value out of range"); out.aa = static_cast(member->as_int()); } return out; @@ -498,6 +501,7 @@ smithy::Outcome DeserializeGetIntEnumOutput(const smithy::Docu return smithy::Error::Serialization("GetIntEnumOutput: missing required member: result"); } if (!member->is_int()) return smithy::Error::Serialization("GetIntEnumOutput.result: unexpected type on the wire"); + if (member->as_int() < -2147483648LL || member->as_int() > 2147483647LL) return smithy::Error::Serialization("GetIntEnumOutput.result: value out of range"); out.result = static_cast(member->as_int()); } return out; From 88d562c8418a2e08645ee1db7ae4d7ec826ad118 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:43:05 +0000 Subject: [PATCH 3/9] Validate intEnum membership in servers, matching string enums (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit String enums already fail request validation outside the modeled value set; intEnum members were accepted silently. Same suite-exact ValidationException message with the value set spelled in ints (the smithy-rs convention), same @internal policy: wire-valid but unadvertised. The generated smoke and response suites promptly caught that a default-constructed intEnum (0) is not usually a member — minimal test values now pick the first modeled member, exactly as string enums always did. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- .../codegen/NodeLiteralGenerator.java | 17 ++++++-- .../codegen/ValidationGenerator.java | 41 ++++++++++++++++++- .../codegen/ValidationGeneratorTest.java | 34 +++++++++++++++ docs/server-guide.md | 3 +- .../roundtrip/jsonrpc/generated/src/server.cc | 6 +++ .../roundtrip/rest/generated/src/server.cc | 6 +++ .../roundtrip/rpc/generated/src/server.cc | 6 +++ .../rpcv2cbor/generated/src/server.cc | 19 +++++++++ .../simplerestjson/generated/src/server.cc | 11 +++++ .../generated/tests/server_request_tests.cc | 1 + .../generated/tests/server_response_tests.cc | 2 + .../generated/tests/smoke_test.cc | 2 + .../malformed/server_malformed_test.cc | 36 ++++++++++++++++ 13 files changed, 178 insertions(+), 6 deletions(-) diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/NodeLiteralGenerator.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/NodeLiteralGenerator.java index cc897454..7be4d931 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/NodeLiteralGenerator.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/NodeLiteralGenerator.java @@ -190,10 +190,10 @@ String minimalExpression(Shape shape) { private String minimalExpression(Shape shape, MemberShape member) { return switch (shape.getType()) { case BOOLEAN -> "false"; - case BYTE, SHORT, INTEGER, LONG, FLOAT, DOUBLE, INT_ENUM -> - minimalNumberExpression(shape, member); + case BYTE, SHORT, INTEGER, LONG, FLOAT, DOUBLE -> minimalNumberExpression(shape, member); case STRING -> minimalStringExpression(shape, member); case ENUM -> minimalEnumExpression(shape); + case INT_ENUM -> minimalIntEnumExpression(shape); case BLOB -> "smithy::Blob()"; case TIMESTAMP -> "smithy::Timestamp::FromEpochMilliseconds(0)"; case DOCUMENT -> "smithy::Document(smithy::DocumentMap{})"; @@ -226,7 +226,6 @@ private String minimalNumberExpression(Shape shape, MemberShape member) { .orElse(java.math.BigDecimal.ZERO); String plain = value.stripTrailingZeros().toPlainString(); return switch (shape.getType()) { - case INT_ENUM -> "static_cast<" + typeName(shape) + ">(" + plain + ")"; case LONG -> plain + "LL"; case FLOAT -> (plain.contains(".") ? plain : plain + ".0") + "F"; case DOUBLE -> plain.contains(".") ? plain : plain + ".0"; @@ -272,6 +271,14 @@ private String minimalEnumExpression(Shape shape) { return typeName(shape) + "::FromString(" + CppLiterals.stringLiteral(first) + ")"; } + private String minimalIntEnumExpression(Shape shape) { + // The first modeled member, not 0: a default-constructed intEnum is only + // valid when 0 happens to be in the value set, and servers validate + // membership (issue #109). + Integer first = shape.asIntEnumShape().orElseThrow().getEnumValues().values().iterator().next(); + return "static_cast<" + typeName(shape) + ">(" + first + ")"; + } + private String minimalStructureExpression(StructureShape shape) { if (shape.getId().toString().equals("smithy.api#Unit")) { return "smithy::Unit{}"; @@ -320,7 +327,9 @@ private boolean needsExplicitMinimal( } boolean constrainedDefault = switch (shape.getType()) { - case ENUM -> true; + // intEnum joins enum: the default-constructed value (0 / unknown) + // fails server-side membership validation (issue #109). + case ENUM, INT_ENUM -> true; case STRING -> effective(shape, member, software.amazon.smithy.model.traits.LengthTrait.class) .flatMap(software.amazon.smithy.model.traits.LengthTrait::getMin) diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java index f0972fc8..3e1ee3c9 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/ValidationGenerator.java @@ -130,7 +130,8 @@ private boolean memberHasConstraints(MemberShape member) { || hasTrait(member, target, RangeTrait.class) || hasTrait(member, target, PatternTrait.class) || hasTrait(member, target, UniqueItemsTrait.class) - || target.isEnumShape(); + || target.isEnumShape() + || target.isIntEnumShape(); } private static boolean hasTrait(MemberShape member, Shape target, Class t) { @@ -418,6 +419,11 @@ private void writeValueChecks( : valueExpr; writeEnumCheck(w, memberTarget, checked, pathVar); } + if (memberTarget.isIntEnumShape()) { + // intEnum can't be a map key (keys are strings), so enumAsRawString + // never applies here. + writeIntEnumCheck(w, memberTarget, valueExpr, pathVar); + } } private static String plainNumber(BigDecimal value) { @@ -646,6 +652,39 @@ private void writeUniqueItemsCheck(CppWriter w, String valueExpr, String pathVar w.closeBlock("}"); } + private void writeIntEnumCheck(CppWriter w, Shape target, String valueExpr, String pathVar) { + var shape = target.asIntEnumShape().orElseThrow(); + String type = context.cppSymbols().toSymbol(target).getName(); + // Validity spans every member — @internal included, mirroring string + // enums, whose internal members FromString to a real enumerator and pass + // the kUnknown check — while the advertised set in the message omits + // them (same policy as writeEnumCheck). + String condition = + shape.members().stream() + .map( + member -> + valueExpr + + " != " + + type + + "::" + + TypeGenerators.enumConstant(member.getMemberName())) + .collect(java.util.stream.Collectors.joining(" && ")); + String set = + shape.members().stream() + .filter( + member -> !member.hasTrait(software.amazon.smithy.model.traits.InternalTrait.class)) + .map(member -> String.valueOf(shape.getEnumValues().get(member.getMemberName()))) + .collect(java.util.stream.Collectors.joining(", ")); + w.openBlock("if ($L) {", condition); + w.write( + "helpers::AddValidationFailure(failures, $L, \"Value at '\" + $L + \"' failed to satisfy " + + "constraint: Member must satisfy enum value set: [$L]\");", + pathVar, + pathVar, + set); + w.closeBlock("}"); + } + private void writeEnumCheck(CppWriter w, Shape target, String valueExpr, String pathVar) { // @internal enum members stay valid on the wire but are omitted from the // advertised value set (the suite pins this, incl. legacy @enum internal tags). diff --git a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/ValidationGeneratorTest.java b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/ValidationGeneratorTest.java index 64d36c6f..cea810f5 100644 --- a/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/ValidationGeneratorTest.java +++ b/codegen/smithy-cpp-codegen/src/test/java/io/smithycpp/codegen/ValidationGeneratorTest.java @@ -104,4 +104,38 @@ void lengthCountsCodePointsForStringsAndElementsForCollections() { assertTrue(server.contains("(*value.tags).size()"), server); assertTrue(server.contains("Member must have length greater than or equal to 1"), server); } + + @Test + void intEnumMembershipFollowsTheStringEnumPolicy() { + // intEnum members validate against the value set the way string enums do + // (issue #109; upstream suite message form, malformed-enum.smithy + // convention with the int spellings smithy-rs/-typescript emit): validity + // spans every member — @internal included — while the advertised set in + // the message omits internal members. + String server = + generateServer( + """ + $version: "2.0" + namespace test.validation + use smithy.cpp.protocols#jsonRpc2 + + @jsonRpc2 + service Svc { version: "1", operations: [Op] } + operation Op { input := { weight: Weight } } + + intEnum Weight { + LIGHT = 1 + HEAVY = 2 + + @internal + LEGACY = 99 + } + """); + assertTrue(server.contains("!= Weight::kLight"), server); + assertTrue(server.contains("!= Weight::kHeavy"), server); + assertTrue(server.contains("!= Weight::kLegacy"), server); + assertTrue( + server.contains("failed to satisfy constraint: Member must satisfy enum value set: [1, 2]"), + server); + } } diff --git a/docs/server-guide.md b/docs/server-guide.md index 1219f3f5..2f4537dc 100644 --- a/docs/server-guide.md +++ b/docs/server-guide.md @@ -495,7 +495,8 @@ way the chat example's e2e tests do. Inputs are validated against the model's constraint traits after parsing and before your handler runs: `@required` (top-level body/query/header members), `@length` (strings count -Unicode code points), `@range`, `@pattern`, `@uniqueItems`, and enum membership, recursively +Unicode code points), `@range`, `@pattern`, `@uniqueItems`, and enum membership (string enums +and intEnums alike), recursively through structures, unions, lists, and maps. Failures never reach the handler — the server responds with the standard 400 `ValidationException` wire shape (`message` summary plus a `fieldList` of per-member `{path, message}` entries, JSON-pointer paths like `/list/0`), with diff --git a/examples/roundtrip/jsonrpc/generated/src/server.cc b/examples/roundtrip/jsonrpc/generated/src/server.cc index 723d24d1..f2819687 100644 --- a/examples/roundtrip/jsonrpc/generated/src/server.cc +++ b/examples/roundtrip/jsonrpc/generated/src/server.cc @@ -92,6 +92,12 @@ void ValidateKitchenSink(const types::KitchenSink& value, const std::string& pat helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [low, medium, high]"); } } + if (value.weight.has_value()) { + const std::string member_path = path + "/weight"; + if ((*value.weight) != Weight::kLight && (*value.weight) != Weight::kHeavy) { + helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [1, 2]"); + } + } if (value.uniqueNames.has_value()) { const std::string member_path = path + "/uniqueNames"; { diff --git a/examples/roundtrip/rest/generated/src/server.cc b/examples/roundtrip/rest/generated/src/server.cc index 054ae45a..59f02815 100644 --- a/examples/roundtrip/rest/generated/src/server.cc +++ b/examples/roundtrip/rest/generated/src/server.cc @@ -177,6 +177,12 @@ void ValidateKitchenSink(const types::KitchenSink& value, const std::string& pat helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [low, medium, high]"); } } + if (value.weight.has_value()) { + const std::string member_path = path + "/weight"; + if ((*value.weight) != Weight::kLight && (*value.weight) != Weight::kHeavy) { + helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [1, 2]"); + } + } if (value.uniqueNames.has_value()) { const std::string member_path = path + "/uniqueNames"; { diff --git a/examples/roundtrip/rpc/generated/src/server.cc b/examples/roundtrip/rpc/generated/src/server.cc index e342c28f..80885397 100644 --- a/examples/roundtrip/rpc/generated/src/server.cc +++ b/examples/roundtrip/rpc/generated/src/server.cc @@ -81,6 +81,12 @@ void ValidateKitchenSink(const types::KitchenSink& value, const std::string& pat helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [low, medium, high]"); } } + if (value.weight.has_value()) { + const std::string member_path = path + "/weight"; + if ((*value.weight) != Weight::kLight && (*value.weight) != Weight::kHeavy) { + helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [1, 2]"); + } + } if (value.uniqueNames.has_value()) { const std::string member_path = path + "/uniqueNames"; { diff --git a/protocol-tests/rpcv2cbor/generated/src/server.cc b/protocol-tests/rpcv2cbor/generated/src/server.cc index 5e89f5a7..0d59b19a 100644 --- a/protocol-tests/rpcv2cbor/generated/src/server.cc +++ b/protocol-tests/rpcv2cbor/generated/src/server.cc @@ -92,6 +92,12 @@ void ValidateDefaults(const types::Defaults& value, const std::string& path, std helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [FOO, BAR, BAZ]"); } } + { + const std::string member_path = path + "/defaultIntEnum"; + if (value.defaultIntEnum != TestIntEnum::kOne && value.defaultIntEnum != TestIntEnum::kTwo) { + helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [1, 2]"); + } + } } void ValidateDenseSetMap(const std::map>& value, const std::string& path, std::vector* failures) { @@ -134,6 +140,15 @@ void ValidateFooEnumList(const std::vector& value, const std::string& p } } +void ValidateIntegerEnumList(const std::vector& value, const std::string& path, std::vector* failures) { + for (std::size_t i = 0; i < value.size(); ++i) { + const std::string item_path = path + "/" + std::to_string(i); + if (value[i] != IntegerEnum::kA && value[i] != IntegerEnum::kB && value[i] != IntegerEnum::kC) { + helpers::AddValidationFailure(failures, item_path, "Value at '" + item_path + "' failed to satisfy constraint: Member must satisfy enum value set: [1, 2, 3]"); + } + } +} + void ValidateRpcV2CborListsInput(const types::RpcV2CborListsInput& value, const std::string& path, std::vector* failures) { if (value.stringSet.has_value()) { const std::string member_path = path + "/stringSet"; @@ -153,6 +168,10 @@ void ValidateRpcV2CborListsInput(const types::RpcV2CborListsInput& value, const const std::string member_path = path + "/enumList"; helpers::ValidateFooEnumList((*value.enumList), member_path, failures); } + if (value.intEnumList.has_value()) { + const std::string member_path = path + "/intEnumList"; + helpers::ValidateIntegerEnumList((*value.intEnumList), member_path, failures); + } } void ValidateSparseSetMap(const std::map>>& value, const std::string& path, std::vector* failures) { diff --git a/protocol-tests/simplerestjson/generated/src/server.cc b/protocol-tests/simplerestjson/generated/src/server.cc index bd378769..30ac90db 100644 --- a/protocol-tests/simplerestjson/generated/src/server.cc +++ b/protocol-tests/simplerestjson/generated/src/server.cc @@ -260,6 +260,15 @@ void ValidateGetEnumInput(const types::GetEnumInput& value, const std::string& p } } +void ValidateGetIntEnumInput(const types::GetIntEnumInput& value, const std::string& path, std::vector* failures) { + { + const std::string member_path = path + "/aa"; + if (value.aa != EnumResult::kFirst && value.aa != EnumResult::kSecond) { + helpers::AddValidationFailure(failures, member_path, "Value at '" + member_path + "' failed to satisfy constraint: Member must satisfy enum value set: [1, 2]"); + } + } +} + void ValidateHealthInput(const types::HealthInput& value, const std::string& path, std::vector* failures) { if (value.query.has_value()) { const std::string member_path = path + "/query"; @@ -781,6 +790,8 @@ PizzaAdminServiceServer::PizzaAdminServiceServer(std::shared_ptrGetIntEnum(*input, context); if (!outcome) return helpers::ErrorToResponse(outcome.error()); return helpers::BuildGetIntEnumResponse(*outcome); diff --git a/protocol-tests/simplerestjson/generated/tests/server_request_tests.cc b/protocol-tests/simplerestjson/generated/tests/server_request_tests.cc index 46679bf0..d992b1ab 100644 --- a/protocol-tests/simplerestjson/generated/tests/server_request_tests.cc +++ b/protocol-tests/simplerestjson/generated/tests/server_request_tests.cc @@ -43,6 +43,7 @@ GetEnumOutput MinimalGetEnumOutput() { GetIntEnumOutput MinimalGetIntEnumOutput() { return [] { GetIntEnumOutput v{}; + v.result = static_cast(1); return v; }(); } diff --git a/protocol-tests/simplerestjson/generated/tests/server_response_tests.cc b/protocol-tests/simplerestjson/generated/tests/server_response_tests.cc index cb8db79e..83d9b87b 100644 --- a/protocol-tests/simplerestjson/generated/tests/server_response_tests.cc +++ b/protocol-tests/simplerestjson/generated/tests/server_response_tests.cc @@ -43,6 +43,7 @@ GetEnumOutput MinimalGetEnumOutput() { GetIntEnumOutput MinimalGetIntEnumOutput() { return [] { GetIntEnumOutput v{}; + v.result = static_cast(1); return v; }(); } @@ -241,6 +242,7 @@ smithy::http::HttpRequest MinimalRequestForGetIntEnum() { auto client = *PizzaAdminServiceClient::Create(std::move(config)); GetIntEnumInput input = [] { GetIntEnumInput v{}; + v.aa = static_cast(1); return v; }(); (void)client.GetIntEnum(input); diff --git a/protocol-tests/simplerestjson/generated/tests/smoke_test.cc b/protocol-tests/simplerestjson/generated/tests/smoke_test.cc index 24cca76d..93640b10 100644 --- a/protocol-tests/simplerestjson/generated/tests/smoke_test.cc +++ b/protocol-tests/simplerestjson/generated/tests/smoke_test.cc @@ -45,6 +45,7 @@ GetEnumOutput MinimalGetEnumOutput() { GetIntEnumOutput MinimalGetIntEnumOutput() { return [] { GetIntEnumOutput v{}; + v.result = static_cast(1); return v; }(); } @@ -231,6 +232,7 @@ TEST(PizzaAdminServiceSmokeTest, GetIntEnumRoundTrips) { PizzaAdminServiceClient client = MakeClient(std::make_shared()); const GetIntEnumInput input = [] { GetIntEnumInput v{}; + v.aa = static_cast(1); return v; }(); const auto outcome = client.GetIntEnum(input); diff --git a/protocol-tests/simplerestjson/malformed/server_malformed_test.cc b/protocol-tests/simplerestjson/malformed/server_malformed_test.cc index 34d33435..ffd185de 100644 --- a/protocol-tests/simplerestjson/malformed/server_malformed_test.cc +++ b/protocol-tests/simplerestjson/malformed/server_malformed_test.cc @@ -138,6 +138,42 @@ TEST_F(SimpleRestJsonMalformedTest, WrongContentTypeIs415) { EXPECT_EQ(handler_->calls, 0); } +TEST_F(SimpleRestJsonMalformedTest, IntEnumLabelBeyondInt32IsRejectedBeforeTheHandler) { + // The label parser bounds intEnum at int32 like Integer (issue #109's + // serde-side fix pins the body path; this pins the text path). + smithy::http::HttpRequest request; + request.method = "GET"; + request.target = "/get-int-enum/99999999999"; + const auto response = Send(request); + EXPECT_EQ(response.status, 400) << response.body; + EXPECT_EQ(response.headers.Get("x-error-type").value_or(""), "SerializationException"); + EXPECT_EQ(handler_->calls, 0); +} + +TEST_F(SimpleRestJsonMalformedTest, IntEnumLabelOutsideTheValueSetFailsValidation) { + // In-range but unknown values fail membership validation with the + // string-enum suite message, ints spelled the way smithy-rs emits them + // (issue #109). + smithy::http::HttpRequest request; + request.method = "GET"; + request.target = "/get-int-enum/3"; + const auto response = Send(request); + EXPECT_EQ(response.status, 400) << response.body; + EXPECT_EQ(response.headers.Get("x-error-type").value_or(""), "ValidationException"); + EXPECT_EQ(handler_->calls, 0); + + const auto body = smithy::json::Decode(response.body); + ASSERT_TRUE(body.ok()) << response.body; + const smithy::Document* field_list = body->Find("fieldList"); + ASSERT_NE(field_list, nullptr) << response.body; + ASSERT_EQ(field_list->as_list().size(), 1u) << response.body; + const auto& failure = field_list->as_list()[0]; + EXPECT_EQ(failure.Find("path")->as_string(), "/aa"); + EXPECT_EQ(failure.Find("message")->as_string(), + "Value at '/aa' failed to satisfy constraint: Member must satisfy enum value set: " + "[1, 2]"); +} + TEST_F(SimpleRestJsonMalformedTest, EnumLabelViolationReportsTheSuiteExactMessage) { smithy::http::HttpRequest request; request.method = "GET"; From 50ebf643c07287c985bb1df629d34e4e06a146da Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:43:19 +0000 Subject: [PATCH 4/9] Classify jsonRpc2 error codes on the full int64 (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParseError truncated error.code to int before its 100-599 range test, so a peer sending 2^32+404 classified as HTTP 404 — and 5xx aliases came back retryable. The interop suite now pins a hand-rolled peer sending 21474837103: not a 503, lands in the 400 class, not retryable. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- .../smithycpp/codegen/JsonRpc2Protocol.java | 6 ++-- examples/jsonrpc2/generated/src/client.cc | 4 +-- examples/jsonrpc2/interop_wire_test.cc | 28 +++++++++++++++++++ .../roundtrip/jsonrpc/generated/src/client.cc | 4 +-- .../jsonrpc2/generated/src/client.cc | 4 +-- 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java index d22d8b22..4ea46dcd 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java @@ -110,9 +110,11 @@ public void writeClientHelpers(CppWriter w, CppContext context) { w.openBlock( "if (const smithy::Document* code = error->Find(\"code\"); " + "code != nullptr && code->is_int()) {"); - w.write("const auto rpc_code = static_cast(code->as_int());"); + // Classified on the full int64: narrowing first would alias 2^32+404 + // onto 404 and misroute status/retryability (issue #109). + w.write("const std::int64_t rpc_code = code->as_int();"); w.write( - "parsed.status = rpc_code >= 100 && rpc_code < 600 ? rpc_code " + "parsed.status = rpc_code >= 100 && rpc_code < 600 ? static_cast(rpc_code) " + ": (rpc_code == -32603 ? 500 : 400);"); w.closeBlock("}"); w.write( diff --git a/examples/jsonrpc2/generated/src/client.cc b/examples/jsonrpc2/generated/src/client.cc index 7f5fb7e1..52b18786 100644 --- a/examples/jsonrpc2/generated/src/client.cc +++ b/examples/jsonrpc2/generated/src/client.cc @@ -52,8 +52,8 @@ struct ParsedError { const smithy::Document* error = doc->Find("error"); if (error == nullptr || !error->is_map()) return parsed; if (const smithy::Document* code = error->Find("code"); code != nullptr && code->is_int()) { - const auto rpc_code = static_cast(code->as_int()); - parsed.status = rpc_code >= 100 && rpc_code < 600 ? rpc_code : (rpc_code == -32603 ? 500 : 400); + const std::int64_t rpc_code = code->as_int(); + parsed.status = rpc_code >= 100 && rpc_code < 600 ? static_cast(rpc_code) : (rpc_code == -32603 ? 500 : 400); } if (const smithy::Document* message = error->Find("message"); message != nullptr && message->is_string()) parsed.message = message->as_string(); if (const smithy::Document* data = error->Find("data"); data != nullptr) parsed.doc = *data; diff --git a/examples/jsonrpc2/interop_wire_test.cc b/examples/jsonrpc2/interop_wire_test.cc index 938b4df5..3a0f7a94 100644 --- a/examples/jsonrpc2/interop_wire_test.cc +++ b/examples/jsonrpc2/interop_wire_test.cc @@ -139,6 +139,34 @@ TEST(JsonRpc2InteropTest, GeneratedClientTalksToAHandRolledPeer) { EXPECT_EQ(divided.error().detail()->message, "nope"); } +// error.code is classified on the full int64 (issue #109): a peer sending +// 5*2^32+503 must not be read as HTTP 503 — the old static_cast +// truncation did exactly that, marking the error retryable and misrouting +// status-based handling. Out-of-band codes collapse to the 400 class. +class HugeErrorCodePeer final : public smithy::http::HttpClient { + public: + smithy::Outcome Send(const smithy::http::HttpRequest&) override { + smithy::http::HttpResponse response{ + 200, {}, R"({"jsonrpc":"2.0","error":{"code":21474837103,"message":"kaboom"},"id":1})"}; + response.headers.Set("content-type", "application/json"); + return response; + } +}; + +TEST(JsonRpc2InteropTest, ErrorCodesBeyondInt32AreNotTruncatedIntoTheHttpRange) { + smithy::ClientConfig config; + config.http_client = std::make_shared(); + auto client = CalculatorClient::Create(std::move(config)); + ASSERT_TRUE(client.ok()) << client.error().message(); + + const auto divided = client->Divide(DivideInput{.dividend = 1, .divisor = 1}); + ASSERT_FALSE(divided.ok()); + EXPECT_EQ(divided.error().message(), "kaboom"); + // 21474837103 truncated to int is 503 — which would classify as a + // retryable server error. The full-width read lands in the 400 class. + EXPECT_FALSE(divided.error().retryable()); +} + // @idempotencyToken members auto-fill over jsonRpc2 like any other protocol: // an unset token reaches the peer as a UUID inside params, and an explicit // one passes through untouched. diff --git a/examples/roundtrip/jsonrpc/generated/src/client.cc b/examples/roundtrip/jsonrpc/generated/src/client.cc index 16d87ed0..0df3aff7 100644 --- a/examples/roundtrip/jsonrpc/generated/src/client.cc +++ b/examples/roundtrip/jsonrpc/generated/src/client.cc @@ -47,8 +47,8 @@ struct ParsedError { const smithy::Document* error = doc->Find("error"); if (error == nullptr || !error->is_map()) return parsed; if (const smithy::Document* code = error->Find("code"); code != nullptr && code->is_int()) { - const auto rpc_code = static_cast(code->as_int()); - parsed.status = rpc_code >= 100 && rpc_code < 600 ? rpc_code : (rpc_code == -32603 ? 500 : 400); + const std::int64_t rpc_code = code->as_int(); + parsed.status = rpc_code >= 100 && rpc_code < 600 ? static_cast(rpc_code) : (rpc_code == -32603 ? 500 : 400); } if (const smithy::Document* message = error->Find("message"); message != nullptr && message->is_string()) parsed.message = message->as_string(); if (const smithy::Document* data = error->Find("data"); data != nullptr) parsed.doc = *data; diff --git a/protocol-tests/jsonrpc2/generated/src/client.cc b/protocol-tests/jsonrpc2/generated/src/client.cc index 5700439c..4ba03312 100644 --- a/protocol-tests/jsonrpc2/generated/src/client.cc +++ b/protocol-tests/jsonrpc2/generated/src/client.cc @@ -52,8 +52,8 @@ struct ParsedError { const smithy::Document* error = doc->Find("error"); if (error == nullptr || !error->is_map()) return parsed; if (const smithy::Document* code = error->Find("code"); code != nullptr && code->is_int()) { - const auto rpc_code = static_cast(code->as_int()); - parsed.status = rpc_code >= 100 && rpc_code < 600 ? rpc_code : (rpc_code == -32603 ? 500 : 400); + const std::int64_t rpc_code = code->as_int(); + parsed.status = rpc_code >= 100 && rpc_code < 600 ? static_cast(rpc_code) : (rpc_code == -32603 ? 500 : 400); } if (const smithy::Document* message = error->Find("message"); message != nullptr && message->is_string()) parsed.message = message->as_string(); if (const smithy::Document* data = error->Find("data"); data != nullptr) parsed.doc = *data; From 0c47996a336fc98c9d3461ffd87e7d0fb2a2c76e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:43:19 +0000 Subject: [PATCH 5/9] Pin the numeric bounds across the module boundary, both directions (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One suite driving the roundtrip REST fixture at three depths: the generated serde directly (hostile Documents), the generated server over the wire (SerializationException before the handler, ValidationException with the suite-exact message), and the generated client parsing hostile responses — plus the deliberate asymmetry: clients keep unknown-but-in-range intEnum values for forward compatibility, servers reject them. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- examples/roundtrip/rest/BUILD.bazel | 20 ++ .../rest/numeric_bounds_wire_test.cc | 204 ++++++++++++++++++ 2 files changed, 224 insertions(+) create mode 100644 examples/roundtrip/rest/numeric_bounds_wire_test.cc diff --git a/examples/roundtrip/rest/BUILD.bazel b/examples/roundtrip/rest/BUILD.bazel index b1c89cc5..93f1c816 100644 --- a/examples/roundtrip/rest/BUILD.bazel +++ b/examples/roundtrip/rest/BUILD.bazel @@ -15,6 +15,26 @@ cc_test( ], ) +# Issue #109 narrowing fixes, pinned across the module boundary in both +# directions: intEnum int32 bounds + membership validation, float overflow +# rejection, and the deliberate client/server asymmetry for unknown values. +cc_test( + name = "numeric_bounds_wire_test", + size = "small", + srcs = ["numeric_bounds_wire_test.cc"], + copts = SMITHY_COPTS, + deps = [ + "//examples/roundtrip/rest/generated:client", + "//examples/roundtrip/rest/generated:serde", + "//examples/roundtrip/rest/generated:server", + "//runtime:client", + "//runtime:core", + "//runtime:http", + "//runtime:json", + "@googletest//:gtest_main", + ], +) + cc_test( name = "api_key_wire_test", size = "small", diff --git a/examples/roundtrip/rest/numeric_bounds_wire_test.cc b/examples/roundtrip/rest/numeric_bounds_wire_test.cc new file mode 100644 index 00000000..5477307e --- /dev/null +++ b/examples/roundtrip/rest/numeric_bounds_wire_test.cc @@ -0,0 +1,204 @@ +// Wire-level pins for the issue-#109 narrowing fixes, across the module +// boundary (generated code ↔ runtime) in both directions: +// +// - intEnum members share the int32 bounds check with Integer members — a +// wire value of 2^32+2 must fail the parse, never alias onto a valid +// enumerator via the truncating cast. +// - float members reject finite doubles beyond float range — the raw +// static_cast would be UB ([conv.double], UBSan float-cast-overflow). +// - Servers additionally validate intEnum membership (ValidationException, +// string-enum policy) while clients keep unknown-but-in-range values for +// forward compatibility. That asymmetry is deliberate and pinned here. + +#include + +#include +#include +#include +#include +#include +#include + +#include "example/roundtrip/rest/client.h" +#include "example/roundtrip/rest/serde.h" +#include "example/roundtrip/rest/server.h" +#include "smithy/client/config.h" +#include "smithy/core/document.h" +#include "smithy/http/transport.h" +#include "smithy/json/json.h" + +namespace example::roundtrip::rest { +namespace { + +// --- Direct serde boundary: hostile Documents into the generated parser. --- + +smithy::Document SinkDoc(const char* member, smithy::Document value) { + smithy::DocumentMap map; + map.emplace("name", smithy::Document(std::string("n"))); + map.emplace(member, std::move(value)); + return smithy::Document(std::move(map)); +} + +TEST(NumericBoundsSerdeTest, IntEnumBeyondInt32FailsInsteadOfAliasing) { + // 2^32+2 truncates to 2 (a valid Weight) under the old cast. + auto sink = DeserializeKitchenSink(SinkDoc("weight", smithy::Document(std::int64_t{4294967298}))); + ASSERT_FALSE(sink.ok()); + EXPECT_EQ(sink.error().message(), "KitchenSink.weight: value out of range"); +} + +TEST(NumericBoundsSerdeTest, IntEnumKeepsUnknownInRangeValues) { + // Model evolution: a value the client's model doesn't know yet still + // parses (matching string enums' unknown handling); only servers reject + // it, via validation. + auto sink = DeserializeKitchenSink(SinkDoc("weight", smithy::Document(std::int64_t{7}))); + ASSERT_TRUE(sink.ok()) << sink.error().message(); + ASSERT_TRUE(sink->weight.has_value()); + EXPECT_EQ(*sink->weight, static_cast(7)); +} + +TEST(NumericBoundsSerdeTest, FloatBeyondRangeFailsInsteadOfUb) { + auto sink = DeserializeKitchenSink(SinkDoc("ratio", smithy::Document(1e300))); + ASSERT_FALSE(sink.ok()); + EXPECT_EQ(sink.error().message(), "KitchenSink.ratio: value out of range"); +} + +TEST(NumericBoundsSerdeTest, FloatEdgeAndNonFiniteValuesStillParse) { + // The exact float maximum is in range... + const double float_max = static_cast(std::numeric_limits::max()); + auto edge = DeserializeKitchenSink(SinkDoc("ratio", smithy::Document(float_max))); + ASSERT_TRUE(edge.ok()) << edge.error().message(); + EXPECT_EQ(*edge->ratio, std::numeric_limits::max()); + // ...and the Smithy non-finite spellings narrow losslessly, never caught + // in the overflow net. + auto inf = DeserializeKitchenSink(SinkDoc("ratio", smithy::Document(std::string("-Infinity")))); + ASSERT_TRUE(inf.ok()) << inf.error().message(); + EXPECT_TRUE(std::isinf(*inf->ratio)); + EXPECT_LT(*inf->ratio, 0.0F); +} + +// --- Server over the wire: hostile JSON bodies into the generated router. --- + +class RecordingHandler : public RoundTripRestHandler { + public: + smithy::Outcome DescribeSink(const DescribeSinkInput&, + const smithy::server::RequestContext&) override { + ++calls; + return DescribeSinkOutput{}; + } + smithy::Outcome PutSink(const PutSinkInput& input, + const smithy::server::RequestContext&) override { + ++calls; + last_sink = input.sink; + return PutSinkOutput{.sinkId = "s1"}; + } + smithy::Outcome UploadAttachment( + const UploadAttachmentInput&, const smithy::server::RequestContext&) override { + ++calls; + return UploadAttachmentOutput{}; + } + int calls = 0; + std::optional last_sink; +}; + +class NumericBoundsServerTest : public testing::Test { + protected: + smithy::http::HttpResponse PutSinkBody(const std::string& body) { + smithy::http::HttpRequest request; + request.method = "PUT"; + request.target = "/sinks/s1?limit=5"; + request.headers.Set("content-type", "application/json"); + request.headers.Set("x-sink-created", "Sun, 06 Nov 1994 08:49:37 GMT"); + request.body = body; + return server_.Handler()(request); + } + + std::shared_ptr handler_ = std::make_shared(); + RoundTripRestServer server_{handler_}; +}; + +TEST_F(NumericBoundsServerTest, IntEnumBeyondInt32IsRejectedBeforeTheHandler) { + const auto response = PutSinkBody(R"({"sink":{"name":"n","weight":4294967298}})"); + EXPECT_EQ(response.status, 400) << response.body; + EXPECT_EQ(response.headers.Get("x-error-type").value_or(""), "SerializationException"); + EXPECT_EQ(handler_->calls, 0); +} + +TEST_F(NumericBoundsServerTest, FloatBeyondRangeIsRejectedBeforeTheHandler) { + const auto response = PutSinkBody(R"({"sink":{"name":"n","ratio":1e300}})"); + EXPECT_EQ(response.status, 400) << response.body; + EXPECT_EQ(response.headers.Get("x-error-type").value_or(""), "SerializationException"); + EXPECT_EQ(handler_->calls, 0); +} + +TEST_F(NumericBoundsServerTest, UnknownIntEnumValueFailsValidationWithTheSuiteMessage) { + const auto response = PutSinkBody(R"({"sink":{"name":"n","weight":3}})"); + EXPECT_EQ(response.status, 400) << response.body; + EXPECT_EQ(response.headers.Get("x-error-type").value_or(""), "ValidationException"); + EXPECT_EQ(handler_->calls, 0); + auto body = smithy::json::Decode(response.body); + ASSERT_TRUE(body.ok()) << response.body; + const smithy::Document* field_list = body->Find("fieldList"); + ASSERT_NE(field_list, nullptr) << response.body; + ASSERT_EQ(field_list->as_list().size(), 1u) << response.body; + const auto& failure = field_list->as_list()[0]; + EXPECT_EQ(failure.Find("path")->as_string(), "/sink/weight"); + EXPECT_EQ(failure.Find("message")->as_string(), + "Value at '/sink/weight' failed to satisfy constraint: Member must satisfy enum value " + "set: [1, 2]"); +} + +TEST_F(NumericBoundsServerTest, ValidValuesReachTheHandlerIntact) { + const auto response = PutSinkBody(R"({"sink":{"name":"n","weight":2,"ratio":2.5}})"); + EXPECT_EQ(response.status, 200) << response.body; + EXPECT_EQ(handler_->calls, 1); + ASSERT_TRUE(handler_->last_sink.has_value()); + EXPECT_EQ(handler_->last_sink->weight, Weight::kHeavy); + EXPECT_EQ(handler_->last_sink->ratio, 2.5F); +} + +// --- Client over the wire: hostile server responses into the generated +// client. --- + +class CannedTransport final : public smithy::http::HttpClient { + public: + explicit CannedTransport(std::string body) : body_(std::move(body)) {} + + smithy::Outcome Send(const smithy::http::HttpRequest&) override { + smithy::http::HttpResponse response{200, {}, body_}; + response.headers.Set("content-type", "application/json"); + return response; + } + + private: + std::string body_; +}; + +smithy::Outcome Describe(const std::string& body) { + smithy::ClientConfig config; + config.http_client = std::make_shared(body); + auto client = RoundTripRestClient::Create(std::move(config)); + EXPECT_TRUE(client.ok()) << client.error().message(); + return client->DescribeSink(DescribeSinkInput{.sinkId = "s1"}); +} + +TEST(NumericBoundsClientTest, IntEnumBeyondInt32FailsTheResponseParse) { + const auto outcome = Describe(R"({"sink":{"name":"n","weight":4294967298}})"); + ASSERT_FALSE(outcome.ok()); + EXPECT_EQ(outcome.error().message(), "KitchenSink.weight: value out of range"); +} + +TEST(NumericBoundsClientTest, FloatBeyondRangeFailsTheResponseParse) { + const auto outcome = Describe(R"({"sink":{"name":"n","ratio":1e300}})"); + ASSERT_FALSE(outcome.ok()); + EXPECT_EQ(outcome.error().message(), "KitchenSink.ratio: value out of range"); +} + +TEST(NumericBoundsClientTest, UnknownInRangeIntEnumValueSurvivesForForwardCompat) { + const auto outcome = Describe(R"({"sink":{"name":"n","weight":7}})"); + ASSERT_TRUE(outcome.ok()) << outcome.error().message(); + ASSERT_TRUE(outcome->sink.has_value()); + EXPECT_EQ(outcome->sink->weight, static_cast(7)); +} + +} // namespace +} // namespace example::roundtrip::rest From 42a3a146857866e184c6693f635a3f1abe7564a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:43:20 +0000 Subject: [PATCH 6/9] Add the includes random_document.h was borrowing transitively (#109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit std::numeric_limits and std::vector without / — breaks on the libc++ matrix cell (SF.10). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- runtime/tests/testing/random_document.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/runtime/tests/testing/random_document.h b/runtime/tests/testing/random_document.h index 2be648ef..2a184c47 100644 --- a/runtime/tests/testing/random_document.h +++ b/runtime/tests/testing/random_document.h @@ -5,8 +5,10 @@ #define SMITHY_TESTS_TESTING_RANDOM_DOCUMENT_H_ #include +#include #include #include +#include #include "smithy/core/document.h" From 375abd5b4e9bb5eaebf0e319d33ad3d6f9b56a86 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 00:43:20 +0000 Subject: [PATCH 7/9] Document the narrowing fixes and the new intEnum posture (#109) Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- CHANGELOG.md | 21 +++++++++++++++++++++ docs/generated-types.md | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a28a4e0..eb0d1df9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,18 @@ policy in [docs/versioning.md](docs/versioning.md). ### Fixed +- **Numeric wire values no longer truncate into generated narrow types** + (#109). Three holes in the otherwise-uniform range-check posture: an + `intEnum` member cast the raw wire int64 straight into its `int32`-backed + `enum class`, so 2^32+2 silently aliased onto a valid enumerator (byte / + short / integer members already rejected out-of-range values — intEnum now + shares their check, on document bodies and text bindings alike); a `float` + member cast the parsed double unchecked, so a finite wire value beyond + float range (`1e300`) was undefined behavior per [conv.double] — the new + `smithy::FloatFromDouble` rejects it while NaN/±Infinity still pass; and + the jsonRpc2 client truncated `error.code` to `int` *before* its 100–599 + range test, classifying 2^32+404 as HTTP 404 (with 5xx codes wrongly + marked retryable) — codes are now classified on the full int64. - **Modeled redirects: a 3xx `@httpResponseCode` is a success, not an error** (#184). Generated clients tested `status < 200 || status > 299` when the output bound `@httpResponseCode`, so every modeled redirect came back as a @@ -31,6 +43,15 @@ policy in [docs/versioning.md](docs/versioning.md). ### Added +- **Servers validate intEnum membership** (#109). String enums already + failed request validation outside the modeled value set; intEnum members + were accepted silently. They now produce the same suite-exact + `ValidationException` message + (`Member must satisfy enum value set: [1, 2]`), with `@internal` members + staying wire-valid but unadvertised — string-enum policy throughout. + Clients deliberately keep + unknown-but-in-range values for forward compatibility, and the asymmetry is + pinned by `examples/roundtrip/rest/numeric_bounds_wire_test.cc`. - **Redirects are documented and covered** — a "Redirects (3xx)" section in [docs/server-guide.md](docs/server-guide.md) covering both spellings and the `@suppress(["HttpResponseCodeSemantics"])` a static non-2xx `@http` code diff --git a/docs/generated-types.md b/docs/generated-types.md index ebbc55e7..2c95770e 100644 --- a/docs/generated-types.md +++ b/docs/generated-types.md @@ -19,7 +19,7 @@ compatibility contract: changes to it are breaking for consumers of generated co | `structure` | `struct` with public members | Aggregate; `operator==` and `operator<=>` defaulted; every member value-initialized with `{}` | | `union` | class over `std::variant` | See below | | `enum` | class with nested `enum class Value` | See below; unknown wire values preserved | -| `intEnum` | `enum class X : std::int32_t` | | +| `intEnum` | `enum class X : std::int32_t` | Wire values outside int32 fail the parse; unknown in-range values are preserved (servers additionally validate membership) | | `smithy.api#Unit` | `smithy::Unit` | Never declared; maps to the runtime type | | `bigInteger` / `bigDecimal` | — | Rejected with a clear error (planned) | | `@streaming` blob member | trait ignored | Generates as a fully buffered `smithy::Blob`; see the README's [Current limitations](../README.md#current-limitations) | From 832296cd67dbca534a46186b8174cd5b2062b82c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 02:31:04 +0000 Subject: [PATCH 8/9] Fix the float overflow boundary and harden the new pins (review panel) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FloatFromDouble rejected at FLT_MAX, but [conv.double] only goes undefined once round-to-nearest overflows — at 2^128*(1-2^-25). The gap is not theoretical: shortest-round-trip float text, including FormatFloat's own output for FLT_MAX, parses to a double slightly above FLT_MAX that must keep narrowing to it. The boundary is now the true overflow bound, pinned by a FormatFloat->strtod->FloatFromDouble round-trip test and a wire-text serde case. Also from the panel: the jsonRpc2 truncation test used 21474837103, which truncates to 623 and classified as 400 under old and new code alike — a vacuous pin; 21474836983 truncates to 503 and discriminates. Generated jsonRpc2 clients now include for their std::int64_t local instead of borrowing it transitively (SF.10). The Describe test helper fails cleanly instead of dereferencing a failed Outcome. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- .../smithycpp/codegen/JsonRpc2Protocol.java | 3 ++ examples/jsonrpc2/generated/src/client.cc | 1 + examples/jsonrpc2/interop_wire_test.cc | 13 +++++---- .../roundtrip/jsonrpc/generated/src/client.cc | 1 + .../rest/numeric_bounds_wire_test.cc | 13 ++++++++- .../jsonrpc2/generated/src/client.cc | 1 + runtime/src/core/document_serde.cc | 14 ++++++---- runtime/tests/core/document_serde_test.cc | 28 +++++++++++++++---- 8 files changed, 58 insertions(+), 16 deletions(-) diff --git a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java index 4ea46dcd..a109c52b 100644 --- a/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java +++ b/codegen/smithy-cpp-codegen/src/main/java/io/smithycpp/codegen/JsonRpc2Protocol.java @@ -88,6 +88,9 @@ public List clientIncludes() { @Override public void writeClientHelpers(CppWriter w, CppContext context) { + // ParseError declares a std::int64_t local; don't borrow + // transitively (SF.10). + w.addInclude(""); ProtocolSupport.writeSanitizeErrorCode(w); ProtocolSupport.writeParsedErrorStruct(w); // jsonRpc2 error identity lives in the envelope's error object rather than diff --git a/examples/jsonrpc2/generated/src/client.cc b/examples/jsonrpc2/generated/src/client.cc index 52b18786..1e9e5914 100644 --- a/examples/jsonrpc2/generated/src/client.cc +++ b/examples/jsonrpc2/generated/src/client.cc @@ -1,5 +1,6 @@ // Code generated by smithy-cpp (cpp-codegen). DO NOT EDIT. +#include #include #include #include diff --git a/examples/jsonrpc2/interop_wire_test.cc b/examples/jsonrpc2/interop_wire_test.cc index 3a0f7a94..a61b9d1b 100644 --- a/examples/jsonrpc2/interop_wire_test.cc +++ b/examples/jsonrpc2/interop_wire_test.cc @@ -140,14 +140,17 @@ TEST(JsonRpc2InteropTest, GeneratedClientTalksToAHandRolledPeer) { } // error.code is classified on the full int64 (issue #109): a peer sending -// 5*2^32+503 must not be read as HTTP 503 — the old static_cast -// truncation did exactly that, marking the error retryable and misrouting -// status-based handling. Out-of-band codes collapse to the 400 class. +// 21474836983 = 5*2^32+503 must not be read as HTTP 503 — the old +// static_cast truncation did exactly that, marking the error retryable +// and misrouting status-based handling. Out-of-band codes collapse to the +// 400 class. The constant is chosen so the pre-fix code lands *inside* the +// 100-599 window (as a retryable 503): a value that truncates outside it +// would classify as 400 under old and new code alike and pin nothing. class HugeErrorCodePeer final : public smithy::http::HttpClient { public: smithy::Outcome Send(const smithy::http::HttpRequest&) override { smithy::http::HttpResponse response{ - 200, {}, R"({"jsonrpc":"2.0","error":{"code":21474837103,"message":"kaboom"},"id":1})"}; + 200, {}, R"({"jsonrpc":"2.0","error":{"code":21474836983,"message":"kaboom"},"id":1})"}; response.headers.Set("content-type", "application/json"); return response; } @@ -162,7 +165,7 @@ TEST(JsonRpc2InteropTest, ErrorCodesBeyondInt32AreNotTruncatedIntoTheHttpRange) const auto divided = client->Divide(DivideInput{.dividend = 1, .divisor = 1}); ASSERT_FALSE(divided.ok()); EXPECT_EQ(divided.error().message(), "kaboom"); - // 21474837103 truncated to int is 503 — which would classify as a + // 21474836983 truncated to int is 503 — which would classify as a // retryable server error. The full-width read lands in the 400 class. EXPECT_FALSE(divided.error().retryable()); } diff --git a/examples/roundtrip/jsonrpc/generated/src/client.cc b/examples/roundtrip/jsonrpc/generated/src/client.cc index 0df3aff7..94794e20 100644 --- a/examples/roundtrip/jsonrpc/generated/src/client.cc +++ b/examples/roundtrip/jsonrpc/generated/src/client.cc @@ -1,6 +1,7 @@ // Code generated by smithy-cpp (cpp-codegen). DO NOT EDIT. #include +#include #include #include #include diff --git a/examples/roundtrip/rest/numeric_bounds_wire_test.cc b/examples/roundtrip/rest/numeric_bounds_wire_test.cc index 5477307e..5ac212c4 100644 --- a/examples/roundtrip/rest/numeric_bounds_wire_test.cc +++ b/examples/roundtrip/rest/numeric_bounds_wire_test.cc @@ -68,6 +68,12 @@ TEST(NumericBoundsSerdeTest, FloatEdgeAndNonFiniteValuesStillParse) { auto edge = DeserializeKitchenSink(SinkDoc("ratio", smithy::Document(float_max))); ASSERT_TRUE(edge.ok()) << edge.error().message(); EXPECT_EQ(*edge->ratio, std::numeric_limits::max()); + // ...as is what a peer's shortest-round-trip printer (or our own + // FormatFloat) puts on the wire for float max — a double slightly above + // FLT_MAX that still rounds back to it. + auto shortest = DeserializeKitchenSink(SinkDoc("ratio", smithy::Document(3.4028235e38))); + ASSERT_TRUE(shortest.ok()) << shortest.error().message(); + EXPECT_EQ(*shortest->ratio, std::numeric_limits::max()); // ...and the Smithy non-finite spellings narrow losslessly, never caught // in the overflow net. auto inf = DeserializeKitchenSink(SinkDoc("ratio", smithy::Document(std::string("-Infinity")))); @@ -177,7 +183,12 @@ smithy::Outcome Describe(const std::string& body) { smithy::ClientConfig config; config.http_client = std::make_shared(body); auto client = RoundTripRestClient::Create(std::move(config)); - EXPECT_TRUE(client.ok()) << client.error().message(); + if (!client.ok()) { + // Not ASSERT (void-only): fail the test and hand back the creation + // error rather than dereferencing a failed Outcome. + ADD_FAILURE() << client.error().message(); + return client.error(); + } return client->DescribeSink(DescribeSinkInput{.sinkId = "s1"}); } diff --git a/protocol-tests/jsonrpc2/generated/src/client.cc b/protocol-tests/jsonrpc2/generated/src/client.cc index 4ba03312..ae77bcc2 100644 --- a/protocol-tests/jsonrpc2/generated/src/client.cc +++ b/protocol-tests/jsonrpc2/generated/src/client.cc @@ -1,5 +1,6 @@ // Code generated by smithy-cpp (cpp-codegen). DO NOT EDIT. +#include #include #include #include diff --git a/runtime/src/core/document_serde.cc b/runtime/src/core/document_serde.cc index a535c5a5..ab29539f 100644 --- a/runtime/src/core/document_serde.cc +++ b/runtime/src/core/document_serde.cc @@ -54,11 +54,15 @@ Outcome DoubleFromDocument(const Document& doc) { } Outcome FloatFromDouble(double value) { - // Only finite out-of-range values are rejected: casting those is undefined - // behavior ([conv.double]), while NaN and ±Infinity narrow losslessly and - // are legal Smithy float values on every wire. - if (std::isfinite(value) && - (value < -std::numeric_limits::max() || value > std::numeric_limits::max())) { + // Only finite values that overflow the cast are rejected: [conv.double] + // goes undefined once the round-to-nearest result falls outside float's + // range, which happens at 2^128·(1−2^−25) — NOT at FLT_MAX. The gap + // matters: shortest-round-trip float text (FormatFloat's own output for + // FLT_MAX, "3.4028235e+38") parses to a double slightly above FLT_MAX + // that still rounds back to it and must keep parsing. NaN and ±Infinity + // narrow losslessly and are legal Smithy float values on every wire. + constexpr double kFloatOverflowBound = 0x1.ffffffp+127; // 2^128 - 2^103 + if (std::isfinite(value) && (value <= -kFloatOverflowBound || value >= kFloatOverflowBound)) { return Error::Serialization("number: value out of float range"); } return static_cast(value); diff --git a/runtime/tests/core/document_serde_test.cc b/runtime/tests/core/document_serde_test.cc index aa23bc94..52915e58 100644 --- a/runtime/tests/core/document_serde_test.cc +++ b/runtime/tests/core/document_serde_test.cc @@ -3,7 +3,9 @@ #include #include +#include #include +#include #include "smithy/core/uuid.h" @@ -96,17 +98,33 @@ TEST(DocumentSerdeTest, FloatFromDoubleNarrowsInRangeValues) { EXPECT_EQ(*FloatFromDouble(1e-300), 0.0F); } +TEST(DocumentSerdeTest, FloatFromDoubleAcceptsDoublesThatRoundToFloatMax) { + // Shortest-round-trip float text — what FormatFloat itself emits for + // FLT_MAX — parses to a double slightly above FLT_MAX that still rounds + // back to it. Rejecting at FLT_MAX instead of the true overflow boundary + // would 400 this library's own wire output. + const std::string text = FormatFloat(std::numeric_limits::max()); + const double reparsed = std::strtod(text.c_str(), nullptr); + EXPECT_GT(reparsed, static_cast(std::numeric_limits::max())); + ASSERT_TRUE(FloatFromDouble(reparsed).ok()) << text; + EXPECT_EQ(*FloatFromDouble(reparsed), std::numeric_limits::max()); + // Anything strictly below the round-to-nearest overflow boundary + // (2^128 - 2^103) rounds to FLT_MAX and stays accepted. + const double just_below_bound = std::nextafter(0x1.ffffffp+127, 0.0); + EXPECT_EQ(*FloatFromDouble(just_below_bound), std::numeric_limits::max()); + EXPECT_EQ(*FloatFromDouble(-just_below_bound), -std::numeric_limits::max()); +} + TEST(DocumentSerdeTest, FloatFromDoubleRejectsFiniteOverflow) { // The raw static_cast would be UB for these ([conv.double]); a hostile // request body carrying 1e300 for a float member must fail the parse. EXPECT_FALSE(FloatFromDouble(1e300).ok()); EXPECT_FALSE(FloatFromDouble(-1e300).ok()); EXPECT_FALSE(FloatFromDouble(std::numeric_limits::max()).ok()); - // Just past the float edge in double precision is already overflow. - EXPECT_FALSE( - FloatFromDouble(std::nextafter(static_cast(std::numeric_limits::max()), - std::numeric_limits::infinity())) - .ok()); + // The exact round-to-nearest overflow boundary (ties-to-even lands on + // 2^128) is the first rejected value, in both directions. + EXPECT_FALSE(FloatFromDouble(0x1.ffffffp+127).ok()); + EXPECT_FALSE(FloatFromDouble(-0x1.ffffffp+127).ok()); } TEST(DocumentSerdeTest, FloatFromDoublePassesNonFiniteThrough) { From 6d7397573ba754ff66c607d9779c14aff5edbaa2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 20:58:12 +0000 Subject: [PATCH 9/9] Sharpen the changelog's path claims; note intEnum in model evolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-bot nits on #197, both verified: the intEnum text-binding path was already bounded on main (ParseInt64Text + int64Bounds) — the hole this PR closes is the document-body cast, so the changelog now says which path changed; and model-evolution.md's add-enum-value row now covers intEnums (clients keep unknown in-range values, servers reject outside the modeled set) alongside string enums. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014C7WdBD99mUFWGxMvGSjSU --- CHANGELOG.md | 16 +++++++++------- docs/model-evolution.md | 2 +- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb0d1df9..655d1708 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,13 +10,15 @@ policy in [docs/versioning.md](docs/versioning.md). - **Numeric wire values no longer truncate into generated narrow types** (#109). Three holes in the otherwise-uniform range-check posture: an - `intEnum` member cast the raw wire int64 straight into its `int32`-backed - `enum class`, so 2^32+2 silently aliased onto a valid enumerator (byte / - short / integer members already rejected out-of-range values — intEnum now - shares their check, on document bodies and text bindings alike); a `float` - member cast the parsed double unchecked, so a finite wire value beyond - float range (`1e300`) was undefined behavior per [conv.double] — the new - `smithy::FloatFromDouble` rejects it while NaN/±Infinity still pass; and + `intEnum` member in a document body cast the raw wire int64 straight into + its `int32`-backed `enum class`, so 2^32+2 silently aliased onto a valid + enumerator (byte / short / integer members already rejected out-of-range + values, and intEnum *text* bindings were already bounded — the body path + now shares the same check); a `float` member cast the parsed double + unchecked on both the body and text-binding paths, so a finite wire value + beyond float range (`1e300`) was undefined behavior per [conv.double] — + the new `smithy::FloatFromDouble` rejects it while NaN/±Infinity still + pass; and the jsonRpc2 client truncated `error.code` to `int` *before* its 100–599 range test, classifying 2^32+404 as HTTP 404 (with 5xx codes wrongly marked retryable) — codes are now classified on the full int64. diff --git a/docs/model-evolution.md b/docs/model-evolution.md index 786394d1..aab60c89 100644 --- a/docs/model-evolution.md +++ b/docs/model-evolution.md @@ -161,7 +161,7 @@ compiling) are separate questions: | Add optional member | ✅ old readers ignore it; absent → unset `std::optional` | ✅ additive | | Add member with `@default` | ✅ absent → the default | ✅ additive (plain member, default-initialized) | | Add operation | ✅ old clients never call it | ⚠️ handlers must implement the new pure-virtual method (compile error guides) | -| Add enum value | ✅ old readers preserve unknown values (`Value::kUnknown` + original text) | ✅ additive | +| Add enum value | ✅ old *clients* preserve unknown values (string enums: `Value::kUnknown` + original text; intEnums keep the in-range value) — ⚠️ old *servers* reject values outside their modeled set with 400 `ValidationException` | ✅ additive | | Promote optional → `@required` + `@default` | ✅ absence on the wire keeps the default (the generator's evolution leniency) | ✅ member becomes plain (non-optional) — call sites reading `.has_value()` need updating | | Promote optional → `@required` (no default) | ❌ old writers that omit it now fail deserialization | ❌ member type changes | | Rename member | ❌ wire key changes — unless the old wire name is kept via `@jsonName` | ❌ compile errors at every use |