From 5c78741259facd4af8737b992fbdebe76d21403a Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sat, 26 Nov 2022 21:10:23 -0600 Subject: [PATCH 01/12] Removes Debug for shape with sensitive trait This commit centralizes a place where the codegen excludes the Debug trait if a shape has the sensitive trait. Previously the exclusion was handled locally in each shape, e.g. StructureGenerator and EnumGenerator. However, that approach may overlook a certain shape we also need to treat as such. We now handle the exclusion of the Debug trait in one place, BaseSymbolMetadataProvider. --- .../core/smithy/SymbolMetadataProvider.kt | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index e8fa8b06e0..734f09dc9c 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -15,6 +15,7 @@ import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape import software.amazon.smithy.model.traits.EnumDefinition import software.amazon.smithy.model.traits.EnumTrait +import software.amazon.smithy.model.traits.SensitiveTrait import software.amazon.smithy.model.traits.StreamingTrait import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.RustMetadata @@ -77,13 +78,20 @@ abstract class SymbolMetadataProvider(private val base: RustSymbolProvider) : Wr class BaseSymbolMetadataProvider( base: RustSymbolProvider, private val model: Model, - additionalAttributes: List, + private val additionalAttributes: List, ) : SymbolMetadataProvider(base) { - private val containerDefault = RustMetadata( - Attribute.Derives(defaultDerives.toSet()), - additionalAttributes = additionalAttributes, - visibility = Visibility.PUBLIC, - ) + private fun containerDefault(shape: Shape): RustMetadata { + val setOfDerives = if (shape.hasTrait()) { + defaultDerives.toSet() - RuntimeType.Debug + } else { + defaultDerives.toSet() + } + return RustMetadata( + Attribute.Derives(setOfDerives), + additionalAttributes = additionalAttributes, + visibility = Visibility.PUBLIC, + ) + } override fun memberMeta(memberShape: MemberShape): RustMetadata { val container = model.expectShape(memberShape.container) @@ -113,15 +121,20 @@ class BaseSymbolMetadataProvider( } override fun structureMeta(structureShape: StructureShape): RustMetadata { - return containerDefault + // In the case of fields being sensitive, we cannot impl the derived Debug + // trait because they reveal the sensitive data. At a container level, + // however, we don't know if a structure has sensitive fields without + // further looking into them. We err on the side of caution and require + // that a structure implements a custom debug trait. + return containerDefault(structureShape).withoutDerives(RuntimeType.Debug) } override fun unionMeta(unionShape: UnionShape): RustMetadata { - return containerDefault + return containerDefault(unionShape) } override fun enumMeta(stringShape: StringShape): RustMetadata { - return containerDefault.withDerives( + return containerDefault(stringShape).withDerives( RuntimeType.std.member("hash::Hash"), ).withDerives( // enums can be eq because they can only contain ints and strings From 079eff0109264ce072426d8b6068664dee00d371 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sat, 26 Nov 2022 21:20:09 -0600 Subject: [PATCH 02/12] Stop excluding the Debug trait locally This commit updates EnumGenerator and StructureGenerator based on the change made to BaseSymbolMetadataProvider in the previous commit. Now that the exclusion of the Debug trait was centralized, those classes in question no longer need to do so individually. --- .../rust/codegen/core/smithy/generators/EnumGenerator.kt | 9 ++------- .../codegen/core/smithy/generators/StructureGenerator.kt | 3 +-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt index 4dc4d62ffd..011c8152ec 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt @@ -94,12 +94,7 @@ open class EnumGenerator( ) { protected val symbol: Symbol = symbolProvider.toSymbol(shape) protected val enumName: String = symbol.name - private val isSensitive = shape.shouldRedact(model) - protected val meta = if (isSensitive) { - symbol.expectRustMetadata().withoutDerives(RuntimeType.Debug) - } else { - symbol.expectRustMetadata() - } + protected val meta = symbol.expectRustMetadata() protected val sortedMembers: List = enumTrait.values.sortedBy { it.value }.map { EnumMemberModel(it, symbolProvider) } protected open var target: CodegenTarget = CodegenTarget.CLIENT @@ -136,7 +131,7 @@ open class EnumGenerator( renderUnnamedEnum() } - if (isSensitive) { + if (shape.shouldRedact(model)) { renderDebugImplForSensitiveEnum() } } diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt index 3972307ac0..3a0be61d40 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt @@ -144,8 +144,7 @@ open class StructureGenerator( val containerMeta = symbol.expectRustMetadata() writer.documentShape(shape, model) writer.deprecatedShape(shape) - val withoutDebug = containerMeta.derives.copy(derives = containerMeta.derives.derives - RuntimeType.Debug) - containerMeta.copy(derives = withoutDebug).render(writer) + containerMeta.render(writer) writer.rustBlock("struct $name ${lifetimeDeclaration()}") { writer.forEachMember(members) { member, memberName, memberSymbol -> From e72fe8c8ef29e5b49f1fe9005dca13f791a33235 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sat, 26 Nov 2022 21:33:08 -0600 Subject: [PATCH 03/12] Implement a custom Debug trait in BuilderGenerator This commit implements a custom Debug trait in BuilderGenerator now that the derived Debug trait is excluded from BaseSymbolMetadataProvider for the structure container. The implementation of the custom Debug trait pretty much follows that of StructureGenerator. --- .../smithy/generators/BuilderGenerator.kt | 24 ++++++++++++++++--- .../smithy/generators/BuilderGeneratorTest.kt | 24 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt index df39a50841..56322a00fa 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt @@ -44,6 +44,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.rustType import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.hasTrait +import software.amazon.smithy.rust.codegen.core.util.redactIfNecessary import software.amazon.smithy.rust.codegen.core.util.toSnakeCase // TODO(https://github.com/awslabs/smithy-rs/issues/1401) This builder generator is only used by the client. @@ -110,6 +111,7 @@ class BuilderGenerator( private val runtimeConfig = symbolProvider.config().runtimeConfig private val members: List = shape.allMembers.values.toList() private val structureSymbol = symbolProvider.toSymbol(shape) + private val builderName = "Builder" fun render(writer: RustWriter) { val symbol = symbolProvider.toSymbol(shape) @@ -117,6 +119,7 @@ class BuilderGenerator( val segments = shape.builderSymbol(symbolProvider).namespace.split("::") writer.withInlineModule(shape.builderSymbol(symbolProvider).module()) { renderBuilder(this) + renderDebugImpl(this) } } @@ -195,12 +198,10 @@ class BuilderGenerator( } private fun renderBuilder(writer: RustWriter) { - val builderName = "Builder" - writer.docs("A builder for #D.", structureSymbol) // Matching derives to the main structure + `Default` since we are a builder and everything is optional. val baseDerives = structureSymbol.expectRustMetadata().derives - val derives = baseDerives.derives.intersect(setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)) + RuntimeType.Default + val derives = baseDerives.derives.intersect(setOf(RuntimeType.PartialEq, RuntimeType.Clone)) + RuntimeType.Default baseDerives.copy(derives = derives).render(writer) writer.rustBlock("pub struct $builderName") { for (member in members) { @@ -232,6 +233,23 @@ class BuilderGenerator( } } + private fun renderDebugImpl(writer: RustWriter) { + writer.rustBlock("impl #T for $builderName", RuntimeType.Debug) { + writer.rustBlock("fn fmt(&self, f: &mut #1T::Formatter<'_>) -> #1T::Result", RuntimeType.stdfmt) { + rust("""let mut formatter = f.debug_struct(${builderName.dq()});""") + members.forEach { member -> + val memberName = symbolProvider.toMemberName(member) + val fieldValue = member.redactIfNecessary(model, "self.$memberName") + + rust( + "formatter.field(${memberName.dq()}, &$fieldValue);", + ) + } + rust("formatter.finish()") + } + } + } + private fun RustWriter.renderVecHelper(member: MemberShape, memberName: String, coreType: RustType.Vec) { docs("Appends an item to `$memberName`.") rust("///") diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt index fb10a12cc4..efbfbe5ab4 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt @@ -24,6 +24,7 @@ internal class BuilderGeneratorTest { private val model = StructureGeneratorTest.model private val inner = StructureGeneratorTest.inner private val struct = StructureGeneratorTest.struct + private val credentials = StructureGeneratorTest.credentials @Test fun `generate builders`() { @@ -94,4 +95,27 @@ internal class BuilderGeneratorTest { """, ) } + + @Test + fun `builder for a struct with sensitive fields should implement the debug trait as such`() { + val provider = testSymbolProvider(model) + val writer = RustWriter.forModule("model") + val credsGenerator = StructureGenerator(model, provider, writer, credentials) + val builderGenerator = BuilderGenerator(model, provider, credentials) + credsGenerator.render() + builderGenerator.render(writer) + writer.implBlock(credentials, provider) { + builderGenerator.renderConvenienceMethod(this) + } + writer.compileAndTest( + """ + use super::*; + let builder = Credentials::builder() + .username("admin") + .password("pswd") + .secret_key("12345"); + assert_eq!(format!("{:?}", builder), "Builder { username: Some(\"admin\"), password: \"*** Sensitive Data Redacted ***\", secret_key: \"*** Sensitive Data Redacted ***\" }"); + """, + ) + } } From efc096f059edfce909185b2d191af6964b24ecfc Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sat, 26 Nov 2022 22:02:34 -0600 Subject: [PATCH 04/12] Implement a custom Debug trait in UnionGenerator This commit implements a custom Debug trait in BuilderGenerator now that the derived Debug trait is excluded from BaseSymbolMetadataProvider for the union container. The implementation of the custom Debug trait pretty much follows that of EnumGenerator. --- .../core/smithy/generators/UnionGenerator.kt | 26 +++++++++++- .../smithy/generators/UnionGeneratorTest.kt | 40 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt index 5075430ed3..977931bafe 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt @@ -16,9 +16,13 @@ import software.amazon.smithy.rust.codegen.core.rustlang.docs import software.amazon.smithy.rust.codegen.core.rustlang.documentShape import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustBlock +import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.smithy.renamedFrom +import software.amazon.smithy.rust.codegen.core.util.REDACTION +import software.amazon.smithy.rust.codegen.core.util.shouldRedact import software.amazon.smithy.rust.codegen.core.util.toSnakeCase fun CodegenTarget.renderUnknownVariant() = when (this) { @@ -47,16 +51,20 @@ class UnionGenerator( private val renderUnknownVariant: Boolean = true, ) { private val sortedMembers: List = shape.allMembers.values.sortedBy { symbolProvider.toMemberName(it) } + private val unionSymbol = symbolProvider.toSymbol(shape) fun render() { renderUnion() + renderImplBlock() + if (shape.shouldRedact(model)) { + renderDebugImplForSensitiveUnion() + } } private fun renderUnion() { writer.documentShape(shape, model) writer.deprecatedShape(shape) - val unionSymbol = symbolProvider.toSymbol(shape) val containerMeta = unionSymbol.expectRustMetadata() containerMeta.render(writer) writer.rustBlock("enum ${unionSymbol.name}") { @@ -82,6 +90,9 @@ class UnionGenerator( rust("Unknown,") } } + } + + private fun renderImplBlock() { writer.rustBlock("impl ${unionSymbol.name}") { sortedMembers.forEach { member -> val memberSymbol = symbolProvider.toSymbol(member) @@ -109,6 +120,19 @@ class UnionGenerator( } } } + private fun renderDebugImplForSensitiveUnion() { + writer.rustTemplate( + """ + impl #{Debug} for ${unionSymbol.name} { + fn fmt(&self, f: &mut #{StdFmt}::Formatter<'_>) -> #{StdFmt}::Result { + write!(f, $REDACTION) + } + } + """, + "Debug" to RuntimeType.Debug, + "StdFmt" to RuntimeType.stdfmt, + ) + } companion object { const val UnknownVariantName = "Unknown" diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt index d2ea8b9de1..d99257f802 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt @@ -15,6 +15,7 @@ import software.amazon.smithy.rust.codegen.core.testutil.TestWorkspace import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest import software.amazon.smithy.rust.codegen.core.testutil.testSymbolProvider +import software.amazon.smithy.rust.codegen.core.util.REDACTION import software.amazon.smithy.rust.codegen.core.util.lookup class UnionGeneratorTest { @@ -125,6 +126,45 @@ class UnionGeneratorTest { project.compileAndTest() } + @Test + fun `impl debug for non-sensitive union should implement the derived debug trait`() { + val writer = generateUnion( + """ + union MyUnion { + foo: PrimitiveInteger + bar: String, + } + """, + ) + + writer.compileAndTest( + """ + assert_eq!(format!("{:?}", MyUnion::Foo(3)), "Foo(3)"); + assert_eq!(format!("{:?}", MyUnion::Bar("bar".to_owned())), "Bar(\"bar\")"); + """, + ) + } + + @Test + fun `impl debug for sensitive union should redact text`() { + val writer = generateUnion( + """ + @sensitive + union MyUnion { + foo: PrimitiveInteger, + bar: String, + } + """, + ) + + writer.compileAndTest( + """ + assert_eq!(format!("{:?}", MyUnion::Foo(3)), $REDACTION); + assert_eq!(format!("{:?}", MyUnion::Bar("bar".to_owned())), $REDACTION); + """, + ) + } + private fun generateUnion(modelSmithy: String, unionName: String = "MyUnion", unknownVariant: Boolean = true): RustWriter { val model = "namespace test\n$modelSmithy".asSmithyModel() val provider: SymbolProvider = testSymbolProvider(model) From 46e23e560616b9c776912e68190bd991d751c66c Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sun, 27 Nov 2022 11:52:40 -0600 Subject: [PATCH 05/12] Implement a custom Debug trait in ServerBuilderGenerator This commit implements a custom Debug trait in ServerBuilderGenerator now that the derived Debug trait is excluded from BaseSymbolMetadataProvider for the structure container. The implementation of the custom Debug trait pretty much follows that of StructureGenerator. --- .../generators/ServerBuilderGenerator.kt | 23 +++++++- .../generators/ServerBuilderGeneratorTest.kt | 52 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt index 9923cc3c7d..cd56dc6f43 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt @@ -39,8 +39,10 @@ import software.amazon.smithy.rust.codegen.core.smithy.mapRustType import software.amazon.smithy.rust.codegen.core.smithy.module import software.amazon.smithy.rust.codegen.core.smithy.rustType import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait +import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.core.util.letIf +import software.amazon.smithy.rust.codegen.core.util.redactIfNecessary import software.amazon.smithy.rust.codegen.core.util.toSnakeCase import software.amazon.smithy.rust.codegen.server.smithy.ServerCodegenContext import software.amazon.smithy.rust.codegen.server.smithy.ServerRuntimeType @@ -166,7 +168,7 @@ class ServerBuilderGenerator( // Matching derives to the main structure, - `PartialEq` (see class documentation for why), + `Default` // since we are a builder and everything is optional. val baseDerives = structureSymbol.expectRustMetadata().derives - val derives = baseDerives.derives.intersect(setOf(RuntimeType.Debug, RuntimeType.Clone)) + RuntimeType.Default + val derives = baseDerives.derives.intersect(setOf(RuntimeType.Clone)) + RuntimeType.Default baseDerives.copy(derives = derives).render(writer) writer.rustBlock("pub${ if (visibility == Visibility.PUBCRATE) " (crate)" else "" } struct Builder") { members.forEach { renderBuilderMember(this, it) } @@ -184,6 +186,8 @@ class ServerBuilderGenerator( } renderBuildFn(this) } + + renderImplDebugForBuilder(writer) } private fun renderImplFromConstraintViolationForRequestRejection(writer: RustWriter) { @@ -417,6 +421,23 @@ class ServerBuilderGenerator( ) } + private fun renderImplDebugForBuilder(writer: RustWriter) { + writer.rustBlock("impl #T for Builder", RuntimeType.Debug) { + writer.rustBlock("fn fmt(&self, f: &mut #1T::Formatter<'_>) -> #1T::Result", RuntimeType.stdfmt) { + rust("""let mut formatter = f.debug_struct("Builder");""") + members.forEach { member -> + val memberName = symbolProvider.toMemberName(member) + val fieldValue = member.redactIfNecessary(model, "self.$memberName") + + rust( + "formatter.field(${memberName.dq()}, &$fieldValue);", + ) + } + rust("formatter.finish()") + } + } + } + /** * Returns the symbol for a builder's member. * All builder members are optional, but only some are `Option`s where `T` needs to be constrained. diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt new file mode 100644 index 0000000000..51f1415caa --- /dev/null +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt @@ -0,0 +1,52 @@ +package software.amazon.smithy.rust.codegen.server.smithy.generators + +import org.junit.jupiter.api.Test +import software.amazon.smithy.model.shapes.StructureShape +import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget +import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator +import software.amazon.smithy.rust.codegen.core.smithy.generators.implBlock +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.testutil.compileAndTest +import software.amazon.smithy.rust.codegen.core.util.lookup +import software.amazon.smithy.rust.codegen.server.smithy.testutil.serverTestCodegenContext + +class ServerBuilderGeneratorTest { + @Test + fun `it respects the sensitive trait in Debug impl`() { + val model = """ + namespace test + @sensitive + string SecretKey + + @sensitive + string Password + + structure Credentials { + username: String, + password: Password, + secretKey: SecretKey + } + """.asSmithyModel() + + val codegenContext = serverTestCodegenContext(model) + val writer = RustWriter.forModule("model") + val shape = model.lookup("test#Credentials") + StructureGenerator(model, codegenContext.symbolProvider, writer, shape).render(CodegenTarget.SERVER) + val builderGenerator = ServerBuilderGenerator(codegenContext, shape) + builderGenerator.render(writer) + writer.implBlock(shape, codegenContext.symbolProvider) { + builderGenerator.renderConvenienceMethod(this) + } + writer.compileAndTest( + """ + use super::*; + let builder = Credentials::builder() + .username(Some("admin".to_owned())) + .password(Some("pswd".to_owned())) + .secret_key(Some("12345".to_owned())); + assert_eq!(format!("{:?}", builder), "Builder { username: Some(\"admin\"), password: \"*** Sensitive Data Redacted ***\", secret_key: \"*** Sensitive Data Redacted ***\" }"); + """, + ) + } +} From c70f2ad77f03d84d071361adef76b4e1d51c81b6 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sun, 27 Nov 2022 13:37:32 -0600 Subject: [PATCH 06/12] Add the Copyright header --- .../server/smithy/generators/ServerBuilderGeneratorTest.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt index 51f1415caa..24688f3654 100644 --- a/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt @@ -1,3 +1,8 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + package software.amazon.smithy.rust.codegen.server.smithy.generators import org.junit.jupiter.api.Test From a681108c6a5759048d285da12d2e6d24a73e656d Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Mon, 28 Nov 2022 11:53:52 -0600 Subject: [PATCH 07/12] Update CHANGELOG.next.toml --- CHANGELOG.next.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 7dedabec59..c4e4e13125 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -487,13 +487,13 @@ meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "all" } author = "lsr0" [[aws-sdk-rust]] -message = "Implementation of the Debug trait for enums can redact what is printed per the sensitive trait." -references = ["smithy-rs#1983"] +message = "Implementation of the Debug trait for container shapes can redact what is printed per the sensitive trait." +references = ["smithy-rs#1983", "smithy-rs#2029"] meta = { "breaking" = true, "tada" = false, "bug" = false } author = "ysaito1001" [[smithy-rs]] -message = "Implementation of the Debug trait for enums can redact what is printed per the sensitive trait." -references = ["smithy-rs#1983"] +message = "Implementation of the Debug trait for container shapes can redact what is printed per the sensitive trait." +references = ["smithy-rs#1983", "smithy-rs#2029"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "all" } author = "ysaito1001" From e3d24ed07f1dffd2a2f0a06b595d1215ce9ff27f Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Tue, 29 Nov 2022 14:49:20 -0600 Subject: [PATCH 08/12] Update Debug impl for UnionGenerator This commit updates the implementation of a custom Debug trait impl for UnionGenerator. Turns out that in a Union, a member target can be marked as sensitive separately outside the Union. Therefore, the implementation of a custom Debug trait has two cases depending on where the sensitive trait appears, either it is applied to the whole Union or to a member target. --- .../core/smithy/generators/UnionGenerator.kt | 41 +++++++++++++++++-- .../smithy/generators/UnionGeneratorTest.kt | 22 ++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt index 977931bafe..3fb2d72bb8 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt @@ -9,6 +9,7 @@ import software.amazon.smithy.codegen.core.SymbolProvider import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.MemberShape import software.amazon.smithy.model.shapes.UnionShape +import software.amazon.smithy.model.traits.SensitiveTrait import software.amazon.smithy.rust.codegen.core.rustlang.Attribute import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.deprecatedShape @@ -22,6 +23,8 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata import software.amazon.smithy.rust.codegen.core.smithy.renamedFrom import software.amazon.smithy.rust.codegen.core.util.REDACTION +import software.amazon.smithy.rust.codegen.core.util.dq +import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.core.util.shouldRedact import software.amazon.smithy.rust.codegen.core.util.toSnakeCase @@ -56,8 +59,12 @@ class UnionGenerator( fun render() { renderUnion() renderImplBlock() - if (shape.shouldRedact(model)) { - renderDebugImplForSensitiveUnion() + if (!unionSymbol.expectRustMetadata().derives.derives.contains(RuntimeType.Debug)) { + if (shape.hasTrait()) { + renderDebugImplForUnion() + } else { + renderDebugImplForUnionMemberWise() + } } } @@ -102,7 +109,11 @@ class UnionGenerator( if (sortedMembers.size == 1) { Attribute.Custom("allow(irrefutable_let_patterns)").render(this) } - rust("/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner #D.", unionSymbol, memberSymbol) + rust( + "/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner #D.", + unionSymbol, + memberSymbol, + ) rust("/// Returns `Err(&Self)` if it can't be converted.") rustBlock("pub fn as_$funcNamePart(&self) -> std::result::Result<&#T, &Self>", memberSymbol) { rust("if let ${unionSymbol.name}::$variantName(val) = &self { Ok(val) } else { Err(self) }") @@ -120,7 +131,8 @@ class UnionGenerator( } } } - private fun renderDebugImplForSensitiveUnion() { + + private fun renderDebugImplForUnion() { writer.rustTemplate( """ impl #{Debug} for ${unionSymbol.name} { @@ -134,10 +146,31 @@ class UnionGenerator( ) } + private fun renderDebugImplForUnionMemberWise() { + writer.rustBlock("impl #T for ${unionSymbol.name}", RuntimeType.Debug) { + writer.rustBlock("fn fmt(&self, f: &mut #1T::Formatter<'_>) -> #1T::Result", RuntimeType.stdfmt) { + rustBlock("match self") { + sortedMembers.forEach { member -> + val memberName = symbolProvider.toMemberName(member) + if (member.shouldRedact(model)) { + rust("${unionSymbol.name}::$memberName(_) => f.debug_tuple($REDACTION).finish(),") + } else { + rust("${unionSymbol.name}::$memberName(val) => f.debug_tuple(${memberName.dq()}).field(&val).finish(),") + } + } + if (renderUnknownVariant) { + rust("${unionSymbol.name}::$UnknownVariantName => f.debug_tuple(${UnknownVariantName.dq()}).finish(),") + } + } + } + } + } + companion object { const val UnknownVariantName = "Unknown" } } + fun unknownVariantError(union: String) = "Cannot serialize `$union::${UnionGenerator.UnknownVariantName}` for the request. " + "The `Unknown` variant is intended for responses only. " + diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt index d99257f802..7b6f32410e 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGeneratorTest.kt @@ -165,6 +165,28 @@ class UnionGeneratorTest { ) } + @Test + fun `impl debug for union should redact text for sensitive member target`() { + val writer = generateUnion( + """ + @sensitive + string Bar + + union MyUnion { + foo: PrimitiveInteger, + bar: Bar, + } + """, + ) + + writer.compileAndTest( + """ + assert_eq!(format!("{:?}", MyUnion::Foo(3)), "Foo(3)"); + assert_eq!(format!("{:?}", MyUnion::Bar("bar".to_owned())), $REDACTION); + """, + ) + } + private fun generateUnion(modelSmithy: String, unionName: String = "MyUnion", unknownVariant: Boolean = true): RustWriter { val model = "namespace test\n$modelSmithy".asSmithyModel() val provider: SymbolProvider = testSymbolProvider(model) From 824d7d6b094a8b112cd31d542cf25208a2898d87 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Tue, 29 Nov 2022 15:50:45 -0600 Subject: [PATCH 09/12] Peek at member sensitivity for Debug trait (in|ex)clusion This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1034205685. With this change, structure shapes no longer need to exclude the Debug trait unconditionally. The upshot is that we may be able to avoid a custom Debug impl for a structure where the derived Debug will do, i.e. when there is no sensitive trait either at a container level or at a member level. --- .../core/smithy/SymbolMetadataProvider.kt | 16 +++++++++------- .../core/smithy/generators/BuilderGenerator.kt | 16 +++++++++------- .../core/smithy/generators/StructureGenerator.kt | 4 +++- .../smithy/generators/ServerBuilderGenerator.kt | 8 +++++--- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index 734f09dc9c..6ae9886cec 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -81,7 +81,14 @@ class BaseSymbolMetadataProvider( private val additionalAttributes: List, ) : SymbolMetadataProvider(base) { private fun containerDefault(shape: Shape): RustMetadata { - val setOfDerives = if (shape.hasTrait()) { + val isSensitive = shape.hasTrait() || + // Checking the shape's direct members for the sensitive trait should suffice. + // Whether their descendants, i.e. a member's member, is sensitive does not + // affect the inclusion/exclusion of the derived Debug trait of _this_ container + // shape; any sensitive descendant should still be printed as redacted. + shape.members().any { it.getMemberTrait(model, SensitiveTrait::class.java).isPresent } + + val setOfDerives = if (isSensitive) { defaultDerives.toSet() - RuntimeType.Debug } else { defaultDerives.toSet() @@ -121,12 +128,7 @@ class BaseSymbolMetadataProvider( } override fun structureMeta(structureShape: StructureShape): RustMetadata { - // In the case of fields being sensitive, we cannot impl the derived Debug - // trait because they reveal the sensitive data. At a container level, - // however, we don't know if a structure has sensitive fields without - // further looking into them. We err on the side of caution and require - // that a structure implements a custom debug trait. - return containerDefault(structureShape).withoutDerives(RuntimeType.Debug) + return containerDefault(structureShape) } override fun unionMeta(unionShape: UnionShape): RustMetadata { diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt index 56322a00fa..522ee95a82 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt @@ -111,15 +111,21 @@ class BuilderGenerator( private val runtimeConfig = symbolProvider.config().runtimeConfig private val members: List = shape.allMembers.values.toList() private val structureSymbol = symbolProvider.toSymbol(shape) + private val builderSymbol = shape.builderSymbol(symbolProvider) + private val baseDerives = structureSymbol.expectRustMetadata().derives + private val builderDerives = baseDerives.derives.intersect(setOf(RuntimeType.Debug, RuntimeType.PartialEq, RuntimeType.Clone)) + RuntimeType.Default private val builderName = "Builder" fun render(writer: RustWriter) { val symbol = symbolProvider.toSymbol(shape) writer.docs("See #D.", symbol) - val segments = shape.builderSymbol(symbolProvider).namespace.split("::") + shape.builderSymbol(symbolProvider).namespace.split("::") writer.withInlineModule(shape.builderSymbol(symbolProvider).module()) { + // Matching derives to the main structure + `Default` since we are a builder and everything is optional. renderBuilder(this) - renderDebugImpl(this) + if (!builderDerives.contains(RuntimeType.Debug)) { + renderDebugImpl(this) + } } } @@ -145,7 +151,6 @@ class BuilderGenerator( } fun renderConvenienceMethod(implBlock: RustWriter) { - val builderSymbol = shape.builderSymbol(symbolProvider) implBlock.docs("Creates a new builder-style object to manufacture #D.", structureSymbol) implBlock.rustBlock("pub fn builder() -> #T", builderSymbol) { write("#T::default()", builderSymbol) @@ -199,10 +204,7 @@ class BuilderGenerator( private fun renderBuilder(writer: RustWriter) { writer.docs("A builder for #D.", structureSymbol) - // Matching derives to the main structure + `Default` since we are a builder and everything is optional. - val baseDerives = structureSymbol.expectRustMetadata().derives - val derives = baseDerives.derives.intersect(setOf(RuntimeType.PartialEq, RuntimeType.Clone)) + RuntimeType.Default - baseDerives.copy(derives = derives).render(writer) + baseDerives.copy(derives = builderDerives).render(writer) writer.rustBlock("pub struct $builderName") { for (member in members) { val memberName = symbolProvider.toMemberName(member) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt index 3a0be61d40..2fb106a609 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt @@ -153,7 +153,9 @@ open class StructureGenerator( } renderStructureImpl() - renderDebugImpl() + if (!containerMeta.derives.derives.contains(RuntimeType.Debug)) { + renderDebugImpl() + } } protected fun RustWriter.forEachMember( diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt index cd56dc6f43..e49a046e64 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt @@ -168,8 +168,8 @@ class ServerBuilderGenerator( // Matching derives to the main structure, - `PartialEq` (see class documentation for why), + `Default` // since we are a builder and everything is optional. val baseDerives = structureSymbol.expectRustMetadata().derives - val derives = baseDerives.derives.intersect(setOf(RuntimeType.Clone)) + RuntimeType.Default - baseDerives.copy(derives = derives).render(writer) + val builderDerives = baseDerives.derives.intersect(setOf(RuntimeType.Debug, RuntimeType.Clone)) + RuntimeType.Default + baseDerives.copy(derives = builderDerives).render(writer) writer.rustBlock("pub${ if (visibility == Visibility.PUBCRATE) " (crate)" else "" } struct Builder") { members.forEach { renderBuilderMember(this, it) } } @@ -187,7 +187,9 @@ class ServerBuilderGenerator( renderBuildFn(this) } - renderImplDebugForBuilder(writer) + if (!builderDerives.contains(RuntimeType.Debug)) { + renderImplDebugForBuilder(writer) + } } private fun renderImplFromConstraintViolationForRequestRejection(writer: RustWriter) { From 0381da2caf721c119e1f67fa966052953ad4a705 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sat, 3 Dec 2022 10:04:14 -0600 Subject: [PATCH 10/12] Remove statement that does not seem to take effect This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036288146 --- .../rust/codegen/core/smithy/generators/BuilderGenerator.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt index 522ee95a82..159c1974d4 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt @@ -119,7 +119,6 @@ class BuilderGenerator( fun render(writer: RustWriter) { val symbol = symbolProvider.toSymbol(shape) writer.docs("See #D.", symbol) - shape.builderSymbol(symbolProvider).namespace.split("::") writer.withInlineModule(shape.builderSymbol(symbolProvider).module()) { // Matching derives to the main structure + `Default` since we are a builder and everything is optional. renderBuilder(this) From 0f8102b5d93919e9d6b968cafce89f9d343d2c95 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sat, 3 Dec 2022 10:06:30 -0600 Subject: [PATCH 11/12] Rename renderDebugImplForUnion -> renderFullyRedactedDebugImpl This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036290722 --- .../rust/codegen/core/smithy/generators/UnionGenerator.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt index 3fb2d72bb8..fdbcad8a9f 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt @@ -61,7 +61,7 @@ class UnionGenerator( renderImplBlock() if (!unionSymbol.expectRustMetadata().derives.derives.contains(RuntimeType.Debug)) { if (shape.hasTrait()) { - renderDebugImplForUnion() + renderFullyRedactedDebugImpl() } else { renderDebugImplForUnionMemberWise() } @@ -132,7 +132,7 @@ class UnionGenerator( } } - private fun renderDebugImplForUnion() { + private fun renderFullyRedactedDebugImpl() { writer.rustTemplate( """ impl #{Debug} for ${unionSymbol.name} { From 64f44cab47ecc352a02b08ebabb0c72856051e16 Mon Sep 17 00:00:00 2001 From: Yuki Saito Date: Sat, 3 Dec 2022 10:07:40 -0600 Subject: [PATCH 12/12] Rename renderDebugImplForUnionMemberWise -> renderDebugImpl This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036291209 --- .../rust/codegen/core/smithy/generators/UnionGenerator.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt index fdbcad8a9f..b0d5ca0ad8 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt @@ -63,7 +63,7 @@ class UnionGenerator( if (shape.hasTrait()) { renderFullyRedactedDebugImpl() } else { - renderDebugImplForUnionMemberWise() + renderDebugImpl() } } } @@ -146,7 +146,7 @@ class UnionGenerator( ) } - private fun renderDebugImplForUnionMemberWise() { + private fun renderDebugImpl() { writer.rustBlock("impl #T for ${unionSymbol.name}", RuntimeType.Debug) { writer.rustBlock("fn fmt(&self, f: &mut #1T::Formatter<'_>) -> #1T::Result", RuntimeType.stdfmt) { rustBlock("match self") {