Make sure to rewind the body stream at the start of each request retry attempt. (#1712)

* Make sure to rewind the body stream at the start of each request retry attempt.

* Add change log entry

* Add request start try unit test

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

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; }

sdk/core/azure-core/src/http/retry_policy.cpp

@@ -137,6 +137,7 @@ std::unique_ptr<RawResponse> 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<RawResponse> Azure::Core::Http::RetryPolicy::Send(
       }
     }
 
-    if (auto bodyStream = request.GetBodyStream())
-    {
-      bodyStream->Rewind();
-    }
-
     if (shouldLog)
     {
       std::ostringstream log;

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<Azure::Core::Http::NullBodyStream*>(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<Azure::Core::Http::NullBodyStream*>(req.GetBodyStream());
+      EXPECT_TRUE(d);
+    }
+
+    {
+      Http::HttpMethod httpMethod = Http::HttpMethod::Post;
+      Http::Url url("http://test.com");
+
+      std::vector<uint8_t> 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<uint8_t> 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<Azure::Core::Http::MemoryBodyStream*>(req.GetBodyStream());
+      EXPECT_TRUE(d);
+
+      req.StartTry();
+
+      d = dynamic_cast<Azure::Core::Http::MemoryBodyStream*>(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