Skip to content

Commit ffa8ffa

Browse files
stgatesstgates
committed
Misc improvements to Boost http_client:
1. Removed unnecessary check on request input stream. 2. Added some comments to help better understand what is going on. 3. Replaced a few C style casts. 4. Removed unnecessary size_t variable on each request context.
1 parent d431693 commit ffa8ffa

File tree

1 file changed

+55
-93
lines changed

1 file changed

+55
-93
lines changed

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

Lines changed: 55 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,7 @@ namespace web { namespace http
274274
}
275275

276276
std::unique_ptr<boost::asio::ssl::stream<tcp::socket &> > m_ssl_stream;
277-
size_t m_known_size;
278-
size_t m_current_size;
277+
uint64_t m_known_size;
279278
bool m_needChunked;
280279
bool m_timedout;
281280
boost::asio::streambuf m_body_buf;
@@ -377,34 +376,16 @@ namespace web { namespace http
377376
if (ctx->m_request.headers().match(header_names::transfer_encoding, transferencoding) && transferencoding == "chunked")
378377
{
379378
ctx->m_needChunked = true;
380-
}
381-
382-
bool has_body;
383-
384-
if (ctx->m_request.headers().match(header_names::content_length, ctx->m_known_size))
385-
{
386-
// Have request body if content length header field is non-zero.
387-
has_body = (0 != ctx->m_known_size);
388-
}
389-
else
379+
}
380+
else if (!ctx->m_request.headers().match(header_names::content_length, ctx->m_known_size))
390381
{
391-
// Stream without content length is the signal of requiring transcoding.
382+
// Stream without content length is the signal of requiring transfer encoding chunked.
392383
if (ctx->m_request.body())
393384
{
394-
has_body = true;
395385
ctx->m_needChunked = true;
396386
extra_headers.append(header_names::transfer_encoding);
397387
extra_headers.append(":chunked" + CRLF);
398388
}
399-
else
400-
{
401-
has_body = false;
402-
}
403-
}
404-
405-
if (has_body && !_check_streambuf(ctx, ctx->_get_readbuffer(), "Input stream is not open"))
406-
{
407-
return;
408389
}
409390

410391
request_stream << flatten_http_headers(ctx->m_request.headers());
@@ -424,7 +405,6 @@ namespace web { namespace http
424405
// If the connection is new (unresolved and unconnected socket), then start async
425406
// call to resolve first, leading eventually to request write.
426407
tcp::resolver::query query(host, utility::conversions::print_string(port));
427-
428408
m_resolver.async_resolve(query, boost::bind(&linux_client::handle_resolve, shared_from_this(), boost::asio::placeholders::error, boost::asio::placeholders::iterator, ctx));
429409
}
430410

@@ -455,26 +435,9 @@ namespace web { namespace http
455435
#if defined(__APPLE__) || defined(ANDROID)
456436
bool m_openssl_failed;
457437
#endif
458-
459-
static bool _check_streambuf(std::shared_ptr<linux_client_request_context> ctx, concurrency::streams::streambuf<uint8_t> rdbuf, const utility::char_t* msg)
460-
{
461-
if (!rdbuf.is_open())
462-
{
463-
auto eptr = rdbuf.exception();
464-
if (!(eptr == nullptr))
465-
{
466-
ctx->report_exception(eptr);
467-
}
468-
else
469-
{
470-
ctx->report_exception(http_exception(msg));
471-
}
472-
}
473-
return rdbuf.is_open();
474-
}
475438

476439
// Helper function to create ssl stream and set verification options.
477-
void reset_ssl_stream(std::shared_ptr<linux_client_request_context> &ctx)
440+
void reset_ssl_stream(const std::shared_ptr<linux_client_request_context> &ctx)
478441
{
479442
boost::asio::ssl::context sslContext(boost::asio::ssl::context::sslv23);
480443
sslContext.set_default_verify_paths();
@@ -509,15 +472,15 @@ namespace web { namespace http
509472
}
510473
}
511474

512-
void write_request(std::shared_ptr<linux_client_request_context> ctx)
475+
void write_request(std::shared_ptr<linux_client_request_context> &ctx)
513476
{
514477
if (ctx->m_ssl_stream)
515478
{
516479
ctx->m_ssl_stream->async_handshake(boost::asio::ssl::stream_base::client, boost::bind(&linux_client::handle_handshake, shared_from_this(), boost::asio::placeholders::error, ctx));
517480
}
518481
else
519482
{
520-
boost::asio::async_write(ctx->m_connection->socket(), ctx->m_body_buf, boost::bind(&linux_client::handle_write_request, shared_from_this(), boost::asio::placeholders::error, ctx));
483+
boost::asio::async_write(ctx->m_connection->socket(), ctx->m_body_buf, boost::bind(&linux_client::handle_write_headers, shared_from_this(), boost::asio::placeholders::error, ctx));
521484
}
522485
}
523486

@@ -617,7 +580,7 @@ namespace web { namespace http
617580
{
618581
if (!ec)
619582
{
620-
boost::asio::async_write(*ctx->m_ssl_stream, ctx->m_body_buf, boost::bind(&linux_client::handle_write_request, shared_from_this(), boost::asio::placeholders::error, ctx));
583+
boost::asio::async_write(*ctx->m_ssl_stream, ctx->m_body_buf, boost::bind(&linux_client::handle_write_headers, shared_from_this(), boost::asio::placeholders::error, ctx));
621584
}
622585
else
623586
{
@@ -629,6 +592,7 @@ namespace web { namespace http
629592
{
630593
if (ec)
631594
{
595+
// Reuse error handling.
632596
return handle_write_body(ec, ctx);
633597
}
634598

@@ -646,9 +610,10 @@ namespace web { namespace http
646610
}
647611
}
648612

613+
const auto & chunkSize = client_config().chunksize();
649614
auto readbuf = ctx->_get_readbuffer();
650-
uint8_t *buf = boost::asio::buffer_cast<uint8_t *>(ctx->m_body_buf.prepare(client_config().chunksize() + http::details::chunked_encoding::additional_encoding_space));
651-
readbuf.getn(buf + http::details::chunked_encoding::data_offset, client_config().chunksize())
615+
uint8_t *buf = boost::asio::buffer_cast<uint8_t *>(ctx->m_body_buf.prepare(chunkSize + http::details::chunked_encoding::additional_encoding_space));
616+
readbuf.getn(buf + http::details::chunked_encoding::data_offset, chunkSize)
652617
.then([=](pplx::task<size_t> op)
653618
{
654619
size_t readSize = 0;
@@ -661,11 +626,11 @@ namespace web { namespace http
661626
ctx->report_exception(std::current_exception());
662627
return;
663628
}
664-
const size_t offset = http::details::chunked_encoding::add_chunked_delimiters(buf, client_config().chunksize() + http::details::chunked_encoding::additional_encoding_space, readSize);
629+
630+
const size_t offset = http::details::chunked_encoding::add_chunked_delimiters(buf, chunkSize + http::details::chunked_encoding::additional_encoding_space, readSize);
665631
ctx->m_body_buf.commit(readSize + http::details::chunked_encoding::additional_encoding_space);
666632
ctx->m_body_buf.consume(offset);
667-
ctx->m_current_size += readSize;
668-
ctx->m_uploaded += (size64_t)readSize;
633+
ctx->m_uploaded += static_cast<uint64_t>(readSize);
669634
if (ctx->m_ssl_stream)
670635
{
671636
if (readSize != 0)
@@ -697,8 +662,9 @@ namespace web { namespace http
697662

698663
void handle_write_large_body(const boost::system::error_code& ec, std::shared_ptr<linux_client_request_context> ctx)
699664
{
700-
if (ec || ctx->m_current_size >= ctx->m_known_size)
665+
if (ec || ctx->m_uploaded >= ctx->m_known_size)
701666
{
667+
// Reuse error handling.
702668
return handle_write_body(ec, ctx);
703669
}
704670

@@ -716,25 +682,10 @@ namespace web { namespace http
716682
}
717683
}
718684

719-
auto readbuf = ctx->_get_readbuffer();
720-
const size_t readSize = std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_current_size);
721-
722-
readbuf.getn(boost::asio::buffer_cast<uint8_t *>(ctx->m_body_buf.prepare(readSize)), readSize)
723-
.then([=](pplx::task<size_t> op)
685+
auto write_chunk = [=](size_t chunkSize)
724686
{
725-
size_t actualSize = 0;
726-
try
727-
{
728-
actualSize = op.get();
729-
}
730-
catch (...)
731-
{
732-
ctx->report_exception(std::current_exception());
733-
return;
734-
}
735-
ctx->m_uploaded += (size64_t)actualSize;
736-
ctx->m_current_size += actualSize;
737-
ctx->m_body_buf.commit(actualSize);
687+
ctx->m_uploaded += static_cast<uint64_t>(chunkSize);
688+
ctx->m_body_buf.commit(chunkSize);
738689

739690
if (ctx->m_ssl_stream)
740691
{
@@ -746,15 +697,34 @@ namespace web { namespace http
746697
boost::asio::async_write(ctx->m_connection->socket(), ctx->m_body_buf,
747698
boost::bind(&linux_client::handle_write_large_body, shared_from_this(), boost::asio::placeholders::error, ctx));
748699
}
700+
};
701+
702+
const size_t readSize = std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_uploaded);
703+
704+
auto readbuf = ctx->_get_readbuffer();
705+
readbuf.getn(boost::asio::buffer_cast<uint8_t *>(ctx->m_body_buf.prepare(readSize)), readSize)
706+
.then([=](pplx::task<size_t> op)
707+
{
708+
try
709+
{
710+
write_chunk(op.get());
711+
}
712+
catch (...)
713+
{
714+
ctx->report_exception(std::current_exception());
715+
return;
716+
}
749717
});
750718
}
751719

752-
void handle_write_request(const boost::system::error_code& ec, std::shared_ptr<linux_client_request_context> ctx)
720+
void handle_write_headers(const boost::system::error_code& ec, std::shared_ptr<linux_client_request_context> ctx)
753721
{
754-
if (!ec)
722+
if(ec)
723+
{
724+
ctx->report_error("Failed to write request headers", ec, httpclient_errorcode_context::writeheader);
725+
}
726+
else
755727
{
756-
ctx->m_current_size = 0;
757-
758728
if (ctx->m_needChunked)
759729
{
760730
handle_write_chunked_body(ec, ctx);
@@ -764,10 +734,6 @@ namespace web { namespace http
764734
handle_write_large_body(ec, ctx);
765735
}
766736
}
767-
else
768-
{
769-
ctx->report_error("Failed to write request headers", ec, httpclient_errorcode_context::writeheader);
770-
}
771737
}
772738

773739
void handle_write_body(const boost::system::error_code& ec, std::shared_ptr<linux_client_request_context> ctx)
@@ -866,7 +832,7 @@ namespace web { namespace http
866832

867833
void read_headers(const std::shared_ptr<linux_client_request_context> &ctx)
868834
{
869-
ctx->m_needChunked = false;
835+
auto needChunked = false;
870836
std::istream response_stream(&ctx->m_body_buf);
871837
std::string header;
872838
while (std::getline(response_stream, header) && header != "\r")
@@ -881,7 +847,7 @@ namespace web { namespace http
881847

882848
if (boost::iequals(name, header_names::transfer_encoding))
883849
{
884-
ctx->m_needChunked = boost::iequals(value, U("chunked"));
850+
needChunked = boost::iequals(value, U("chunked"));
885851
}
886852

887853
if (boost::iequals(name, header_names::connection))
@@ -903,7 +869,7 @@ namespace web { namespace http
903869

904870
// note: need to check for 'chunked' here as well, azure storage sends both
905871
// transfer-encoding:chunked and content-length:0 (although HTTP says not to)
906-
if (ctx->m_request.method() == U("HEAD") || (!ctx->m_needChunked && ctx->m_known_size == 0))
872+
if (ctx->m_request.method() == U("HEAD") || (!needChunked && ctx->m_known_size == 0))
907873
{
908874
// we can stop early - no body
909875
auto progress = ctx->m_request._get_impl()->_progress_handler();
@@ -924,8 +890,7 @@ namespace web { namespace http
924890
}
925891
else
926892
{
927-
ctx->m_current_size = 0;
928-
if (!ctx->m_needChunked)
893+
if (!needChunked)
929894
{
930895
async_read_until_buffersize(std::min(ctx->m_known_size, client_config().chunksize()),
931896
boost::bind(&linux_client::handle_read_content, shared_from_this(), boost::asio::placeholders::error, ctx), ctx);
@@ -997,8 +962,7 @@ namespace web { namespace http
997962
{
998963
if (!ec)
999964
{
1000-
ctx->m_current_size += to_read;
1001-
ctx->m_downloaded += (size64_t)to_read;
965+
ctx->m_downloaded += static_cast<uint64_t>(to_read);
1002966
auto progress = ctx->m_request._get_impl()->_progress_handler();
1003967
if (progress)
1004968
{
@@ -1022,7 +986,7 @@ namespace web { namespace http
1022986
try
1023987
{
1024988
op.wait();
1025-
ctx->complete_request(ctx->m_current_size);
989+
ctx->complete_request(ctx->m_downloaded);
1026990
}
1027991
catch (...)
1028992
{
@@ -1076,7 +1040,7 @@ namespace web { namespace http
10761040
{
10771041
if (ec == boost::asio::error::eof && ctx->m_known_size == std::numeric_limits<size_t>::max())
10781042
{
1079-
ctx->m_known_size = ctx->m_current_size + ctx->m_body_buf.size();
1043+
ctx->m_known_size = ctx->m_downloaded + ctx->m_body_buf.size();
10801044
}
10811045
else
10821046
{
@@ -1099,24 +1063,23 @@ namespace web { namespace http
10991063
}
11001064
}
11011065

1102-
if (ctx->m_current_size < ctx->m_known_size)
1066+
if (ctx->m_downloaded < ctx->m_known_size)
11031067
{
11041068
ctx->reset_timer(static_cast<int>(client_config().timeout().count()));
11051069

11061070
// more data need to be read
11071071
writeBuffer.putn(boost::asio::buffer_cast<const uint8_t *>(ctx->m_body_buf.data()),
1108-
std::min(ctx->m_body_buf.size(), ctx->m_known_size - ctx->m_current_size))
1072+
std::min(ctx->m_body_buf.size(), ctx->m_known_size - ctx->m_downloaded))
11091073
.then([=](pplx::task<size_t> op)
11101074
{
11111075
size_t writtenSize = 0;
11121076
try
11131077
{
11141078
writtenSize = op.get();
1115-
ctx->m_downloaded += (size64_t)writtenSize;
1116-
ctx->m_current_size += writtenSize;
1079+
ctx->m_downloaded += static_cast<uint64_t>(writtenSize);
11171080
ctx->m_body_buf.consume(writtenSize);
11181081

1119-
async_read_until_buffersize(std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_current_size),
1082+
async_read_until_buffersize(std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_downloaded),
11201083
boost::bind(&linux_client::handle_read_content, shared_from_this(), boost::asio::placeholders::error, ctx), ctx);
11211084
}
11221085
catch (...)
@@ -1133,7 +1096,7 @@ namespace web { namespace http
11331096
try
11341097
{
11351098
op.wait();
1136-
ctx->complete_request(ctx->m_current_size);
1099+
ctx->complete_request(ctx->m_downloaded);
11371100
}
11381101
catch (...)
11391102
{
@@ -1167,7 +1130,6 @@ namespace web { namespace http
11671130
const std::shared_ptr<linux_connection> &connection)
11681131
: request_context(client, request)
11691132
, m_known_size(0)
1170-
, m_current_size(0)
11711133
, m_needChunked(false)
11721134
, m_timedout(false)
11731135
, m_timeout_timer(crossplat::threadpool::shared_instance().service())

0 commit comments

Comments
 (0)