diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index e3ce9aee7..7d4518523 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -11,6 +11,10 @@ - Renamed `GetString()` to `ToString()` in `Azure::Core::DateTime`. - Renamed `GetUuidString()` tp `ToString()` in `Azure::Core::Uuid`. +### Bug Fixes + +- Make sure to rewind the body stream at the start of each request retry attempt, including the first. + ## 1.0.0-beta.6 (2021-02-09) ### New Features diff --git a/sdk/core/azure-core/src/http/request.cpp b/sdk/core/azure-core/src/http/request.cpp index 0e17bbccb..f539a03f2 100644 --- a/sdk/core/azure-core/src/http/request.cpp +++ b/sdk/core/azure-core/src/http/request.cpp @@ -40,6 +40,13 @@ void Request::StartTry() { this->m_retryModeEnabled = true; this->m_retryHeaders.clear(); + + // Make sure to rewind the body stream before each attempt, including the first. + // It's possible the request doesn't have a body, so make sure to check if a body stream exists. + if (auto bodyStream = this->GetBodyStream()) + { + bodyStream->Rewind(); + } } HttpMethod Request::GetMethod() const { return this->m_method; } diff --git a/sdk/core/azure-core/src/http/retry_policy.cpp b/sdk/core/azure-core/src/http/retry_policy.cpp index dc4e80398..0638a1f10 100644 --- a/sdk/core/azure-core/src/http/retry_policy.cpp +++ b/sdk/core/azure-core/src/http/retry_policy.cpp @@ -137,6 +137,7 @@ std::unique_ptr Azure::Core::Http::RetryPolicy::Send( request.StartTry(); // creates a copy of original query parameters from request auto originalQueryParameters = request.GetUrl().GetQueryParameters(); + try { auto response = nextHttpPolicy.Send(ctx, request); @@ -158,11 +159,6 @@ std::unique_ptr Azure::Core::Http::RetryPolicy::Send( } } - if (auto bodyStream = request.GetBodyStream()) - { - bodyStream->Rewind(); - } - if (shouldLog) { std::ostringstream log; diff --git a/sdk/core/azure-core/test/ut/http.cpp b/sdk/core/azure-core/test/ut/http.cpp index 5977b38fb..b32245004 100644 --- a/sdk/core/azure-core/test/ut/http.cpp +++ b/sdk/core/azure-core/test/ut/http.cpp @@ -159,4 +159,73 @@ namespace Azure { namespace Core { namespace Test { EXPECT_FALSE(r.Length.HasValue()); } } + + TEST(TestHttp, RequestStartTry) + { + { + Http::HttpMethod httpMethod = Http::HttpMethod::Post; + Http::Url url("http://test.com"); + Http::Request req(httpMethod, url); + + Azure::Core::Http::NullBodyStream* d + = dynamic_cast(req.GetBodyStream()); + EXPECT_TRUE(d); + + req.StartTry(); + + EXPECT_NO_THROW(req.AddHeader("namE", "retryValue")); + + auto headers = req.GetHeaders(); + + EXPECT_TRUE(headers.count("name")); + + req.StartTry(); + headers = req.GetHeaders(); + + EXPECT_FALSE(headers.count("name")); + + d = dynamic_cast(req.GetBodyStream()); + EXPECT_TRUE(d); + } + + { + Http::HttpMethod httpMethod = Http::HttpMethod::Post; + Http::Url url("http://test.com"); + + std::vector data = {1, 2, 3, 4}; + Azure::Core::Http::MemoryBodyStream stream(data); + + // Change the offset of the stream to be non-zero by reading a byte. + std::vector temp(2); + EXPECT_EQ( + Azure::Core::Http::BodyStream::ReadToCount( + GetApplicationContext(), stream, temp.data(), 1), + 1); + + EXPECT_EQ(temp[0], 1); + EXPECT_EQ(temp[1], 0); + + Http::Request req(httpMethod, url, &stream); + + Azure::Core::Http::MemoryBodyStream* d + = dynamic_cast(req.GetBodyStream()); + EXPECT_TRUE(d); + + req.StartTry(); + + d = dynamic_cast(req.GetBodyStream()); + EXPECT_TRUE(d); + + // Verify that StartTry rewound the stream back. + auto getStream = req.GetBodyStream(); + EXPECT_EQ( + Azure::Core::Http::BodyStream::ReadToCount( + GetApplicationContext(), *getStream, temp.data(), 2), + 2); + + EXPECT_EQ(temp[0], 1); + EXPECT_EQ(temp[1], 2); + } + } + }}} // namespace Azure::Core::Test