From 1614bce979a201ada1e3436358edb7bd1834b5d6 Mon Sep 17 00:00:00 2001 From: Sam Bisciglia Date: Thu, 25 Aug 2022 12:59:59 -0400 Subject: [PATCH] Validates embedded errors for multipart upload. (#2057) --- .../aws/client/AWSClientTest.cpp | 20 +++++++++++++++- .../aws/core/AmazonWebServiceRequest.h | 10 ++++++++ aws-cpp-sdk-core/source/client/AWSClient.cpp | 4 +--- .../model/CompleteMultipartUploadRequest.h | 1 + .../model/CompleteMultipartUploadRequest.cpp | 22 +++++++++++++++++ .../s3/model/CompleteMultipartUploadRequest.h | 1 + .../model/CompleteMultipartUploadRequest.cpp | 22 +++++++++++++++++ .../domainmodels/codegeneration/Shape.java | 9 +++++++ .../cpp/s3/S3RestXmlCppClientGenerator.java | 8 ++++++- .../velocity/cpp/RequestHeader.vm | 3 +++ .../velocity/cpp/xml/XmlRequestSource.vm | 24 +++++++++++++++++++ .../testing/mocks/aws/client/MockAWSClient.h | 11 +++++++++ 12 files changed, 130 insertions(+), 5 deletions(-) diff --git a/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp b/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp index 5011883955f..e68338d6724 100644 --- a/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp +++ b/aws-cpp-sdk-core-tests/aws/client/AWSClientTest.cpp @@ -102,13 +102,18 @@ class AWSClientTestSuite : public ::testing::Test } void QueueMockResponse(HttpResponseCode code, const HeaderValueCollection& headers) + { + QueueMockResponse(code, headers, "ss"); + } + + void QueueMockResponse(HttpResponseCode code, const HeaderValueCollection& headers, const Aws::String& body) { auto httpRequest = CreateHttpRequest(URI("http://www.uri.com/path/to/res"), HttpMethod::HTTP_GET, Aws::Utils::Stream::DefaultResponseStreamFactoryMethod); httpRequest->SetResolvedRemoteHost("127.0.0.1"); auto httpResponse = Aws::MakeShared(ALLOCATION_TAG, httpRequest); httpResponse->SetResponseCode(code); - httpResponse->GetResponseBody() << ""; + httpResponse->GetResponseBody() << body; for(auto&& header : headers) { httpResponse->AddHeader(header.first, header.second); @@ -550,6 +555,19 @@ TEST_F(AWSClientTestSuite, TestRecursionDetection) } } +TEST_F(AWSClientTestSuite, TestErrorInBodyOfResponse) +{ + HeaderValueCollection responseHeaders; + AmazonWebServiceRequestMock request; + QueueMockResponse(HttpResponseCode::OK, responseHeaders, "SomeExceptionTestErrorInBodyOfResponse"); + auto outcome = client->MakeRequest(request); + + ASSERT_TRUE(!outcome.IsSuccess()); + ASSERT_EQ(outcome.GetError().GetErrorType(), CoreErrors::SLOW_DOWN); + ASSERT_EQ(outcome.GetError().GetMessage(), "TestErrorInBodyOfResponse"); + ASSERT_EQ(outcome.GetError().GetExceptionName(), "TestErrorInBodyOfResponse"); +} + TEST(AWSClientTest, TestBuildHttpRequestWithHeadersOnly) { HeaderValueCollection headerValues; diff --git a/aws-cpp-sdk-core/include/aws/core/AmazonWebServiceRequest.h b/aws-cpp-sdk-core/include/aws/core/AmazonWebServiceRequest.h index 181d4b3201f..551270dc80b 100644 --- a/aws-cpp-sdk-core/include/aws/core/AmazonWebServiceRequest.h +++ b/aws-cpp-sdk-core/include/aws/core/AmazonWebServiceRequest.h @@ -14,6 +14,7 @@ #include #include #include +#include namespace Aws { @@ -75,6 +76,15 @@ namespace Aws */ virtual bool SignBody() const { return true; } + /** + * Defaults to false, if a derived class returns true it indicates that the body has an embedded error. + */ + virtual bool HasEmbeddedError(Aws::IOStream& body, const Aws::Http::HeaderValueCollection& header) const { + (void) body; + (void) header; + return false; + } + /** * Defaults to false, if this is set to true, it supports chunked transfer encoding. */ diff --git a/aws-cpp-sdk-core/source/client/AWSClient.cpp b/aws-cpp-sdk-core/source/client/AWSClient.cpp index ea15e452a9f..46bf6ad36e1 100644 --- a/aws-cpp-sdk-core/source/client/AWSClient.cpp +++ b/aws-cpp-sdk-core/source/client/AWSClient.cpp @@ -529,7 +529,7 @@ HttpResponseOutcome AWSClient::AttemptOneRequest(const std::shared_ptrGetResponseBody(), httpResponse->GetHeaders())) { AWS_LOGSTREAM_DEBUG(AWS_CLIENT_LOG_TAG, "Request returned error. Attempting to generate appropriate error codes from response"); auto error = BuildAWSError(httpResponse); @@ -1297,8 +1297,6 @@ AWSError AWSXMLClient::BuildAWSError(const std::shared_ptrGetResponseCode() != HttpResponseCode::OK); - // When trying to build an AWS Error from a response which is an FStream, we need to rewind the // file pointer back to the beginning in order to correctly read the input using the XML string iterator if ((httpResponse->GetResponseBody().tellp() > 0) diff --git a/aws-cpp-sdk-s3-crt/include/aws/s3-crt/model/CompleteMultipartUploadRequest.h b/aws-cpp-sdk-s3-crt/include/aws/s3-crt/model/CompleteMultipartUploadRequest.h index 8cdcfaa7ea7..1963f7648d9 100644 --- a/aws-cpp-sdk-s3-crt/include/aws/s3-crt/model/CompleteMultipartUploadRequest.h +++ b/aws-cpp-sdk-s3-crt/include/aws/s3-crt/model/CompleteMultipartUploadRequest.h @@ -42,6 +42,7 @@ namespace Model Aws::Http::HeaderValueCollection GetRequestSpecificHeaders() const override; + bool HasEmbeddedError(IOStream &body, const Http::HeaderValueCollection &header) const override; /** *

Name of the bucket to which the multipart upload was initiated.

When diff --git a/aws-cpp-sdk-s3-crt/source/model/CompleteMultipartUploadRequest.cpp b/aws-cpp-sdk-s3-crt/source/model/CompleteMultipartUploadRequest.cpp index 837bad9ada1..d42e11ef9db 100644 --- a/aws-cpp-sdk-s3-crt/source/model/CompleteMultipartUploadRequest.cpp +++ b/aws-cpp-sdk-s3-crt/source/model/CompleteMultipartUploadRequest.cpp @@ -35,6 +35,28 @@ CompleteMultipartUploadRequest::CompleteMultipartUploadRequest() : { } +bool CompleteMultipartUploadRequest::HasEmbeddedError(Aws::IOStream &body, + const Aws::Http::HeaderValueCollection &header) const +{ + // Header is unused + (void) header; + + auto readPointer = body.tellg(); + XmlDocument doc = XmlDocument::CreateFromXmlStream(body); + + if (!doc.WasParseSuccessful()) { + body.seekg(readPointer); + return false; + } + + if (doc.GetRootElement().GetName() == "Error") { + body.seekg(readPointer); + return true; + } + body.seekg(readPointer); + return false; +} + Aws::String CompleteMultipartUploadRequest::SerializePayload() const { XmlDocument payloadDoc = XmlDocument::CreateWithRootNode("CompleteMultipartUpload"); diff --git a/aws-cpp-sdk-s3/include/aws/s3/model/CompleteMultipartUploadRequest.h b/aws-cpp-sdk-s3/include/aws/s3/model/CompleteMultipartUploadRequest.h index 1402607da02..d2c11cafe02 100644 --- a/aws-cpp-sdk-s3/include/aws/s3/model/CompleteMultipartUploadRequest.h +++ b/aws-cpp-sdk-s3/include/aws/s3/model/CompleteMultipartUploadRequest.h @@ -42,6 +42,7 @@ namespace Model Aws::Http::HeaderValueCollection GetRequestSpecificHeaders() const override; + bool HasEmbeddedError(IOStream &body, const Http::HeaderValueCollection &header) const override; /** *

Name of the bucket to which the multipart upload was initiated.

When diff --git a/aws-cpp-sdk-s3/source/model/CompleteMultipartUploadRequest.cpp b/aws-cpp-sdk-s3/source/model/CompleteMultipartUploadRequest.cpp index 67daa1effc9..1abb06581cb 100644 --- a/aws-cpp-sdk-s3/source/model/CompleteMultipartUploadRequest.cpp +++ b/aws-cpp-sdk-s3/source/model/CompleteMultipartUploadRequest.cpp @@ -35,6 +35,28 @@ CompleteMultipartUploadRequest::CompleteMultipartUploadRequest() : { } +bool CompleteMultipartUploadRequest::HasEmbeddedError(Aws::IOStream &body, + const Aws::Http::HeaderValueCollection &header) const +{ + // Header is unused + (void) header; + + auto readPointer = body.tellg(); + XmlDocument doc = XmlDocument::CreateFromXmlStream(body); + + if (!doc.WasParseSuccessful()) { + body.seekg(readPointer); + return false; + } + + if (doc.GetRootElement().GetName() == "Error") { + body.seekg(readPointer); + return true; + } + body.seekg(readPointer); + return false; +} + Aws::String CompleteMultipartUploadRequest::SerializePayload() const { XmlDocument payloadDoc = XmlDocument::CreateWithRootNode("CompleteMultipartUpload"); diff --git a/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/domainmodels/codegeneration/Shape.java b/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/domainmodels/codegeneration/Shape.java index f7580bd9c94..d23058909e6 100644 --- a/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/domainmodels/codegeneration/Shape.java +++ b/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/domainmodels/codegeneration/Shape.java @@ -50,6 +50,7 @@ public class Shape { private boolean sensitive; private boolean hasPreSignedUrl; private boolean document; + private boolean hasEmbeddedErrors = false; public boolean isMap() { return "map".equals(type.toLowerCase()); @@ -89,6 +90,14 @@ public boolean isDocument() { return "structure".equals(type.toLowerCase()) && document; } + public boolean hasEmbeddedErrors() { + return this.hasEmbeddedErrors; + } + + public void setEmbeddedErrors(boolean hasEmbeddedErrors) { + this.hasEmbeddedErrors = hasEmbeddedErrors; + } + public boolean isPrimitive() { return !isMap() && !isList() && !isStructure() && !isString() && !isEnum() && !isBlob() && !isTimeStamp(); } diff --git a/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/s3/S3RestXmlCppClientGenerator.java b/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/s3/S3RestXmlCppClientGenerator.java index 65fe8892985..4c940e15fc2 100644 --- a/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/s3/S3RestXmlCppClientGenerator.java +++ b/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/s3/S3RestXmlCppClientGenerator.java @@ -12,13 +12,13 @@ import com.amazonaws.util.awsclientgenerator.domainmodels.codegeneration.cpp.CppShapeInformation; import com.amazonaws.util.awsclientgenerator.domainmodels.codegeneration.cpp.CppViewHelper; import com.amazonaws.util.awsclientgenerator.generators.cpp.RestXmlCppClientGenerator; +import com.google.common.collect.ImmutableSet; import org.apache.velocity.Template; import org.apache.velocity.VelocityContext; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -28,6 +28,7 @@ public class S3RestXmlCppClientGenerator extends RestXmlCppClientGenerator { private static Set opsThatDoNotSupportArnEndpoint = new HashSet<>(); private static Set opsThatDoNotSupportFutureInS3CRT = new HashSet<>(); private static Set bucketLocationConstraints = new HashSet<>(); + private Set functionsWithEmbeddedErrors = ImmutableSet.of("CompleteMultipartUploadRequest"); static { opsThatDoNotSupportVirtualAddressing.add("CreateBucket"); @@ -124,6 +125,11 @@ public SdkFileEntry[] generateSourceFiles(ServiceModel serviceModel) throws Exce replicationStatus.getEnumValues().set(indexOfComplete, "COMPLETED"); } + // Some S3 operations have embedded errors, and we need to search for errors in the response. + serviceModel.getShapes().values().stream() + .filter(shape -> functionsWithEmbeddedErrors.contains(shape.getName())) + .forEach(shape -> shape.setEmbeddedErrors(true)); + // Customized Log Information Shape logTagKeyShape = new Shape(); logTagKeyShape.setName("customizedAccessLogTagKey"); diff --git a/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/RequestHeader.vm b/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/RequestHeader.vm index 75313a2940c..3755e912dce 100644 --- a/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/RequestHeader.vm +++ b/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/RequestHeader.vm @@ -73,6 +73,9 @@ namespace Model Aws::Http::HeaderValueCollection GetRequestSpecificHeaders() const override; #end +#if($shape.hasEmbeddedErrors()) + bool HasEmbeddedError(IOStream &body, const Http::HeaderValueCollection &header) const override; +#end #if($operation.requestAlgorithmMember) Aws::String GetChecksumAlgorithmName() const override; diff --git a/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/xml/XmlRequestSource.vm b/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/xml/XmlRequestSource.vm index 538626eab41..d53970029e0 100644 --- a/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/xml/XmlRequestSource.vm +++ b/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/xml/XmlRequestSource.vm @@ -30,6 +30,30 @@ using namespace Aws::Http; ${typeInfo.className}::${typeInfo.className}()$initializers { } +#if($shape.hasEmbeddedErrors()) + +bool CompleteMultipartUploadRequest::HasEmbeddedError(Aws::IOStream &body, + const Aws::Http::HeaderValueCollection &header) const +{ + // Header is unused + (void) header; + + auto readPointer = body.tellg(); + XmlDocument doc = XmlDocument::CreateFromXmlStream(body); + + if (!doc.WasParseSuccessful()) { + body.seekg(readPointer); + return false; + } + + if (doc.GetRootElement().GetName() == "Error") { + body.seekg(readPointer); + return true; + } + body.seekg(readPointer); + return false; +} +#end Aws::String ${typeInfo.className}::SerializePayload() const { diff --git a/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h b/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h index 8a83baf4381..024cf80600b 100644 --- a/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h +++ b/testing-resources/include/aws/testing/mocks/aws/client/MockAWSClient.h @@ -39,6 +39,13 @@ class AmazonWebServiceRequestMock : public Aws::AmazonWebServiceRequest bool ShouldComputeContentMd5() const override { return m_shouldComputeMd5; } void SetComputeContentMd5(bool value) { m_shouldComputeMd5 = value; } virtual const char* GetServiceRequestName() const override { return "AmazonWebServiceRequestMock"; } + virtual bool HasEmbeddedError(Aws::IOStream& body, const Aws::Http::HeaderValueCollection& header) const override { + (void) header; + std::stringstream ss; + ss << body.rdbuf(); + auto bodyString = ss.str(); + return bodyString.find("TestErrorInBodyOfResponse") != std::string::npos; + } private: std::shared_ptr m_body; @@ -119,6 +126,10 @@ class MockAWSClient : Aws::Client::AWSClient std::shared_ptr m_countedRetryStrategy; Aws::Client::AWSError BuildAWSError(const std::shared_ptr& response) const override { + if (response->GetResponseCode() == Aws::Http::HttpResponseCode::OK) + { + return { Aws::Client::CoreErrors::SLOW_DOWN, "TestErrorInBodyOfResponse", "TestErrorInBodyOfResponse", false }; + } Aws::Client::AWSError error; if (response->HasClientError()) {