Skip to content

Commit bd260cb

Browse files
millemsmillems
authored
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.
1 parent 7334df7 commit bd260cb

File tree

30 files changed

+470
-40
lines changed

30 files changed

+470
-40
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"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."
6+
}

‎bom-internal/pom.xml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,17 @@
292292
</dependency>
293293
<dependency>
294294
<groupId>com.github.tomakehurst</groupId>
295-
<artifactId>wiremock</artifactId>
295+
<artifactId>wiremock-jre8</artifactId>
296296
<version>${wiremock.version}</version>
297297
<scope>test</scope>
298298
</dependency>
299+
<!-- Only use wiremock instead of wiremock-jre8 when working around bugs in the newer version -->
300+
<dependency>
301+
<groupId>com.github.tomakehurst</groupId>
302+
<artifactId>wiremock</artifactId>
303+
<version>2.18.0</version>
304+
<scope>compile</scope>
305+
</dependency>
299306
<dependency>
300307
<groupId>com.google.guava</groupId>
301308
<artifactId>guava</artifactId>

‎core/auth-crt/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@
8080
</dependency>
8181
<dependency>
8282
<groupId>com.github.tomakehurst</groupId>
83-
<artifactId>wiremock</artifactId>
83+
<artifactId>wiremock-jre8</artifactId>
8484
<scope>test</scope>
8585
</dependency>
8686
<dependency>

‎core/auth/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
</dependency>
8585
<dependency>
8686
<groupId>com.github.tomakehurst</groupId>
87-
<artifactId>wiremock</artifactId>
87+
<artifactId>wiremock-jre8</artifactId>
8888
<scope>test</scope>
8989
</dependency>
9090
<dependency>

‎core/aws-core/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@
111111
</dependency>
112112
<dependency>
113113
<groupId>com.github.tomakehurst</groupId>
114-
<artifactId>wiremock</artifactId>
114+
<artifactId>wiremock-jre8</artifactId>
115115
<scope>test</scope>
116116
</dependency>
117117
<dependency>

‎core/metrics-spi/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
</dependency>
4545
<dependency>
4646
<groupId>com.github.tomakehurst</groupId>
47-
<artifactId>wiremock</artifactId>
47+
<artifactId>wiremock-jre8</artifactId>
4848
<scope>test</scope>
4949
</dependency>
5050
<dependency>

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.util.HashMap;
2424
import java.util.List;
2525
import java.util.Map;
26+
import java.util.Optional;
2627
import java.util.stream.Collectors;
2728
import software.amazon.awssdk.annotations.SdkInternalApi;
2829
import software.amazon.awssdk.annotations.ThreadSafe;
@@ -36,6 +37,7 @@
3637
import software.amazon.awssdk.core.traits.MapTrait;
3738
import software.amazon.awssdk.core.traits.PayloadTrait;
3839
import software.amazon.awssdk.core.traits.TimestampFormatTrait;
40+
import software.amazon.awssdk.http.AbortableInputStream;
3941
import software.amazon.awssdk.http.SdkHttpFullResponse;
4042
import software.amazon.awssdk.protocols.core.StringToInstant;
4143
import software.amazon.awssdk.protocols.core.StringToValueConverter;
@@ -225,9 +227,13 @@ private static <TypeT extends SdkPojo> TypeT unmarshallStructured(SdkPojo sdkPoj
225227
JsonNode jsonContent,
226228
JsonUnmarshallerContext context) {
227229
for (SdkField<?> field : sdkPojo.sdkFields()) {
228-
if (isExplicitPayloadMember(field) && field.marshallingType() == MarshallingType.SDK_BYTES &&
229-
context.response().content().isPresent()) {
230-
field.set(sdkPojo, SdkBytes.fromInputStream(context.response().content().get()));
230+
if (isExplicitPayloadMember(field) && field.marshallingType() == MarshallingType.SDK_BYTES) {
231+
Optional<AbortableInputStream> responseContent = context.response().content();
232+
if (responseContent.isPresent()) {
233+
field.set(sdkPojo, SdkBytes.fromInputStream(responseContent.get()));
234+
} else {
235+
field.set(sdkPojo, SdkBytes.fromByteArrayUnsafe(new byte[0]));
236+
}
231237
} else {
232238
JsonNode jsonFieldContent = getJsonNode(jsonContent, field);
233239
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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,25 @@
1717

1818
import static software.amazon.awssdk.awscore.util.AwsHeader.AWS_REQUEST_ID;
1919
import static software.amazon.awssdk.protocols.query.internal.marshall.SimpleTypeQueryMarshaller.defaultTimestampFormats;
20+
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
2021

2122
import java.util.HashMap;
2223
import java.util.List;
2324
import java.util.Map;
2425
import software.amazon.awssdk.annotations.SdkInternalApi;
26+
import software.amazon.awssdk.core.SdkBytes;
2527
import software.amazon.awssdk.core.SdkField;
2628
import software.amazon.awssdk.core.SdkPojo;
2729
import software.amazon.awssdk.core.protocol.MarshallingType;
30+
import software.amazon.awssdk.core.traits.PayloadTrait;
2831
import software.amazon.awssdk.http.SdkHttpFullResponse;
2932
import software.amazon.awssdk.protocols.core.StringToInstant;
3033
import software.amazon.awssdk.protocols.core.StringToValueConverter;
3134
import software.amazon.awssdk.protocols.query.unmarshall.XmlDomParser;
3235
import software.amazon.awssdk.protocols.query.unmarshall.XmlElement;
3336
import software.amazon.awssdk.protocols.query.unmarshall.XmlErrorUnmarshaller;
3437
import software.amazon.awssdk.utils.CollectionUtils;
38+
import software.amazon.awssdk.utils.IoUtils;
3539
import software.amazon.awssdk.utils.Pair;
3640
import software.amazon.awssdk.utils.builder.Buildable;
3741

@@ -69,11 +73,26 @@ private QueryProtocolUnmarshaller(Builder builder) {
6973

7074
public <TypeT extends SdkPojo> Pair<TypeT, Map<String, String>> unmarshall(SdkPojo sdkPojo,
7175
SdkHttpFullResponse response) {
76+
if (responsePayloadIsBlob(sdkPojo)) {
77+
XmlElement document = XmlElement.builder()
78+
.textContent(response.content()
79+
.map(s -> invokeSafely(() -> IoUtils.toUtf8String(s)))
80+
.orElse(""))
81+
.build();
82+
return Pair.of(unmarshall(sdkPojo, document, response), new HashMap<>());
83+
}
84+
7285
XmlElement document = response.content().map(XmlDomParser::parse).orElseGet(XmlElement::empty);
7386
XmlElement resultRoot = hasResultWrapper ? document.getFirstChild() : document;
7487
return Pair.of(unmarshall(sdkPojo, resultRoot, response), parseMetadata(document));
7588
}
7689

90+
private boolean responsePayloadIsBlob(SdkPojo sdkPojo) {
91+
return sdkPojo.sdkFields().stream()
92+
.anyMatch(field -> field.marshallingType() == MarshallingType.SDK_BYTES &&
93+
field.containsTrait(PayloadTrait.class));
94+
}
95+
7796
/**
7897
* This method is also used to unmarshall exceptions. We use this since we've already parsed the XML
7998
* and the result root is in a different location depending on the protocol/service.
@@ -109,6 +128,10 @@ private String metadataKeyName(XmlElement c) {
109128
private SdkPojo unmarshall(QueryUnmarshallerContext context, SdkPojo sdkPojo, XmlElement root) {
110129
if (root != null) {
111130
for (SdkField<?> field : sdkPojo.sdkFields()) {
131+
if (field.containsTrait(PayloadTrait.class) && field.marshallingType() == MarshallingType.SDK_BYTES) {
132+
field.set(sdkPojo, SdkBytes.fromUtf8String(root.textContent()));
133+
}
134+
112135
List<XmlElement> element = root.getElementsByName(field.unmarshallLocationName());
113136
if (!CollectionUtils.isNullOrEmpty(element)) {
114137
QueryUnmarshaller<Object> unmarshaller =
@@ -118,6 +141,7 @@ private SdkPojo unmarshall(QueryUnmarshallerContext context, SdkPojo sdkPojo, Xm
118141
}
119142
}
120143
}
144+
121145
return (SdkPojo) ((Buildable) sdkPojo).build();
122146
}
123147

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.awssdk.protocols.query.unmarshall;
1717

18+
import java.io.IOException;
1819
import java.io.InputStream;
1920
import java.util.HashMap;
2021
import java.util.Iterator;
@@ -27,6 +28,7 @@
2728
import javax.xml.stream.events.XMLEvent;
2829
import software.amazon.awssdk.annotations.SdkProtectedApi;
2930
import software.amazon.awssdk.core.exception.SdkClientException;
31+
import software.amazon.awssdk.utils.LookaheadInputStream;
3032

3133
/**
3234
* Parses an XML document into a simple DOM like structure, {@link XmlElement}.
@@ -40,15 +42,20 @@ private XmlDomParser() {
4042
}
4143

4244
public static XmlElement parse(InputStream inputStream) {
45+
LookaheadInputStream stream = new LookaheadInputStream(inputStream);
4346
try {
44-
XMLEventReader reader = FACTORY.get().createXMLEventReader(inputStream);
47+
if (stream.peek() == -1) {
48+
return XmlElement.empty();
49+
}
50+
51+
XMLEventReader reader = FACTORY.get().createXMLEventReader(stream);
4552
XMLEvent nextEvent;
4653
// Skip ahead to the first start element
4754
do {
4855
nextEvent = reader.nextEvent();
4956
} while (reader.hasNext() && !nextEvent.isStartElement());
5057
return parseElement(nextEvent.asStartElement(), reader);
51-
} catch (XMLStreamException e) {
58+
} catch (IOException | XMLStreamException e) {
5259
throw SdkClientException.create("Could not parse XML response.", e);
5360
}
5461
}

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,13 @@ SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement r
8585

8686
if (root != null && field.location() == MarshallLocation.PAYLOAD) {
8787
if (!context.response().content().isPresent()) {
88-
// This is a payload field, but the service sent no content. Do not populate this field (leave it null).
88+
// This is a payload field, but the service sent no content. Do not populate this field (leave it null or
89+
// empty).
90+
if (field.marshallingType() == MarshallingType.SDK_BYTES && field.containsTrait(PayloadTrait.class)) {
91+
// SDK bytes bound directly to the payload field should never be left empty
92+
field.set(sdkPojo, SdkBytes.fromByteArrayUnsafe(new byte[0]));
93+
}
94+
8995
continue;
9096
}
9197

@@ -98,9 +104,8 @@ SdkPojo unmarshall(XmlUnmarshallerContext context, SdkPojo sdkPojo, XmlElement r
98104
root.getElementsByName(field.unmarshallLocationName());
99105

100106
if (!CollectionUtils.isNullOrEmpty(element)) {
101-
boolean isFieldBlobTypePayload = payloadMemberAsBlobType.isPresent()
102-
&& payloadMemberAsBlobType.get().equals(field);
103-
107+
boolean isFieldBlobTypePayload = payloadMemberAsBlobType.isPresent() &&
108+
payloadMemberAsBlobType.get().equals(field);
104109
if (isFieldBlobTypePayload) {
105110
field.set(sdkPojo, SdkBytes.fromInputStream(context.response().content().get()));
106111
} else {

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@
1515

1616
package software.amazon.awssdk.protocols.xml.internal.unmarshall;
1717

18-
import java.io.BufferedInputStream;
1918
import java.io.IOException;
20-
import java.io.InputStream;
2119
import java.io.UncheckedIOException;
2220
import java.util.Optional;
2321
import software.amazon.awssdk.annotations.SdkInternalApi;
@@ -30,6 +28,7 @@
3028
import software.amazon.awssdk.http.SdkHttpFullResponse;
3129
import software.amazon.awssdk.protocols.query.unmarshall.XmlDomParser;
3230
import software.amazon.awssdk.protocols.query.unmarshall.XmlElement;
31+
import software.amazon.awssdk.utils.LookaheadInputStream;
3332

3433
/**
3534
* Static methods to assist with parsing the response of AWS XML requests.
@@ -57,12 +56,10 @@ public static XmlElement parse(SdkPojo sdkPojo, SdkHttpFullResponse response) {
5756
}
5857

5958
// Make sure there is content in the stream before passing it to the parser.
60-
InputStream content = ensureMarkSupported(responseContent.get());
61-
content.mark(2);
62-
if (content.read() == -1) {
59+
LookaheadInputStream content = new LookaheadInputStream(responseContent.get());
60+
if (content.peek() == -1) {
6361
return XmlElement.empty();
6462
}
65-
content.reset();
6663

6764
return XmlDomParser.parse(content);
6865
} catch (IOException e) {
@@ -75,14 +72,6 @@ public static XmlElement parse(SdkPojo sdkPojo, SdkHttpFullResponse response) {
7572
}
7673
}
7774

78-
private static InputStream ensureMarkSupported(AbortableInputStream content) {
79-
if (content.markSupported()) {
80-
return content;
81-
}
82-
83-
return new BufferedInputStream(content);
84-
}
85-
8675
/**
8776
* Gets the Member which is a Payload and which is of Blob Type.
8877
* @param sdkPojo

‎core/regions/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
</dependency>
7676
<dependency>
7777
<groupId>com.github.tomakehurst</groupId>
78-
<artifactId>wiremock</artifactId>
78+
<artifactId>wiremock-jre8</artifactId>
7979
<scope>test</scope>
8080
</dependency>
8181
<dependency>

‎core/sdk-core/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@
8383
</dependency>
8484
<dependency>
8585
<groupId>com.github.tomakehurst</groupId>
86-
<artifactId>wiremock</artifactId>
86+
<artifactId>wiremock-jre8</artifactId>
8787
<scope>test</scope>
8888
</dependency>
8989
<dependency>

‎http-clients/apache-client/pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
<artifactId>hamcrest-all</artifactId>
8484
<scope>test</scope>
8585
</dependency>
86+
<!-- Use wiremock instead of wiremock-jre8 because HEAD request handling doesn't work in the newer versions. -->
8687
<dependency>
8788
<groupId>com.github.tomakehurst</groupId>
8889
<artifactId>wiremock</artifactId>

‎http-clients/aws-crt-client/pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
</dependency>
7474

7575
<!--Test Dependencies-->
76+
<!-- Use wiremock instead of wiremock-jre8 because HEAD request handling doesn't work in the newer versions. -->
7677
<dependency>
7778
<groupId>com.github.tomakehurst</groupId>
7879
<artifactId>wiremock</artifactId>

‎http-clients/netty-nio-client/pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@
107107
<version>${awsjavasdk.version}</version>
108108
<scope>test</scope>
109109
</dependency>
110+
<!-- Use wiremock instead of wiremock-jre8 because HEAD request handling doesn't work in the newer versions. -->
110111
<dependency>
111112
<groupId>com.github.tomakehurst</groupId>
112113
<artifactId>wiremock</artifactId>

‎metric-publishers/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@
9898
<scope>test</scope>
9999
</dependency>
100100
<dependency>
101-
<artifactId>wiremock</artifactId>
101+
<artifactId>wiremock-jre8</artifactId>
102102
<groupId>com.github.tomakehurst</groupId>
103103
<scope>test</scope>
104104
</dependency>

‎pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@
9595
<jacksonjr.version>2.13.1</jacksonjr.version>
9696
<eventstream.version>1.0.1</eventstream.version>
9797
<commons.lang.version>3.12.0</commons.lang.version>
98-
<wiremock.version>2.18.0</wiremock.version>
98+
<wiremock.version>2.32.0</wiremock.version>
9999
<slf4j.version>1.7.30</slf4j.version>
100100
<log4j.version>2.17.1</log4j.version>
101101
<commons.io.version>2.5</commons.io.version>

‎services-custom/dynamodb-enhanced/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@
175175
</dependency>
176176
<dependency>
177177
<groupId>com.github.tomakehurst</groupId>
178-
<artifactId>wiremock</artifactId>
178+
<artifactId>wiremock-jre8</artifactId>
179179
<scope>test</scope>
180180
</dependency>
181181
<dependency>

‎services/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@
447447
<scope>test</scope>
448448
</dependency>
449449
<dependency>
450-
<artifactId>wiremock</artifactId>
450+
<artifactId>wiremock-jre8</artifactId>
451451
<groupId>com.github.tomakehurst</groupId>
452452
<scope>test</scope>
453453
</dependency>

‎services/s3/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@
112112
</dependency>
113113
<dependency>
114114
<groupId>com.github.tomakehurst</groupId>
115-
<artifactId>wiremock</artifactId>
115+
<artifactId>wiremock-jre8</artifactId>
116116
<scope>test</scope>
117117
</dependency>
118118
<dependency>

‎test/auth-sts-testing/pom.xml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,12 @@
8383
</dependency>
8484
<dependency>
8585
<groupId>com.github.tomakehurst</groupId>
86-
<artifactId>wiremock</artifactId>
86+
<artifactId>wiremock-jre8</artifactId>
87+
<scope>test</scope>
88+
</dependency>
89+
<dependency>
90+
<groupId>junit</groupId>
91+
<artifactId>junit</artifactId>
8792
<scope>test</scope>
8893
</dependency>
8994
<dependency>

‎test/codegen-generated-classes-test/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@
129129
</dependency>
130130
<dependency>
131131
<groupId>com.github.tomakehurst</groupId>
132-
<artifactId>wiremock</artifactId>
132+
<artifactId>wiremock-jre8</artifactId>
133133
<scope>test</scope>
134134
</dependency>
135135
<dependency>

0 commit comments

Comments
 (0)