From 5487e364bea4e05d678634113ad9d8753372162e Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Mon, 5 Dec 2022 13:44:25 -0600 Subject: [PATCH] Respect the sensitive trait on container shapes (#1983) * Make enum forward-compatible This commit implements the suggested approach described in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092 The idea is that once the user writes a match expression against an enum and assumes that an execution path comes to a particular match arm, we should guarantee that when the user upgrades a version of SDK, the execution path should come to the same match arm as before. * Add unit test to ensure enums are forward-compatible The test first mimics the user's interaction with the enum generated from a model where the user writes a match expression on the enum, wishing to hit the match arm corresponding to Variant3, which is not yet supported by the model. The test then simulates a scenario where the user now has access to the updated enum generated from the next version of the model that does support Variant3. The user's code should compile in both of the use cases. * Generate rustdoc for enum's forward-compatibility This commits generates rustdoc explaining to the users how they might write a match expression against a generated enum so their code is forward-compatible. While it is explained in https://github.com/awslabs/smithy-rs/issues/627#issuecomment-995923092, it can be helpful if the users can read it in rustdoc right below where the enum's definition is shown. * Make snippet in rustdoc text This commit updates a rustdoc code snippet generated for an enum from `rust,no_run` to `text`. We observed that the snippet fails to compile in CI because the name of the enum was not fully qualified; code in rustdoc is treated in a similar way that code in integration tests is treated as being outside of a crate, but not part of the crate, which means the name of the crate needs to be spelled out for a `use` statement if we want to import the enum to the code in rustdoc. However, the name of the crate is usually one-off, generated on the fly for testing, making it not obvious to hard-code it or obtain it programmatically from within Kotlin code. * Suppress missing doc lint for UnknownVariantValue This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because failing to do so will cause tests in CI to fail. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: Russell Cohen * Generate UnknownVariantValue via forInlineFun This commit attempts to create UnknownVariantValue using RuntimeType.forInlineFun. Prior to this commit, it was generated per enum but that caused multiple definitions of UnknownVariantValue in a single file as there can be multiple enums in the file. By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue per module and the enums within the module can refer to the same instance. This commit, however, fails to pass the unit tests in EnumGeneratorTest. The issue seems to be that a RustWriter used in the tests does not end up calling the finalize method, failing to render inline functions. * Replace "target == CodegenTarget.CLIENT" with a helper This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1014390811 but uses an existing helper CodegenTarget.renderUnknownVariant. * Update EnumGeneratorTests to use TestWorkspace This commit addresses failures for unit tests in EnumGeneratorTests. Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need to ensure that the inline functions should be rendered to a Rust source file during testing. RustWriter, used in EnumGeneratorTests prior to this commit, was not capable of doing so. We thus update EnumGeneratorTests to use TestWorkspace by which we ensure that the inline functions are rendered during testing. * Make sure to use the passed-in variable for shapeId This commit fixes a copy-and-paste error introduced in 712c983. The function should use the passed-in variable rather than the hard-coded literal "test#SomeEnum". * Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852 * Update CHANGELOG.next.toml * Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017235315 * Avoid potential name collisions by UnknownVariantValue This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1017237745. It changes the module in which the `UnknownVariantValue` is rendered. Before the commit, it was emitted to the `model` module but that might cause name collisions in the future when a Smithy model has a shape named `UnknownVariantValue`. After this commit, we render it to the `types` module. * Move re-exports from lib.rs to types.rs This commit moves re-export statements in client crates from `lib.rs` to `types.rs`. The motivation is that now that we render the struct `UnknownVariantValue` in a separate file for the `types` module, we can no longer have a `pub mod types {...}` block in `lib.rs` as it cannot coexist with `pub mod types;`. * Add docs on UnknownVariantValue This commit adds public docs on `UnknownVariantValue`. Since it is rendered in `types.rs`, the users may not find the `Unknown` variant of an enum in the same file and may wonder what this struct is for when seeing it in isolation. * Update CHANGELOG.next.toml This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1020414459 * Update the module documentation for types This commit updates rustdoc for the types module to reflect that fact that the module now contains not only re-exports but also an auxiliary struct `UnknownVariantValue`. * Add extensions to run code block depending on the target This commit introduces extensions on CodegenTarget that execute the block of code depending on the target is for client or for server. These extensions are intended to replace if-else expressions throughout the codebase explicitly checking whether the target is for client or for server. We do not update such if-else expressions all at once in this commit. Rather, we are planning to make incremental changes to update them opportunistically. * Respect the sensitive trait on enums This commit fixes #1745. It allows enums to respect the sensitive trait. When annotated as such, an enum will not display its data and instead will show the redacted text upon debug print. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: John DiSanti * Move extensions into CodegenTarget as methods This commit addresses https://github.com/awslabs/smithy-rs/pull/1945#discussion_r1021952411. By moving extensions into the CodegenTarget enum, no separate import is required to use them. * Update codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt Co-authored-by: david-perez * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: david-perez * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: david-perez * Update CHANGELOG.next.toml * Configure the (in|ex)clusion of the Debug trait for containers per members' sensitive trait (#2029) * 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. * 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. * 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. * 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. * 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. * Add the Copyright header * Update CHANGELOG.next.toml * 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. * 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. * Remove statement that does not seem to take effect This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036288146 * Rename renderDebugImplForUnion -> renderFullyRedactedDebugImpl This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036290722 * Rename renderDebugImplForUnionMemberWise -> renderDebugImpl This commit addresses https://github.com/awslabs/smithy-rs/pull/2029#discussion_r1036291209 Co-authored-by: Yuki Saito Co-authored-by: Saito Co-authored-by: Russell Cohen Co-authored-by: Yuki Saito Co-authored-by: John DiSanti Co-authored-by: david-perez --- CHANGELOG.next.toml | 12 ++ .../customize/RequiredCustomizations.kt | 1 + .../core/smithy/SymbolMetadataProvider.kt | 33 +++-- .../smithy/generators/BuilderGenerator.kt | 35 +++-- .../core/smithy/generators/EnumGenerator.kt | 26 ++++ .../smithy/generators/StructureGenerator.kt | 7 +- .../core/smithy/generators/UnionGenerator.kt | 61 +++++++- .../smithy/generators/BuilderGeneratorTest.kt | 24 ++++ .../smithy/generators/EnumGeneratorTest.kt | 131 ++++++++++++++++++ .../smithy/generators/UnionGeneratorTest.kt | 62 +++++++++ .../generators/ServerBuilderGenerator.kt | 27 +++- .../generators/ServerBuilderGeneratorTest.kt | 57 ++++++++ 12 files changed, 452 insertions(+), 24 deletions(-) create mode 100644 codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 3e29e54a45..c7a74a8cde 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -613,3 +613,15 @@ message = "Normalize URI paths per RFC3986 when constructing canonical requests, references = ["smithy-rs#2018"] meta = { "breaking" = false, "tada" = false, "bug" = true } author = "ysaito1001" + +[[aws-sdk-rust]] +message = "Implementation of the Debug trait for container shapes now redacts 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 container shapes now redacts 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" diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt index fd7239e7a0..0567f3a13a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customize/RequiredCustomizations.kt @@ -19,6 +19,7 @@ import software.amazon.smithy.rust.codegen.client.smithy.customizations.pubUseSm import software.amazon.smithy.rust.codegen.client.smithy.generators.config.ConfigCustomization import software.amazon.smithy.rust.codegen.client.smithy.generators.protocol.ClientProtocolGenerator import software.amazon.smithy.rust.codegen.core.rustlang.Feature +import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.smithy.customize.OperationCustomization 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..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 @@ -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,27 @@ 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 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() + } + return RustMetadata( + Attribute.Derives(setOfDerives), + additionalAttributes = additionalAttributes, + visibility = Visibility.PUBLIC, + ) + } override fun memberMeta(memberShape: MemberShape): RustMetadata { val container = model.expectShape(memberShape.container) @@ -113,15 +128,15 @@ class BaseSymbolMetadataProvider( } override fun structureMeta(structureShape: StructureShape): RustMetadata { - return containerDefault + return containerDefault(structureShape) } 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 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..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 @@ -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,13 +111,20 @@ 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("::") writer.withInlineModule(shape.builderSymbol(symbolProvider).module()) { + // Matching derives to the main structure + `Default` since we are a builder and everything is optional. renderBuilder(this) + if (!builderDerives.contains(RuntimeType.Debug)) { + renderDebugImpl(this) + } } } @@ -142,7 +150,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) @@ -195,13 +202,8 @@ 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 - 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) @@ -232,6 +234,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/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 95a46649ea..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 @@ -20,16 +20,19 @@ import software.amazon.smithy.rust.codegen.core.rustlang.documentShape import software.amazon.smithy.rust.codegen.core.rustlang.escape 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.rustlang.withBlock import software.amazon.smithy.rust.codegen.core.smithy.CodegenTarget import software.amazon.smithy.rust.codegen.core.smithy.MaybeRenamed import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata +import software.amazon.smithy.rust.codegen.core.util.REDACTION import software.amazon.smithy.rust.codegen.core.util.doubleQuote import software.amazon.smithy.rust.codegen.core.util.dq import software.amazon.smithy.rust.codegen.core.util.getTrait import software.amazon.smithy.rust.codegen.core.util.orNull +import software.amazon.smithy.rust.codegen.core.util.shouldRedact /** Model that wraps [EnumDefinition] to calculate and cache values required to generate the Rust enum source. */ class EnumMemberModel(private val definition: EnumDefinition, private val symbolProvider: RustSymbolProvider) { @@ -127,6 +130,10 @@ open class EnumGenerator( } else { renderUnnamedEnum() } + + if (shape.shouldRedact(model)) { + renderDebugImplForSensitiveEnum() + } } private fun renderUnnamedEnum() { @@ -229,6 +236,25 @@ open class EnumGenerator( } } + /** + * Manually implement the `Debug` trait for the enum if marked as sensitive. + * + * It prints the redacted text regardless of the variant it is asked to print. + */ + private fun renderDebugImplForSensitiveEnum() { + writer.rustTemplate( + """ + impl #{Debug} for $enumName { + fn fmt(&self, f: &mut #{StdFmt}::Formatter<'_>) -> #{StdFmt}::Result { + write!(f, $REDACTION) + } + } + """, + "Debug" to RuntimeType.Debug, + "StdFmt" to RuntimeType.stdfmt, + ) + } + protected open fun renderFromForStr() { writer.rustBlock("impl #T<&str> for $enumName", RuntimeType.From) { rustBlock("fn from(s: &str) -> Self") { 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..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 @@ -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 -> @@ -154,7 +153,9 @@ open class StructureGenerator( } renderStructureImpl() - renderDebugImpl() + if (!containerMeta.derives.derives.contains(RuntimeType.Debug)) { + renderDebugImpl() + } } protected fun RustWriter.forEachMember( 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..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 @@ -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 @@ -16,9 +17,15 @@ 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.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 fun CodegenTarget.renderUnknownVariant() = when (this) { @@ -47,16 +54,24 @@ 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 (!unionSymbol.expectRustMetadata().derives.derives.contains(RuntimeType.Debug)) { + if (shape.hasTrait()) { + renderFullyRedactedDebugImpl() + } else { + renderDebugImpl() + } + } } 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 +97,9 @@ class UnionGenerator( rust("Unknown,") } } + } + + private fun renderImplBlock() { writer.rustBlock("impl ${unionSymbol.name}") { sortedMembers.forEach { member -> val memberSymbol = symbolProvider.toSymbol(member) @@ -91,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) }") @@ -110,10 +132,45 @@ class UnionGenerator( } } + private fun renderFullyRedactedDebugImpl() { + 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, + ) + } + + 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") { + 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/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 ***\" }"); + """, + ) + } } diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt index 963a436d08..a36fbf0b08 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGeneratorTest.kt @@ -20,6 +20,7 @@ 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.testutil.unitTest +import software.amazon.smithy.rust.codegen.core.util.REDACTION import software.amazon.smithy.rust.codegen.core.util.expectTrait import software.amazon.smithy.rust.codegen.core.util.lookup import software.amazon.smithy.rust.codegen.core.util.orNull @@ -426,4 +427,134 @@ class EnumGeneratorTest { val variant3AsVariant3 = "SomeEnum::Variant3" expectMatchExpressionCompiles(modelV2, "test#SomeEnum", variant3AsVariant3) } + + @Test + fun `impl debug for non-sensitive enum should implement the derived debug trait`() { + val model = """ + namespace test + @enum([ + { name: "Foo", value: "Foo" }, + { name: "Bar", value: "Bar" }, + ]) + string SomeEnum + """.asSmithyModel() + + val shape = model.lookup("test#SomeEnum") + val trait = shape.expectTrait() + val provider = testSymbolProvider(model) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "impl_debug_for_non_sensitive_enum_should_implement_the_derived_debug_trait", + """ + assert_eq!(format!("{:?}", SomeEnum::Foo), "Foo"); + assert_eq!(format!("{:?}", SomeEnum::Bar), "Bar"); + assert_eq!( + format!("{:?}", SomeEnum::from("Baz")), + "Unknown(UnknownVariantValue(\"Baz\"))" + ); + """, + ) + } + project.compileAndTest() + } + + @Test + fun `impl debug for sensitive enum should redact text`() { + val model = """ + namespace test + @sensitive + @enum([ + { name: "Foo", value: "Foo" }, + { name: "Bar", value: "Bar" }, + ]) + string SomeEnum + """.asSmithyModel() + + val shape = model.lookup("test#SomeEnum") + val trait = shape.expectTrait() + val provider = testSymbolProvider(model) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "impl_debug_for_sensitive_enum_should_redact_text", + """ + assert_eq!(format!("{:?}", SomeEnum::Foo), $REDACTION); + assert_eq!(format!("{:?}", SomeEnum::Bar), $REDACTION); + """, + ) + } + project.compileAndTest() + } + + @Test + fun `impl debug for non-sensitive unnamed enum should implement the derived debug trait`() { + val model = """ + namespace test + @enum([ + { value: "Foo" }, + { value: "Bar" }, + ]) + string SomeEnum + """.asSmithyModel() + + val shape = model.lookup("test#SomeEnum") + val trait = shape.expectTrait() + val provider = testSymbolProvider(model) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "impl_debug_for_non_sensitive_unnamed_enum_should_implement_the_derived_debug_trait", + """ + for variant in SomeEnum::values() { + assert_eq!( + format!("{:?}", SomeEnum(variant.to_string())), + format!("SomeEnum(\"{}\")", variant.to_owned()) + ); + } + """, + ) + } + project.compileAndTest() + } + + @Test + fun `impl debug for sensitive unnamed enum should redact text`() { + val model = """ + namespace test + @sensitive + @enum([ + { value: "Foo" }, + { value: "Bar" }, + ]) + string SomeEnum + """.asSmithyModel() + + val shape = model.lookup("test#SomeEnum") + val trait = shape.expectTrait() + val provider = testSymbolProvider(model) + val project = TestWorkspace.testProject(provider) + project.withModule(RustModule.Model) { + val generator = EnumGenerator(model, provider, this, shape, trait) + generator.render() + unitTest( + "impl_debug_for_sensitive_unnamed_enum_should_redact_text", + """ + for variant in SomeEnum::values() { + assert_eq!( + format!("{:?}", SomeEnum(variant.to_string())), + $REDACTION + ); + } + """, + ) + } + project.compileAndTest() + } } 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..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 @@ -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,67 @@ 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); + """, + ) + } + + @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) 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..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 @@ -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,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.Debug, 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) } } @@ -184,6 +186,10 @@ class ServerBuilderGenerator( } renderBuildFn(this) } + + if (!builderDerives.contains(RuntimeType.Debug)) { + renderImplDebugForBuilder(writer) + } } private fun renderImplFromConstraintViolationForRequestRejection(writer: RustWriter) { @@ -417,6 +423,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..24688f3654 --- /dev/null +++ b/codegen-server/src/test/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorTest.kt @@ -0,0 +1,57 @@ +/* + * 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 +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 ***\" }"); + """, + ) + } +}