Skip to content

Commit

Permalink
Fix several serde issues with HTTP headers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kstich committed Apr 1, 2020
1 parent b9300aa commit f16ee81
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -146,6 +147,7 @@ public void generateSharedComponents(GenerationContext context) {
HttpProtocolGeneratorUtils.generateMetadataDeserializer(context, getApplicationProtocol().getResponseType());
HttpProtocolGeneratorUtils.generateCollectBody(context);
HttpProtocolGeneratorUtils.generateCollectBodyString(context);
HttpProtocolGeneratorUtils.generateHttpBindingUtils(context);
}

/**
Expand Down Expand Up @@ -355,26 +357,26 @@ 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);
});
}

// 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);
});
Expand Down Expand Up @@ -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(),
Expand All @@ -973,16 +975,17 @@ 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(),
outputName + ".headers[header]", binding.getMember(), target);

// 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);
});
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit f16ee81

Please sign in to comment.