Skip to content

Commit 922b02d

Browse files
stgatesstgates
committed
Merge branch 'development' of https://git01.codeplex.com/casablanca into enable_stream_timeout
2 parents 9a69f5c + bad874e commit 922b02d

File tree

7 files changed

+82
-60
lines changed

7 files changed

+82
-60
lines changed

‎.gitignore

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,8 @@ obj/
4444
[Bb]in
4545
Binaries/
4646
Intermediate/
47+
# The packages folder can be ignored because of Package Restore
48+
**/packages/*
49+
# except build/, which is used as an MSBuild target.
50+
!**/packages/build/
51+

‎Release/include/cpprest/containerstream.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,9 @@ namespace Concurrency { namespace streams {
325325
{
326326
pos_type beg(0);
327327

328-
// Inorder to support relative seeking from the end postion we need to fix an end position.
328+
// In order to support relative seeking from the end position we need to fix an end position.
329329
// Technically, there is no end for the stream buffer as new writes would just expand the buffer.
330-
// For now, we assume that the current write_end is the end of the buffer. We use this aritifical
330+
// For now, we assume that the current write_end is the end of the buffer. We use this artificial
331331
// end to restrict the read head from seeking beyond what is available.
332332

333333
pos_type end(m_size);

‎Release/src/http/client/http_linux.cpp

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ namespace web { namespace http
436436
boost::asio::deadline_timer m_timer;
437437
};
438438

439-
uint64_t m_known_size;
439+
uint64_t m_content_length;
440440
bool m_needChunked;
441441
timeout_timer m_timer;
442442
boost::asio::streambuf m_body_buf;
@@ -526,7 +526,7 @@ namespace web { namespace http
526526
{
527527
ctx->m_needChunked = true;
528528
}
529-
else if (!ctx->m_request.headers().match(header_names::content_length, ctx->m_known_size))
529+
else if (!ctx->m_request.headers().match(header_names::content_length, ctx->m_content_length))
530530
{
531531
// Stream without content length is the signal of requiring transfer encoding chunked.
532532
if (ctx->m_request.body())
@@ -734,7 +734,7 @@ namespace web { namespace http
734734

735735
void handle_write_large_body(const boost::system::error_code& ec, const std::shared_ptr<linux_client_request_context> &ctx)
736736
{
737-
if (ec || ctx->m_uploaded >= ctx->m_known_size)
737+
if (ec || ctx->m_uploaded >= ctx->m_content_length)
738738
{
739739
// Reuse error handling.
740740
return handle_write_body(ec, ctx);
@@ -762,15 +762,20 @@ namespace web { namespace http
762762
ctx->m_connection->async_write(ctx->m_body_buf, boost::bind(&linux_client::handle_write_large_body, shared_from_this(), boost::asio::placeholders::error, ctx));
763763
};
764764

765-
const auto readSize = static_cast<size_t>(std::min(static_cast<uint64_t>(client_config().chunksize()), ctx->m_known_size - ctx->m_uploaded));
766-
765+
const auto readSize = static_cast<size_t>(std::min(static_cast<uint64_t>(client_config().chunksize()), ctx->m_content_length - ctx->m_uploaded));
767766
auto readbuf = ctx->_get_readbuffer();
768767
readbuf.getn(boost::asio::buffer_cast<uint8_t *>(ctx->m_body_buf.prepare(readSize)), readSize)
769768
.then([=](pplx::task<size_t> op)
770769
{
771770
try
772771
{
773-
write_chunk(op.get());
772+
const auto actualReadSize = op.get();
773+
if(actualReadSize == 0)
774+
{
775+
ctx->report_exception(http_exception("Unexpected end of request body stream encountered before Content-Length satisfied."));
776+
return;
777+
}
778+
write_chunk(actualReadSize);
774779
}
775780
catch (...)
776781
{
@@ -922,12 +927,12 @@ namespace web { namespace http
922927
}
923928
ctx->complete_headers();
924929

925-
ctx->m_known_size = std::numeric_limits<size_t>::max(); // Without Content-Length header, size should be same as TCP stream - set it size_t max.
926-
ctx->m_response.headers().match(header_names::content_length, ctx->m_known_size);
930+
ctx->m_content_length = std::numeric_limits<size_t>::max(); // Without Content-Length header, size should be same as TCP stream - set it size_t max.
931+
ctx->m_response.headers().match(header_names::content_length, ctx->m_content_length);
927932

928933
// note: need to check for 'chunked' here as well, azure storage sends both
929934
// transfer-encoding:chunked and content-length:0 (although HTTP says not to)
930-
if (ctx->m_request.method() == U("HEAD") || (!needChunked && ctx->m_known_size == 0))
935+
if (ctx->m_request.method() == U("HEAD") || (!needChunked && ctx->m_content_length == 0))
931936
{
932937
// we can stop early - no body
933938
auto progress = ctx->m_request._get_impl()->_progress_handler();
@@ -950,7 +955,7 @@ namespace web { namespace http
950955
{
951956
if (!needChunked)
952957
{
953-
async_read_until_buffersize(static_cast<size_t>(std::min(ctx->m_known_size, static_cast<uint64_t>(client_config().chunksize()))),
958+
async_read_until_buffersize(static_cast<size_t>(std::min(ctx->m_content_length, static_cast<uint64_t>(client_config().chunksize()))),
954959
boost::bind(&linux_client::handle_read_content, shared_from_this(), boost::asio::placeholders::error, ctx), ctx);
955960
}
956961
else
@@ -1061,9 +1066,9 @@ namespace web { namespace http
10611066

10621067
if (ec)
10631068
{
1064-
if (ec == boost::asio::error::eof && ctx->m_known_size == std::numeric_limits<size_t>::max())
1069+
if (ec == boost::asio::error::eof && ctx->m_content_length == std::numeric_limits<size_t>::max())
10651070
{
1066-
ctx->m_known_size = ctx->m_downloaded + ctx->m_body_buf.size();
1071+
ctx->m_content_length = ctx->m_downloaded + ctx->m_body_buf.size();
10671072
}
10681073
else
10691074
{
@@ -1087,11 +1092,11 @@ namespace web { namespace http
10871092
}
10881093
}
10891094

1090-
if (ctx->m_downloaded < ctx->m_known_size)
1095+
if (ctx->m_downloaded < ctx->m_content_length)
10911096
{
10921097
// more data need to be read
10931098
writeBuffer.putn(boost::asio::buffer_cast<const uint8_t *>(ctx->m_body_buf.data()),
1094-
static_cast<size_t>(std::min(static_cast<uint64_t>(ctx->m_body_buf.size()), ctx->m_known_size - ctx->m_downloaded)))
1099+
static_cast<size_t>(std::min(static_cast<uint64_t>(ctx->m_body_buf.size()), ctx->m_content_length - ctx->m_downloaded)))
10951100
.then([=](pplx::task<size_t> op)
10961101
{
10971102
size_t writtenSize = 0;
@@ -1101,7 +1106,7 @@ namespace web { namespace http
11011106
ctx->m_downloaded += static_cast<uint64_t>(writtenSize);
11021107
ctx->m_body_buf.consume(writtenSize);
11031108

1104-
async_read_until_buffersize(static_cast<size_t>(std::min(static_cast<uint64_t>(client_config().chunksize()), ctx->m_known_size - ctx->m_downloaded)),
1109+
async_read_until_buffersize(static_cast<size_t>(std::min(static_cast<uint64_t>(client_config().chunksize()), ctx->m_content_length - ctx->m_downloaded)),
11051110
boost::bind(&linux_client::handle_read_content, shared_from_this(), boost::asio::placeholders::error, ctx), ctx);
11061111
}
11071112
catch (...)
@@ -1141,7 +1146,7 @@ namespace web { namespace http
11411146
http_request &request,
11421147
const std::shared_ptr<linux_connection> &connection)
11431148
: request_context(client, request)
1144-
, m_known_size(0)
1149+
, m_content_length(0)
11451150
, m_needChunked(false)
11461151
, m_timer(static_cast<int>(client->client_config().timeout().count()))
11471152
, m_connection(connection)

‎Release/src/http/client/http_win7.cpp

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,6 @@ class winhttp_client : public _http_client_communicator
567567
}
568568

569569
// There is a request body that needs to be transferred.
570-
571570
if (content_length == std::numeric_limits<size_t>::max())
572571
{
573572
// The content length is unknown and the application set a stream. This is an
@@ -655,7 +654,7 @@ class winhttp_client : public _http_client_communicator
655654
// Capture the current read position of the stream.
656655
auto rbuf = winhttp_context->_get_readbuffer();
657656

658-
// Record starting position incase request is challenged for authorization
657+
// Record starting position in case request is challenged for authorization
659658
// and needs to seek back to where reading is started from.
660659
winhttp_context->m_startingPosition = rbuf.getpos(std::ios_base::in);
661660

@@ -835,20 +834,16 @@ class winhttp_client : public _http_client_communicator
835834
}
836835
_ASSERTE(read != static_cast<size_t>(-1));
837836

838-
if ( read == 0 )
837+
if (read == 0)
839838
{
840-
// Unexpected end-of-stream.
841-
if (!(rbuf.exception() == nullptr))
842-
p_request_context->report_exception(rbuf.exception());
843-
else
844-
p_request_context->report_error(GetLastError(), _XPLATSTR("Error reading outgoing HTTP body from its stream."));
839+
p_request_context->report_exception(http_exception(U("Unexpected end of request body stream encountered before Content-Length met.")));
845840
return;
846841
}
847842

848843
p_request_context->m_remaining_to_write -= read;
849844

850845
// Stop writing chunks after this one if no more data.
851-
if ( p_request_context->m_remaining_to_write == 0 )
846+
if (p_request_context->m_remaining_to_write == 0)
852847
{
853848
p_request_context->m_bodyType = no_body;
854849
}

‎Release/src/http/client/http_win8.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -279,16 +279,15 @@ class IResponseStream
279279
*pcbWritten = 0;
280280
}
281281

282+
if (cb == 0)
283+
{
284+
return S_OK;
285+
}
286+
282287
try
283288
{
284289
auto buffer = context->_get_writebuffer();
285-
if ( cb == 0 )
286-
{
287-
buffer.sync().wait();
288-
return S_OK;
289-
}
290-
291-
const size_t count = buffer.putn((const uint8_t *)pv, (size_t)cb).get();
290+
const size_t count = buffer.putn(reinterpret_cast<const uint8_t *>(pv), static_cast<size_t>(cb)).get();
292291

293292
_ASSERTE(count != static_cast<size_t>(-1));
294293
_ASSERTE(count <= static_cast<size_t>(ULONG_MAX));
@@ -299,7 +298,7 @@ class IResponseStream
299298
context->m_downloaded += count;
300299

301300
auto progress = context->m_request._get_impl()->_progress_handler();
302-
if ( progress && count > 0 )
301+
if (progress && count > 0)
303302
{
304303
try { (*progress)(message_direction::download, context->m_downloaded); } catch(...)
305304
{

‎Release/tests/common/TestRunner/test_runner.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,8 @@ bool IsTestIgnored(UnitTest::Test *pTest)
381381
if(pTest->m_properties.Has("Ignore:Windows")) return true;
382382
#elif defined(__APPLE__)
383383
if(pTest->m_properties.Has("Ignore:Apple")) return true;
384+
#elif defined(ANDROID)
385+
if(pTest->m_properties.Has("Ignore:Android")) return true;
384386
#else
385387
if(pTest->m_properties.Has("Ignore:Linux")) return true;
386388
#endif

‎Release/tests/functional/http/client/request_stream_tests.cpp

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -316,28 +316,38 @@ class test_exception : public std::exception
316316
}
317317
};
318318

319-
TEST_FIXTURE(uri_address, set_body_stream_exception, "Ignore", "940704")
319+
// Ignore on WinRT CodePlex 144
320+
#if !defined(__cplusplus_winrt)
321+
TEST_FIXTURE(uri_address, set_body_stream_exception,
322+
"Ignore:Apple", "296",
323+
"Ignore:Android", "296",
324+
"Ignore:Linux", "296")
320325
{
321-
utility::string_t fname = U("set_body_stream_exception.txt");
322-
fill_file(fname);
323-
324326
test_http_server::scoped_server scoped(m_uri);
325327
scoped.server();
326328
http_client client(m_uri);
327329

328-
auto stream = OPEN_R<uint8_t>(fname).get();
330+
streams::producer_consumer_buffer<uint8_t> buf;
331+
const char *data = "abcdefghijklmnopqrstuvwxyz";
332+
buf.putn(reinterpret_cast<const uint8_t *>(data), 26).wait();
333+
329334
http_request msg(methods::POST);
330-
msg.set_body(stream);
335+
msg.set_body(buf.create_istream());
331336
msg.headers().set_content_length(26);
332337

333-
stream.close(std::ios::in, std::make_exception_ptr(test_exception()));
338+
buf.close(std::ios::in, std::make_exception_ptr(test_exception())).wait();
334339

335340
VERIFY_THROWS(client.request(msg).get(), test_exception);
336341
}
342+
#endif
337343

344+
// These tests aren't possible on WinRT because they don't
345+
// specify a Content-Length.
338346
#if !defined(__cplusplus_winrt)
339-
340-
TEST_FIXTURE(uri_address, stream_close_early)
347+
TEST_FIXTURE(uri_address, stream_close_early,
348+
"Ignore:Apple", "296",
349+
"Ignore:Android", "296",
350+
"Ignore:Linux", "296")
341351
{
342352
http_client client(m_uri);
343353
test_http_server::scoped_server scoped(m_uri);
@@ -354,16 +364,16 @@ TEST_FIXTURE(uri_address, stream_close_early)
354364
unsigned char data[5] = { '1', '2', '3', '4', '5' };
355365
buf.putn(&data[0], 5).wait();
356366

357-
buf.close(std::ios::out);
367+
buf.close(std::ios::out).wait();
358368

359-
// Verify that the task completes succesfully
369+
// Verify that the task completes successfully
360370
http_asserts::assert_response_equals(responseTask.get(), status_codes::OK);
361371
}
362372

363-
TEST_FIXTURE(uri_address, stream_close_early_with_exception,
364-
"Ignore", "825361",
365-
"Ignore:Linux", "TBD",
366-
"Ignore:Apple", "The test server has trouble closing.")
373+
TEST_FIXTURE(uri_address, stream_close_early_with_exception,
374+
"Ignore:Apple", "296",
375+
"Ignore:Android", "296",
376+
"Ignore:Linux", "296")
367377
{
368378
http_client client(m_uri);
369379
test_http_server::scoped_server scoped(m_uri);
@@ -376,17 +386,19 @@ TEST_FIXTURE(uri_address, stream_close_early)
376386
unsigned char data[5] = { '1', '2', '3', '4', '5' };
377387
buf.putn(&data[0], 5).wait();
378388

379-
buf.close(std::ios::out, std::make_exception_ptr(test_exception()));
389+
buf.close(std::ios::out, std::make_exception_ptr(test_exception())).wait();
380390

381391
// Verify that the responseTask throws the exception set when closing the stream
382392
VERIFY_THROWS(responseTask.get(), test_exception);
383393
}
384394
#endif
385395

386-
TEST_FIXTURE(uri_address, stream_close_early_with_exception_and_contentlength,
387-
"Ignore", "825361",
388-
"Ignore:Linux", "TBD",
389-
"Ignore:Apple", "The test server has trouble closing.")
396+
// Ignore on WinRT only CodePlex 144
397+
#if !defined(__cplusplus_winrt)
398+
TEST_FIXTURE(uri_address, stream_close_early_with_exception_and_contentlength,
399+
"Ignore:Apple", "296",
400+
"Ignore:Android", "296",
401+
"Ignore:Linux", "296")
390402
{
391403
http_client client(m_uri);
392404
test_http_server::scoped_server scoped(m_uri);
@@ -399,16 +411,19 @@ TEST_FIXTURE(uri_address, stream_close_early_with_exception_and_contentlength,
399411
unsigned char data[5] = { '1', '2', '3', '4', '5' };
400412
buf.putn(&data[0], 5).wait();
401413

402-
buf.close(std::ios::out, std::make_exception_ptr(test_exception()));
414+
buf.close(std::ios::out, std::make_exception_ptr(test_exception())).wait();
403415

404416
// Verify that the responseTask throws the exception set when closing the stream
405417
VERIFY_THROWS(responseTask.get(), test_exception);
406418
}
419+
#endif
407420

421+
// Ignore on WinRT only CodePlex 144
422+
#if !defined(__cplusplus_winrt)
408423
TEST_FIXTURE(uri_address, stream_close_early_with_contentlength,
409-
"Ignore", "825361",
410-
"Ignore:Linux", "TBD",
411-
"Ignore:Apple", "The test server has trouble closing.")
424+
"Ignore:Apple", "296",
425+
"Ignore:Android", "296",
426+
"Ignore:Linux", "296")
412427
{
413428
http_client client(m_uri);
414429
test_http_server::scoped_server scoped(m_uri);
@@ -421,11 +436,12 @@ TEST_FIXTURE(uri_address, stream_close_early_with_contentlength,
421436
unsigned char data[5] = { '1', '2', '3', '4', '5' };
422437
buf.putn(&data[0], 5).wait();
423438

424-
buf.close(std::ios::out);
439+
buf.close(std::ios::out).wait();
425440

426441
// Verify that the responseTask throws the exception set when closing the stream
427442
VERIFY_THROWS(responseTask.get(), http_exception);
428443
}
444+
#endif
429445

430446
TEST_FIXTURE(uri_address, get_with_body_nono)
431447
{

0 commit comments

Comments
 (0)