From f16ee81689bb9cd210352116b5ead731794e5209 Mon Sep 17 00:00:00 2001 From: stickevi Date: Tue, 31 Mar 2020 16:17:45 -0700 Subject: [PATCH] Fix several serde issues with HTTP headers This commit fixes casing comparison issues related to fetch for HTTP bindings of headers and prefix headers for both request and response. It also includes a fix to more robustly validate headers before serialization. --- .../HttpBindingProtocolGenerator.java | 25 +++++++++++-------- .../HttpProtocolGeneratorUtils.java | 11 ++++++++ .../codegen/integration/http-binding-utils.ts | 6 +++++ 3 files changed, 31 insertions(+), 11 deletions(-) create mode 100644 smithy-typescript-codegen/src/main/resources/software/amazon/smithy/typescript/codegen/integration/http-binding-utils.ts diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java index ab3e857f21d..036256eea06 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpBindingProtocolGenerator.java @@ -19,6 +19,7 @@ import java.util.Comparator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -146,6 +147,7 @@ public void generateSharedComponents(GenerationContext context) { HttpProtocolGeneratorUtils.generateMetadataDeserializer(context, getApplicationProtocol().getResponseType()); HttpProtocolGeneratorUtils.generateCollectBody(context); HttpProtocolGeneratorUtils.generateCollectBodyString(context); + HttpProtocolGeneratorUtils.generateHttpBindingUtils(context); } /** @@ -355,10 +357,10 @@ private void writeHeaders( operation.getInput().ifPresent(outputId -> { Model model = context.getModel(); for (HttpBinding binding : bindingIndex.getRequestBindings(operation, Location.HEADER)) { - String memberName = symbolProvider.toMemberName(binding.getMember()); - writer.openBlock("if (input.$L !== undefined) {", "}", memberName, () -> { + String memberLocation = "input." + symbolProvider.toMemberName(binding.getMember()); + writer.openBlock("if (isSerializableHeaderValue($1L)) {", "}", memberLocation, () -> { Shape target = model.expectShape(binding.getMember().getTarget()); - String headerValue = getInputValue(context, binding.getLocation(), "input." + memberName, + String headerValue = getInputValue(context, binding.getLocation(), memberLocation + "!", binding.getMember(), target); writer.write("headers[$S] = $L;", binding.getLocationName(), headerValue); }); @@ -366,15 +368,15 @@ private void writeHeaders( // Handle assembling prefix headers. for (HttpBinding binding : bindingIndex.getRequestBindings(operation, Location.PREFIX_HEADERS)) { - String memberName = symbolProvider.toMemberName(binding.getMember()); - writer.openBlock("if (input.$L !== undefined) {", "}", memberName, () -> { + String memberLocation = "input." + symbolProvider.toMemberName(binding.getMember()); + writer.openBlock("if ($L !== undefined) {", "}", memberLocation, () -> { MapShape prefixMap = model.expectShape(binding.getMember().getTarget()).asMapShape().get(); Shape target = model.expectShape(prefixMap.getValue().getTarget()); // Iterate through each entry in the member. - writer.openBlock("Object.keys(input.$L).forEach(suffix => {", "});", memberName, () -> { + writer.openBlock("Object.keys($L).forEach(suffix => {", "});", memberLocation, () -> { // Use a ! since we already validated the input member is defined above. String headerValue = getInputValue(context, binding.getLocation(), - "input." + memberName + "![suffix]", binding.getMember(), target); + memberLocation + "![suffix]", binding.getMember(), target); // Append the suffix to the defined prefix and serialize the value in to that key. writer.write("headers[$S + suffix] = $L;", binding.getLocationName(), headerValue); }); @@ -951,7 +953,7 @@ private void readHeaders( Model model = context.getModel(); for (HttpBinding binding : bindingIndex.getResponseBindings(operationOrError, Location.HEADER)) { String memberName = symbolProvider.toMemberName(binding.getMember()); - String headerName = binding.getLocationName().toLowerCase(); + String headerName = binding.getLocationName().toLowerCase(Locale.US); writer.openBlock("if ($L.headers[$S] !== undefined) {", "}", outputName, headerName, () -> { Shape target = model.expectShape(binding.getMember().getTarget()); String headerValue = getOutputValue(context, binding.getLocation(), @@ -973,8 +975,9 @@ private void readHeaders( writer.write("contents.$L = {};", memberName); }); - // Generate a single block for each group of prefix headers. - writer.openBlock("if (header.startsWith($S)) {", "}", binding.getLocationName(), () -> { + // Generate a single block for each group of lower-cased prefix headers. + String headerLocation = binding.getLocationName().toLowerCase(Locale.US); + writer.openBlock("if (header.startsWith($S)) {", "}", headerLocation, () -> { MapShape prefixMap = model.expectShape(binding.getMember().getTarget()).asMapShape().get(); Shape target = model.expectShape(prefixMap.getValue().getTarget()); String headerValue = getOutputValue(context, binding.getLocation(), @@ -982,7 +985,7 @@ private void readHeaders( // Extract the non-prefix portion as the key. writer.write("contents.$L[header.substring($L)] = $L;", - memberName, binding.getLocationName().length(), headerValue); + memberName, headerLocation.length(), headerValue); }); } }); diff --git a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java index d58ecb0e4ba..47afb33f680 100644 --- a/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java +++ b/smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/integration/HttpProtocolGeneratorUtils.java @@ -40,6 +40,7 @@ import software.amazon.smithy.typescript.codegen.TypeScriptDependency; import software.amazon.smithy.typescript.codegen.TypeScriptWriter; import software.amazon.smithy.typescript.codegen.integration.ProtocolGenerator.GenerationContext; +import software.amazon.smithy.utils.IoUtils; /** * Utility methods for generating HTTP protocols. @@ -233,6 +234,16 @@ static void generateCollectBodyString(GenerationContext context) { writer.write(""); } + /** + * Writes any additional utils needed for HTTP protocols with bindings. + * + * @param context The generation context. + */ + static void generateHttpBindingUtils(GenerationContext context) { + TypeScriptWriter writer = context.getWriter(); + writer.write(IoUtils.readUtf8Resource(HttpProtocolGeneratorUtils.class, "http-binding-utils.ts")); + } + /** * Writes a function used to dispatch to the proper error deserializer * for each error that the operation can return. The generated function diff --git a/smithy-typescript-codegen/src/main/resources/software/amazon/smithy/typescript/codegen/integration/http-binding-utils.ts b/smithy-typescript-codegen/src/main/resources/software/amazon/smithy/typescript/codegen/integration/http-binding-utils.ts new file mode 100644 index 00000000000..9097eae6464 --- /dev/null +++ b/smithy-typescript-codegen/src/main/resources/software/amazon/smithy/typescript/codegen/integration/http-binding-utils.ts @@ -0,0 +1,6 @@ +function isSerializableHeaderValue(value: any): boolean { + return value !== undefined + && value !== "" + && (!Object.getOwnPropertyNames(value).includes("length") || value.length != 0) + && (!Object.getOwnPropertyNames(value).includes("size") || value.size != 0); +}