Standardize the way SdkBytes are unmarshalled when they're part of a payload and the service returns no content. (#3021)

After this change, when SdkBytes are modeled as a payload, the SDK will always return a non-null empty SdkBytes. When SdkBytes are modeled as a field, they will
be null if the service did not specify them.

Author: millems

.changes/next-release/bugfix-AWSSDKforJavav2-98d2e80.json

@@ -0,0 +1,6 @@
+{
+    "category": "AWS SDK for Java v2", 
+    "contributor": "", 
+    "type": "bugfix", 
+    "description": "Always return an empty SDK bytes when services model their response payload as a blob. Previously, it would either return null, empty bytes or throw an exception depending on the protocol, HTTP client and whether the service was using chunked encoding for their responses."
+}

bom-internal/pom.xml

@@ -292,10 +292,17 @@
             </dependency>
             <dependency>
                 <groupId>com.github.tomakehurst</groupId>
-                <artifactId>wiremock</artifactId>
+                <artifactId>wiremock-jre8</artifactId>
                 <version>${wiremock.version}</version>
                 <scope>test</scope>
             </dependency>
+            <!-- Only use wiremock instead of wiremock-jre8 when working around bugs in the newer version -->
+            <dependency>
+                <groupId>com.github.tomakehurst</groupId>
+                <artifactId>wiremock</artifactId>
+                <version>2.18.0</version>
+                <scope>compile</scope>
+            </dependency>
             <dependency>
                 <groupId>com.google.guava</groupId>
                 <artifactId>guava</artifactId>

core/auth-crt/pom.xml

@@ -80,7 +80,7 @@
         </dependency>
         <dependency>
             <groupId>com.github.tomakehurst</groupId>
-            <artifactId>wiremock</artifactId>
+            <artifactId>wiremock-jre8</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>

core/auth/pom.xml

@@ -84,7 +84,7 @@
         </dependency>
         <dependency>
             <groupId>com.github.tomakehurst</groupId>
-            <artifactId>wiremock</artifactId>
+            <artifactId>wiremock-jre8</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>

core/aws-core/pom.xml

@@ -111,7 +111,7 @@
         </dependency>
         <dependency>
             <groupId>com.github.tomakehurst</groupId>
-            <artifactId>wiremock</artifactId>
+            <artifactId>wiremock-jre8</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>

core/metrics-spi/pom.xml

@@ -44,7 +44,7 @@
         </dependency>
         <dependency>
             <groupId>com.github.tomakehurst</groupId>
-            <artifactId>wiremock</artifactId>
+            <artifactId>wiremock-jre8</artifactId>
             <scope>test</scope>
         </dependency>
         <dependency>

core/protocols/aws-json-protocol/src/main/java/software/amazon/awssdk/protocols/json/internal/unmarshall/JsonProtocolUnmarshaller.java

@@ -23,6 +23,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.stream.Collectors;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.annotations.ThreadSafe;
@@ -36,6 +37,7 @@
 import software.amazon.awssdk.core.traits.MapTrait;
 import software.amazon.awssdk.core.traits.PayloadTrait;
 import software.amazon.awssdk.core.traits.TimestampFormatTrait;
+import software.amazon.awssdk.http.AbortableInputStream;
 import software.amazon.awssdk.http.SdkHttpFullResponse;
 import software.amazon.awssdk.protocols.core.StringToInstant;
 import software.amazon.awssdk.protocols.core.StringToValueConverter;
@@ -225,9 +227,13 @@ private static <TypeT extends SdkPojo> TypeT unmarshallStructured(SdkPojo sdkPoj
                                                                       JsonNode jsonContent,
                                                                       JsonUnmarshallerContext context) {
         for (SdkField<?> field : sdkPojo.sdkFields()) {
-            if (isExplicitPayloadMember(field) && field.marshallingType() == MarshallingType.SDK_BYTES &&
-                context.response().content().isPresent()) {
-                field.set(sdkPojo, SdkBytes.fromInputStream(context.response().content().get()));
+            if (isExplicitPayloadMember(field) && field.marshallingType() == MarshallingType.SDK_BYTES) {
+                Optional<AbortableInputStream> responseContent = context.response().content();
+                if (responseContent.isPresent()) {
+                    field.set(sdkPojo, SdkBytes.fromInputStream(responseContent.get()));
+                } else {
+                    field.set(sdkPojo, SdkBytes.fromByteArrayUnsafe(new byte[0]));
+                }
             } else {
                 JsonNode jsonFieldContent = getJsonNode(jsonContent, field);
                 JsonUnmarshaller<Object> unmarshaller = context.getUnmarshaller(field.location(), field.marshallingType());

core/protocols/aws-query-protocol/src/main/java/software/amazon/awssdk/protocols/query/internal/unmarshall/QueryProtocolUnmarshaller.java

@@ -17,21 +17,25 @@
 
 import static software.amazon.awssdk.awscore.util.AwsHeader.AWS_REQUEST_ID;
 import static software.amazon.awssdk.protocols.query.internal.marshall.SimpleTypeQueryMarshaller.defaultTimestampFormats;
+import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
 
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import software.amazon.awssdk.annotations.SdkInternalApi;
+import software.amazon.awssdk.core.SdkBytes;
 import software.amazon.awssdk.core.SdkField;
 import software.amazon.awssdk.core.SdkPojo;
 import software.amazon.awssdk.core.protocol.MarshallingType;
+import software.amazon.awssdk.core.traits.PayloadTrait;
 import software.amazon.awssdk.http.SdkHttpFullResponse;
 import software.amazon.awssdk.protocols.core.StringToInstant;
 import software.amazon.awssdk.protocols.core.StringToValueConverter;
 import software.amazon.awssdk.protocols.query.unmarshall.XmlDomParser;
 import software.amazon.awssdk.protocols.query.unmarshall.XmlElement;
 import software.amazon.awssdk.protocols.query.unmarshall.XmlErrorUnmarshaller;
 import software.amazon.awssdk.utils.CollectionUtils;
+import software.amazon.awssdk.utils.IoUtils;
 import software.amazon.awssdk.utils.Pair;
 import software.amazon.awssdk.utils.builder.Buildable;
 
@@ -69,11 +73,26 @@ private QueryProtocolUnmarshaller(Builder builder) {
 
     public <TypeT extends SdkPojo> Pair<TypeT, Map<String, String>> unmarshall(SdkPojo sdkPojo,
                                                                                SdkHttpFullResponse response) {
+        if (responsePayloadIsBlob(sdkPojo)) {
+            XmlElement document = XmlElement.builder()
+                                 .textContent(response.content()
+                                                      .map(s -> invokeSafely(() -> IoUtils.toUtf8String(s)))
+                                                      .orElse(""))
+                                 .build();
+            return Pair.of(unmarshall(sdkPojo, document, response), new HashMap<>());
+        }
+
         XmlElement document = response.content().map(XmlDomParser::parse).orElseGet(XmlElement::empty);
         XmlElement resultRoot = hasResultWrapper ? document.getFirstChild() : document;
         return Pair.of(unmarshall(sdkPojo, resultRoot, response), parseMetadata(document));
     }
 
+    private boolean responsePayloadIsBlob(SdkPojo sdkPojo) {
+        return sdkPojo.sdkFields().stream()
+                      .anyMatch(field -> field.marshallingType() == MarshallingType.SDK_BYTES &&
+                                         field.containsTrait(PayloadTrait.class));
+    }
+
     /**
      * This method is also used to unmarshall exceptions. We use this since we've already parsed the XML
      * and the result root is in a different location depending on the protocol/service.
@@ -109,6 +128,10 @@ private String metadataKeyName(XmlElement c) {
     private SdkPojo unmarshall(QueryUnmarshallerContext context, SdkPojo sdkPojo, XmlElement root) {
         if (root != null) {
             for (SdkField<?> field : sdkPojo.sdkFields()) {
+                if (field.containsTrait(PayloadTrait.class) && field.marshallingType() == MarshallingType.SDK_BYTES) {
+                    field.set(sdkPojo, SdkBytes.fromUtf8String(root.textContent()));
+                }
+
                 List<XmlElement> element = root.getElementsByName(field.unmarshallLocationName());
                 if (!CollectionUtils.isNullOrEmpty(element)) {
                     QueryUnmarshaller<Object> unmarshaller =
@@ -118,6 +141,7 @@ private SdkPojo unmarshall(QueryUnmarshallerContext context, SdkPojo sdkPojo, Xm
                 }
             }
         }
+
         return (SdkPojo) ((Buildable) sdkPojo).build();
     }
 

core/protocols/aws-query-protocol/src/main/java/software/amazon/awssdk/protocols/query/unmarshall/XmlDomParser.java

@@ -15,6 +15,7 @@
 
 package software.amazon.awssdk.protocols.query.unmarshall;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -27,6 +28,7 @@
 import javax.xml.stream.events.XMLEvent;
 import software.amazon.awssdk.annotations.SdkProtectedApi;
 import software.amazon.awssdk.core.exception.SdkClientException;
+import software.amazon.awssdk.utils.LookaheadInputStream;
 
 /**
  * Parses an XML document into a simple DOM like structure, {@link XmlElement}.
@@ -40,15 +42,20 @@ private XmlDomParser() {
     }
 
     public static XmlElement parse(InputStream inputStream) {
+        LookaheadInputStream stream = new LookaheadInputStream(inputStream);
         try {
-            XMLEventReader reader = FACTORY.get().createXMLEventReader(inputStream);
+            if (stream.peek() == -1) {
+                return XmlElement.empty();
+            }
+
+            XMLEventReader reader = FACTORY.get().createXMLEventReader(stream);
             XMLEvent nextEvent;
             // Skip ahead to the first start element
             do {
                 nextEvent = reader.nextEvent();
             } while (reader.hasNext() && !nextEvent.isStartElement());
             return parseElement(nextEvent.asStartElement(), reader);
-        } catch (XMLStreamException e) {
+        } catch (IOException | XMLStreamException e) {
             throw SdkClientException.create("Could not parse XML response.", e);
         }
     }

core/protocols/aws-xml-protocol/src/main/java/software/amazon/awssdk/protocols/xml/internal/unmarshall/XmlProtocolUnmarshaller.java

@@ -85,7 +85,13 @@ SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement r
 
             if (root != null && field.location() == MarshallLocation.PAYLOAD) {
                 if (!context.response().content().isPresent()) {
-                    // This is a payload field, but the service sent no content. Do not populate this field (leave it null).
+                    // This is a payload field, but the service sent no content. Do not populate this field (leave it null or
+                    // empty).
+                    if (field.marshallingType() == MarshallingType.SDK_BYTES && field.containsTrait(PayloadTrait.class)) {
+                        // SDK bytes bound directly to the payload field should never be left empty
+                        field.set(sdkPojo, SdkBytes.fromByteArrayUnsafe(new byte[0]));
+                    }
+
                     continue;
                 }
 
@@ -98,9 +104,8 @@ SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement r
                                                root.getElementsByName(field.unmarshallLocationName());
 
                     if (!CollectionUtils.isNullOrEmpty(element)) {
-                        boolean isFieldBlobTypePayload = payloadMemberAsBlobType.isPresent()
-                                && payloadMemberAsBlobType.get().equals(field);
-
+                        boolean isFieldBlobTypePayload = payloadMemberAsBlobType.isPresent() &&
+                                                         payloadMemberAsBlobType.get().equals(field);
                         if (isFieldBlobTypePayload) {
                             field.set(sdkPojo, SdkBytes.fromInputStream(context.response().content().get()));
                         } else {