Skip to content

Commit aba6838

Browse files
Issue-3364: Reproduction + fix(Proposal)
Sign-off: Rajan Y.(rajanyadav0307@gmail.com)
1 parent 6f08e2e commit aba6838

File tree

2 files changed

+151
-21
lines changed

2 files changed

+151
-21
lines changed

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

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <algorithm>
2020
#include <cassert>
2121
#include <thread>
22+
#include <ostream>
2223

2324
using namespace Aws::Client;
2425
using namespace Aws::Http;
@@ -181,6 +182,27 @@ static int64_t GetContentLengthFromHeader(CURL* connectionHandle,
181182
return hasContentLength ? static_cast<int64_t>(contentLength) : -1;
182183
}
183184

185+
// Best-effort output position probe for diagnostics only.
186+
// Returns false if the stream does not support positioning.
187+
static bool TryGetOutputPos(std::ostream& os,
188+
std::ostream::pos_type& outPos) noexcept
189+
{
190+
std::streambuf* sb = os.rdbuf();
191+
if (!sb)
192+
{
193+
return false;
194+
}
195+
196+
const auto pos = sb->pubseekoff(0, std::ios_base::cur, std::ios_base::out);
197+
if (pos == std::ostream::pos_type(std::ostream::off_type(-1)))
198+
{
199+
return false;
200+
}
201+
202+
outPos = pos;
203+
return true;
204+
}
205+
184206
static size_t WriteData(char* ptr, size_t size, size_t nmemb, void* userdata)
185207
{
186208
if (ptr)
@@ -217,38 +239,51 @@ static size_t WriteData(char* ptr, size_t size, size_t nmemb, void* userdata)
217239
}
218240
}
219241

220-
if (response->GetResponseBody().fail()) {
221-
const auto& ref = response->GetResponseBody();
222-
AWS_LOGSTREAM_ERROR(CURL_HTTP_CLIENT_TAG, "Response output stream in bad state (eof: "
223-
<< ref.eof() << ", bad: " << ref.bad() << ")");
224-
return 0;
225-
}
242+
auto& body = response->GetResponseBody();
226243

227-
auto cur = response->GetResponseBody().tellp();
228-
if (response->GetResponseBody().fail()) {
229-
const auto& ref = response->GetResponseBody();
230-
AWS_LOGSTREAM_ERROR(CURL_HTTP_CLIENT_TAG, "Unable to query response output position (eof: "
231-
<< ref.eof() << ", bad: " << ref.bad() << ")");
244+
if (body.fail()) {
245+
const auto& ref = body;
246+
AWS_LOGSTREAM_ERROR(CURL_HTTP_CLIENT_TAG,
247+
"Response output stream in bad state (eof: "
248+
<< ref.eof() << ", bad: " << ref.bad() << ")");
232249
return 0;
233250
}
234251

235-
response->GetResponseBody().write(ptr, static_cast<std::streamsize>(sizeToWrite));
236-
if (response->GetResponseBody().fail()) {
237-
const auto& ref = response->GetResponseBody();
238-
AWS_LOGSTREAM_ERROR(CURL_HTTP_CLIENT_TAG, "Failed to write " << size << " / " << sizeToWrite << " B response"
239-
<< " at " << cur << " (eof: " << ref.eof() << ", bad: " << ref.bad() << ")");
252+
body.write(ptr, static_cast<std::streamsize>(sizeToWrite));
253+
if (body.fail()) {
254+
const auto& ref = body;
255+
256+
std::ostream::pos_type pos{};
257+
const bool hasPos = TryGetOutputPos(body, pos);
258+
259+
Aws::StringStream ss;
260+
ss << "Failed to write " << size << " / " << sizeToWrite << " B response";
261+
if (hasPos) {
262+
ss << " at " << pos;
263+
} else {
264+
ss << " (output stream not seekable)";
265+
}
266+
ss << " (received so far: "
267+
<< context->m_numBytesResponseReceived
268+
<< " B, eof: " << ref.eof()
269+
<< ", bad: " << ref.bad() << ")";
270+
271+
AWS_LOGSTREAM_ERROR(CURL_HTTP_CLIENT_TAG, ss.str());
240272
return 0;
241273
}
274+
242275
if ((context->m_request->IsEventStreamRequest() || context->m_request->HasEventStreamResponse() )
243276
&& !response->HasHeader(Aws::Http::X_AMZN_ERROR_TYPE))
244277
{
245-
response->GetResponseBody().flush();
246-
if (response->GetResponseBody().fail()) {
247-
const auto& ref = response->GetResponseBody();
248-
AWS_LOGSTREAM_ERROR(CURL_HTTP_CLIENT_TAG, "Failed to flush event response (eof: "
278+
body.flush();
279+
if (body.fail()) {
280+
const auto& ref = body;
281+
AWS_LOGSTREAM_ERROR(CURL_HTTP_CLIENT_TAG,
282+
"Failed to flush event response (eof: "
249283
<< ref.eof() << ", bad: " << ref.bad() << ")");
250284
return 0;
251285
}
286+
252287
}
253288
auto& receivedHandler = context->m_request->GetDataReceivedEventHandler();
254289
if (receivedHandler)

tests/aws-cpp-sdk-core-tests/http/HttpClientTest.cpp

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ class HttpClientTest : public Aws::Testing::AwsCppSdkGTestSuite
6767
{
6868
};
6969

70+
class CURLHttpClientTest : public Aws::Testing::AwsCppSdkGTestSuite
71+
{
72+
};
73+
74+
7075
TEST_F(HttpClientTest, TestRandomURLWithNoProxy)
7176
{
7277
auto httpClient = CreateHttpClient(Aws::Client::ClientConfiguration());
@@ -382,7 +387,97 @@ TEST_F(CURLHttpClientTest, TestHttpRequestWorksFine)
382387
EXPECT_EQ(Aws::Http::HttpResponseCode::OK, response->GetResponseCode());
383388
EXPECT_EQ("", response->GetClientErrorMessage());
384389
}
390+
391+
#include <aws/core/utils/memory/stl/AWSVector.h>
392+
#include <streambuf>
393+
394+
// A streambuf that supports writing but does NOT support seeking.
395+
// This reproduces the behavior of many filtering / transforming streams.
396+
class NonSeekableWriteBuf final : public std::streambuf
397+
{
398+
public:
399+
explicit NonSeekableWriteBuf(Aws::Vector<char>& out) : m_out(out) {}
400+
401+
protected:
402+
std::streamsize xsputn(const char* s, std::streamsize n) override
403+
{
404+
if (n > 0)
405+
{
406+
m_out.insert(m_out.end(), s, s + static_cast<size_t>(n));
407+
}
408+
return n;
409+
}
410+
411+
int overflow(int ch) override
412+
{
413+
if (ch == traits_type::eof())
414+
{
415+
return traits_type::not_eof(ch);
416+
}
417+
m_out.push_back(static_cast<char>(ch));
418+
return ch;
419+
}
420+
421+
// Disallow positioning (seek/tell)
422+
pos_type seekoff(off_type, std::ios_base::seekdir, std::ios_base::openmode) override
423+
{
424+
return pos_type(off_type(-1));
425+
}
426+
427+
pos_type seekpos(pos_type, std::ios_base::openmode) override
428+
{
429+
return pos_type(off_type(-1));
430+
}
431+
432+
private:
433+
Aws::Vector<char>& m_out;
434+
};
435+
436+
class NonSeekableIOStream final : public Aws::IOStream
437+
{
438+
public:
439+
NonSeekableIOStream(const Aws::String& /*allocationTag*/, Aws::Vector<char>& out)
440+
: Aws::IOStream(nullptr), m_buf(out)
441+
{
442+
rdbuf(&m_buf);
443+
}
444+
445+
private:
446+
NonSeekableWriteBuf m_buf;
447+
};
448+
449+
// Regression test:
450+
// Ensure CurlHttpClient can write response bodies into a non-seekable output stream.
451+
// Older implementations that call tellp() as part of the write callback may fail here.
452+
TEST_F(CURLHttpClientTest, TestNonSeekableResponseStreamDoesNotAbortTransfer)
453+
{
454+
Aws::Vector<char> captured;
455+
456+
auto request = CreateHttpRequest(
457+
Aws::String("http://127.0.0.1:8778"),
458+
HttpMethod::HTTP_GET,
459+
Aws::Utils::Stream::DefaultResponseStreamFactoryMethod);
460+
461+
request->SetHeaderValue("WaitSeconds", "1");
462+
463+
request->SetResponseStreamFactory([&captured]() -> Aws::IOStream*
464+
{
465+
return Aws::New<NonSeekableIOStream>(ALLOCATION_TAG, ALLOCATION_TAG, captured);
466+
});
467+
468+
Aws::Client::ClientConfiguration config;
469+
config.requestTimeoutMs = 10000;
470+
471+
auto httpClient = CreateHttpClient(config);
472+
auto response = httpClient->MakeRequest(request);
473+
474+
ASSERT_NE(nullptr, response);
475+
476+
ASSERT_FALSE(response->HasClientError()) << response->GetClientErrorMessage();
477+
EXPECT_EQ(Aws::Http::HttpResponseCode::OK, response->GetResponseCode());
478+
479+
}
385480
#endif // ENABLE_CURL_CLIENT
386481
#endif // ENABLE_HTTP_CLIENT_TESTING
387482
#endif // NO_HTTP_CLIENT
388-
#endif // DISABLE_DNS_REQUIRED_TESTS
483+
#endif // DISABLE_DNS_REQUIRED_TESTS

0 commit comments

Comments
 (0)