Skip to content

Commit c10a015

Browse files
bretambrosebretambrose
authored
Merge pull request #29 from awslabs/FB-StreamSeekFix
Fb stream seek fix
2 parents fcae3c8 + 58c7fae commit c10a015

File tree

7 files changed

+39
-45
lines changed

7 files changed

+39
-45
lines changed

‎aws-cpp-sdk-core-tests/utils/stream/PreallocatedStreamBufTest.cpp

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,21 @@ TEST(PreallocatedStreamBufTest, TestStreamWriteHonorsSizeLimitShorterThanBuffer)
162162
buffer[sizeof(bufferStr) - 5] = 0;
163163
ioStream.write(bufferStr, sizeof(bufferStr));
164164
ASSERT_STREQ(shortenedBuffer, (const char*)buffer.GetUnderlyingData());
165-
}
165+
}
166+
167+
TEST(PreallocatedStreamBufTest, TestZeroLengthSeekFromEnd)
168+
{
169+
Array<uint8_t> buffer((uint8_t*)bufferStr, sizeof(bufferStr));
170+
PreallocatedStreamBuf streamBuf(&buffer, buffer.GetLength());
171+
Aws::IOStream ioStream(&streamBuf);
172+
173+
ioStream.seekg(0, std::ios_base::end);
174+
ASSERT_FALSE(ioStream.eof());
175+
176+
// attempting to read a character should fail and hit eof since we're one position after
177+
// the last character
178+
char ch = 0;
179+
ioStream.get(ch);
180+
// could check ch == 0 but I don't think the standard guarantees that
181+
ASSERT_TRUE(ioStream.eof());
182+
}

‎aws-cpp-sdk-core/include/aws/core/http/windows/WinSyncHttpClient.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ namespace Aws
9191
virtual bool DoReadData(void* hHttpRequest, char* body, uint64_t size, uint64_t& read) const = 0;
9292
virtual void* GetClientModule() const = 0;
9393

94-
bool StreamPayloadToRequest(const HttpRequest& request, void* hHttpRequest) const;
94+
bool StreamPayloadToRequest(const HttpRequest& request, void* hHttpRequest, Aws::Utils::RateLimits::RateLimiterInterface* writeLimiter) const;
9595
void LogRequestInternalFailure() const;
9696
std::shared_ptr<HttpResponse> BuildSuccessResponse(const Aws::Http::HttpRequest& request, void* hHttpRequest, Aws::Utils::RateLimits::RateLimiterInterface* readLimiter) const;
9797
void AddHeadersToRequest(const HttpRequest& request, void* hHttpRequest) const;

‎aws-cpp-sdk-core/source/http/curl/CurlHttpClient.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,14 @@ struct CurlWriteCallbackContext
149149

150150
struct CurlReadCallbackContext
151151
{
152-
CurlReadCallbackContext(const CurlHttpClient* client, HttpRequest* request) :
152+
CurlReadCallbackContext(const CurlHttpClient* client, HttpRequest* request, Aws::Utils::RateLimits::RateLimiterInterface* limiter) :
153153
m_client(client),
154+
m_rateLimiter(limiter),
154155
m_request(request)
155156
{}
156157

157158
const CurlHttpClient* m_client;
159+
Aws::Utils::RateLimits::RateLimiterInterface* m_rateLimiter;
158160
HttpRequest* m_request;
159161
};
160162

@@ -341,7 +343,7 @@ std::shared_ptr<HttpResponse> CurlHttpClient::MakeRequest(HttpRequest& request,
341343

342344
response = Aws::MakeShared<StandardHttpResponse>(CURL_HTTP_CLIENT_TAG, request);
343345
CurlWriteCallbackContext writeContext(this, &request, response.get(), readLimiter);
344-
CurlReadCallbackContext readContext(this, &request);
346+
CurlReadCallbackContext readContext(this, &request, writeLimiter);
345347

346348
SetOptCodeForHttpMethod(connectionHandle, request);
347349

@@ -520,6 +522,7 @@ size_t CurlHttpClient::WriteHeader(char* ptr, size_t size, size_t nmemb, void* u
520522

521523
response->AddHeader(headerName, headerValue);
522524
}
525+
523526
return size * nmemb;
524527
}
525528
return 0;
@@ -531,7 +534,7 @@ size_t CurlHttpClient::ReadBody(char* ptr, size_t size, size_t nmemb, void* user
531534
CurlReadCallbackContext* context = reinterpret_cast<CurlReadCallbackContext*>(userdata);
532535
if(context == nullptr)
533536
{
534-
return 0;
537+
return 0;
535538
}
536539

537540
const CurlHttpClient* client = context->m_client;
@@ -554,6 +557,11 @@ size_t CurlHttpClient::ReadBody(char* ptr, size_t size, size_t nmemb, void* user
554557
sentHandler(request, static_cast<long long>(amountRead));
555558
}
556559

560+
if (context->m_rateLimiter)
561+
{
562+
context->m_rateLimiter->ApplyAndPayForCost(static_cast<int64_t>(amountRead));
563+
}
564+
557565
return amountRead;
558566
}
559567

‎aws-cpp-sdk-core/source/http/standard/StandardHttpRequest.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,6 @@ bool StandardHttpRequest::HasHeader(const char* headerName) const
7272
int64_t StandardHttpRequest::GetSize() const
7373
{
7474
int64_t size = 0;
75-
if(bodyStream)
76-
{
77-
bodyStream->seekg(0, bodyStream->end);
78-
size += bodyStream->tellg();
79-
bodyStream->seekg(0, bodyStream->beg);
80-
}
8175

8276
std::for_each(headerMap.cbegin(), headerMap.cend(), [&](const HeaderValueCollection::value_type& kvPair){ size += kvPair.first.length(); size += kvPair.second.length(); });
8377

‎aws-cpp-sdk-core/source/http/windows/WinSyncHttpClient.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ void WinSyncHttpClient::AddHeadersToRequest(const HttpRequest& request, void* hH
9595
}
9696
}
9797

98-
bool WinSyncHttpClient::StreamPayloadToRequest(const HttpRequest& request, void* hHttpRequest) const
98+
bool WinSyncHttpClient::StreamPayloadToRequest(const HttpRequest& request, void* hHttpRequest, Aws::Utils::RateLimits::RateLimiterInterface* writeLimiter) const
9999
{
100100
bool success = true;
101101
auto payloadStream = request.GetContentBody();
@@ -119,6 +119,10 @@ bool WinSyncHttpClient::StreamPayloadToRequest(const HttpRequest& request, void*
119119
{
120120
success = false;
121121
}
122+
else if(writeLimiter)
123+
{
124+
writeLimiter->ApplyAndPayForCost(bytesWritten);
125+
}
122126
}
123127

124128
auto& sentHandler = request.GetDataSentEventHandler();
@@ -289,7 +293,7 @@ std::shared_ptr<HttpResponse> WinSyncHttpClient::MakeRequest(HttpRequest& reques
289293

290294
if(success)
291295
{
292-
success = StreamPayloadToRequest(request, hHttpRequest);
296+
success = StreamPayloadToRequest(request, hHttpRequest, writeLimiter);
293297
}
294298

295299
std::shared_ptr<HttpResponse> response(nullptr);

‎aws-cpp-sdk-core/source/utils/stream/PreallocatedStreamBuf.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ namespace Aws
6060

6161
PreallocatedStreamBuf::pos_type PreallocatedStreamBuf::seekpos(pos_type pos, std::ios_base::openmode which)
6262
{
63-
assert(static_cast<size_t>(pos) < m_lengthToRead);
64-
if (static_cast<size_t>(pos) >= m_lengthToRead)
63+
assert(static_cast<size_t>(pos) <= m_lengthToRead);
64+
if (static_cast<size_t>(pos) > m_lengthToRead)
6565
{
6666
return pos_type(off_type(-1));
6767
}

‎aws-cpp-sdk-transfer-tests/TransferTests.cpp

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -346,12 +346,6 @@ TEST_F(TransferTests, TransferManager_SinglePartUploadTest)
346346

347347
TransferManagerConfiguration transferManagerConfig;
348348
transferManagerConfig.s3Client = m_s3Client;
349-
transferManagerConfig.uploadProgressCallback =
350-
[](const TransferManager*, const TransferHandle& handle)
351-
{ std::cout << "Upload Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n";};
352-
transferManagerConfig.downloadProgressCallback =
353-
[](const TransferManager*, const TransferHandle& handle)
354-
{ std::cout << "Download Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
355349
TransferManager transferManager(transferManagerConfig);
356350

357351
// Test with default behavior of using file name as key
@@ -506,12 +500,7 @@ TEST_F(TransferTests, TransferManager_MediumTest)
506500

507501
TransferManagerConfiguration transferManagerConfig;
508502
transferManagerConfig.s3Client = m_s3Client;
509-
transferManagerConfig.uploadProgressCallback =
510-
[](const TransferManager*, const TransferHandle& handle)
511-
{ std::cout << "Upload Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
512-
transferManagerConfig.downloadProgressCallback =
513-
[](const TransferManager*, const TransferHandle& handle)
514-
{ std::cout << "Download Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
503+
515504
TransferManager transferManager(transferManagerConfig);
516505
std::shared_ptr<TransferHandle> requestPtr = transferManager.UploadFile(MEDIUM_TEST_FILE_NAME, GetTestBucketName(), MEDIUM_FILE_KEY, "text/plain", Aws::Map<Aws::String, Aws::String>());
517506

@@ -582,12 +571,6 @@ TEST_F(TransferTests, TransferManager_BigTest)
582571

583572
TransferManagerConfiguration transferManagerConfig;
584573
transferManagerConfig.s3Client = m_s3Client;
585-
transferManagerConfig.uploadProgressCallback =
586-
[](const TransferManager*, const TransferHandle& handle)
587-
{ std::cout << "Upload Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
588-
transferManagerConfig.downloadProgressCallback =
589-
[](const TransferManager*, const TransferHandle& handle)
590-
{ std::cout << "Download Progress: " << handle.GetBytesTransferred() << " of "<< handle.GetBytesTotalSize() << " bytes\n"; };
591574

592575
TransferManager transferManager(transferManagerConfig);
593576
std::shared_ptr<TransferHandle> requestPtr = transferManager.UploadFile(BIG_TEST_FILE_NAME, GetTestBucketName(), BIG_FILE_KEY, "text/plain", Aws::Map<Aws::String, Aws::String>());
@@ -672,12 +655,6 @@ TEST_F(TransferTests, TransferManager_CancelAndRetryTest)
672655
}
673656
}
674657
};
675-
transferManagerConfig.uploadProgressCallback =
676-
[](const TransferManager*, const TransferHandle& handle)
677-
{ std::cout << "Upload Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
678-
transferManagerConfig.downloadProgressCallback =
679-
[](const TransferManager*, const TransferHandle& handle)
680-
{ std::cout << "Download Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
681658

682659
transferManagerConfig.s3Client = m_s3Client;
683660
TransferManager transferManager(transferManagerConfig);
@@ -774,12 +751,6 @@ TEST_F(TransferTests, TransferManager_AbortAndRetryTest)
774751
}
775752
}
776753
};
777-
transferManagerConfig.uploadProgressCallback =
778-
[](const TransferManager*, const TransferHandle& handle)
779-
{ std::cout << "Upload Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
780-
transferManagerConfig.downloadProgressCallback =
781-
[](const TransferManager*, const TransferHandle& handle)
782-
{ std::cout << "Download Progress: " << handle.GetBytesTransferred() << " of " << handle.GetBytesTotalSize() << " bytes\n"; };
783754

784755
transferManagerConfig.s3Client = m_s3Client;
785756
TransferManager transferManager(transferManagerConfig);

0 commit comments

Comments
 (0)