Skip to content

Commit 9d96d36

Browse files
committed
RFC7230 (HTTP 1.1) 3.2 "Header fields" specifies that whitespace between colon and value is optional, but http_client_asio::read_headers started reading the value at one after the colon, assuming there would always be exactly one space.
1 parent 3070ca2 commit 9d96d36

File tree

1 file changed

+50
-48
lines changed

1 file changed

+50
-48
lines changed

Release/src/http/client/http_client_asio.cpp

Lines changed: 50 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ class asio_client : public _http_client_communicator, public std::enable_shared_
339339
, m_pool(crossplat::threadpool::shared_instance().service(),
340340
base_uri().scheme() == "https" && !_http_client_communicator::client_config().proxy().is_specified(),
341341
std::chrono::seconds(30), // Unused sockets are kept in pool for 30 seconds.
342-
this->client_config().get_ssl_context_callback())
342+
this->client_config().get_ssl_context_callback())
343343
, m_resolver(crossplat::threadpool::shared_instance().service())
344344
{}
345345

@@ -383,14 +383,14 @@ class asio_context : public request_context, public std::enable_shared_from_this
383383
ctx->m_timer.set_ctx(std::weak_ptr<asio_context>(ctx));
384384
return ctx;
385385
}
386-
386+
387387
class ssl_proxy_tunnel : public std::enable_shared_from_this<ssl_proxy_tunnel>
388388
{
389389
public:
390390
ssl_proxy_tunnel(std::shared_ptr<asio_context> context, std::function<void(std::shared_ptr<asio_context>)> ssl_tunnel_established)
391391
: m_ssl_tunnel_established(ssl_tunnel_established), m_context(context)
392392
{}
393-
393+
394394
void start_proxy_connect()
395395
{
396396
auto proxy = m_context->m_http_client->client_config().proxy();
@@ -424,7 +424,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
424424
client->m_resolver.async_resolve(query, boost::bind(&ssl_proxy_tunnel::handle_resolve, shared_from_this(), boost::asio::placeholders::error, boost::asio::placeholders::iterator));
425425
}
426426

427-
private:
427+
private:
428428
void handle_resolve(const boost::system::error_code& ec, tcp::resolver::iterator endpoints)
429429
{
430430
if (ec)
@@ -475,7 +475,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
475475
m_context->report_error("Failed to send connect request to proxy.", err, httpclient_errorcode_context::writebody);
476476
}
477477
}
478-
478+
479479
void handle_status_line(const boost::system::error_code& ec)
480480
{
481481
if (!ec)
@@ -487,23 +487,23 @@ class asio_context : public request_context, public std::enable_shared_from_this
487487
response_stream >> http_version;
488488
status_code status_code;
489489
response_stream >> status_code;
490-
490+
491491
if (!response_stream || http_version.substr(0, 5) != "HTTP/")
492492
{
493493
m_context->report_error("Invalid HTTP status line during proxy connection", ec, httpclient_errorcode_context::readheader);
494494
return;
495495
}
496-
496+
497497
if (status_code != 200)
498498
{
499499
utility::stringstream_t err_ss;
500500
err_ss << U("Expected a 200 response from proxy, received: ") << status_code;
501501
m_context->report_error(err_ss.str(), ec, httpclient_errorcode_context::readheader);
502502
return;
503503
}
504-
504+
505505
m_context->m_connection->upgrade_to_ssl();
506-
506+
507507
m_ssl_tunnel_established(m_context);
508508
}
509509
else
@@ -517,14 +517,14 @@ class asio_context : public request_context, public std::enable_shared_from_this
517517
// Failed to write to socket because connection was already closed while it was in the pool.
518518
// close() here ensures socket is closed in a robust way and prevents the connection from being put to the pool again.
519519
m_context->m_connection->close();
520-
520+
521521
// Create a new context and copy the request object, completion event and
522522
// cancellation registration to maintain the old state.
523523
// This also obtains a new connection from pool.
524524
auto new_ctx = m_context->create_request_context(m_context->m_http_client, m_context->m_request);
525525
new_ctx->m_request_completion = m_context->m_request_completion;
526526
new_ctx->m_cancellationRegistration = m_context->m_cancellationRegistration;
527-
527+
528528
auto client = std::static_pointer_cast<asio_client>(m_context->m_http_client);
529529
// Resend the request using the new context.
530530
client->send_request(new_ctx);
@@ -535,15 +535,15 @@ class asio_context : public request_context, public std::enable_shared_from_this
535535
}
536536
}
537537
}
538-
538+
539539
std::function<void(std::shared_ptr<asio_context>)> m_ssl_tunnel_established;
540540
std::shared_ptr<asio_context> m_context;
541-
541+
542542
boost::asio::streambuf m_request;
543543
boost::asio::streambuf m_response;
544544
};
545-
546-
545+
546+
547547
enum class http_proxy_type
548548
{
549549
none,
@@ -558,11 +558,11 @@ class asio_context : public request_context, public std::enable_shared_from_this
558558
request_context::report_error(make_error_code(std::errc::operation_canceled).value(), "Request canceled by user.");
559559
return;
560560
}
561-
561+
562562
http_proxy_type proxy_type = http_proxy_type::none;
563563
utility::string_t proxy_host;
564564
int proxy_port = -1;
565-
565+
566566
// There is no support for auto-detection of proxies on non-windows platforms, it must be specified explicitly from the client code.
567567
if (m_http_client->client_config().proxy().is_specified())
568568
{
@@ -572,28 +572,28 @@ class asio_context : public request_context, public std::enable_shared_from_this
572572
proxy_port = proxy_uri.port() == -1 ? 8080 : proxy_uri.port();
573573
proxy_host = proxy_uri.host();
574574
}
575-
575+
576576
auto start_http_request_flow = [proxy_type, proxy_host, proxy_port](std::shared_ptr<asio_context> ctx)
577577
{
578578
if (ctx->m_request._cancellation_token().is_canceled())
579579
{
580580
ctx->request_context::report_error(make_error_code(std::errc::operation_canceled).value(), "Request canceled by user.");
581581
return;
582582
}
583-
583+
584584
const auto &base_uri = ctx->m_http_client->base_uri();
585585
const auto full_uri = uri_builder(base_uri).append(ctx->m_request.relative_uri()).to_uri();
586-
586+
587587
// For a normal http proxy, we need to specify the full request uri, otherwise just specify the resource
588588
auto encoded_resource = proxy_type == http_proxy_type::http ? full_uri.to_string() : full_uri.resource().to_string();
589-
589+
590590
if (encoded_resource == "")
591591
{
592592
encoded_resource = "/";
593593
}
594-
594+
595595
const auto &method = ctx->m_request.method();
596-
596+
597597
// stop injection of headers via method
598598
// resource should be ok, since it's been encoded
599599
// and host won't resolve
@@ -602,35 +602,35 @@ class asio_context : public request_context, public std::enable_shared_from_this
602602
ctx->report_exception(http_exception("The method string is invalid."));
603603
return;
604604
}
605-
605+
606606
std::ostream request_stream(&ctx->m_body_buf);
607607
request_stream.imbue(std::locale::classic());
608608
const auto &host = base_uri.host();
609-
609+
610610
request_stream << method << " " << encoded_resource << " " << "HTTP/1.1" << CRLF;
611-
611+
612612
int port = base_uri.port();
613-
613+
614614
if (base_uri.is_port_default())
615615
{
616616
port = (ctx->m_connection->is_ssl() ? 443 : 80);
617617
}
618-
618+
619619
// Add the Host header if user has not specified it explicitly
620620
if (!ctx->m_request.headers().has(header_names::host))
621621
{
622622
request_stream << "Host: " << host << ":" << port << CRLF;
623623
}
624-
624+
625625
// Extra request headers are constructed here.
626626
utility::string_t extra_headers;
627-
627+
628628
// Add header for basic proxy authentication
629629
if (proxy_type == http_proxy_type::http && ctx->m_http_client->client_config().proxy().credentials().is_set())
630630
{
631631
extra_headers.append(ctx->generate_basic_proxy_auth_header());
632632
}
633-
633+
634634
// Check user specified transfer-encoding.
635635
std::string transferencoding;
636636
if (ctx->m_request.headers().match(header_names::transfer_encoding, transferencoding) && transferencoding == "chunked")
@@ -647,26 +647,26 @@ class asio_context : public request_context, public std::enable_shared_from_this
647647
extra_headers.append(":chunked" + CRLF);
648648
}
649649
}
650-
650+
651651
if (proxy_type == http_proxy_type::http)
652652
{
653653
extra_headers.append(header_names::cache_control);
654654
extra_headers.append(": no-store, no-cache" + CRLF);
655655
extra_headers.append(header_names::pragma);
656656
extra_headers.append(": no-cache" + CRLF);
657657
}
658-
658+
659659
request_stream << flatten_http_headers(ctx->m_request.headers());
660660
request_stream << extra_headers;
661661
// Enforce HTTP connection keep alive (even for the old HTTP/1.0 protocol).
662662
request_stream << "Connection: Keep-Alive" << CRLF << CRLF;
663-
663+
664664
// Start connection timeout timer.
665665
if (!ctx->m_timer.has_started())
666666
{
667667
ctx->m_timer.start();
668668
}
669-
669+
670670
if (ctx->m_connection->is_reused() || proxy_type == http_proxy_type::ssl_tunnel)
671671
{
672672
// If socket is a reused connection or we're connected via an ssl-tunneling proxy, try to write the request directly. In both cases we have already established a tcp connection.
@@ -676,16 +676,16 @@ class asio_context : public request_context, public std::enable_shared_from_this
676676
{
677677
// If the connection is new (unresolved and unconnected socket), then start async
678678
// call to resolve first, leading eventually to request write.
679-
679+
680680
// For normal http proxies, we want to connect directly to the proxy server. It will relay our request.
681681
auto tcp_host = proxy_type == http_proxy_type::http ? proxy_host : host;
682682
auto tcp_port = proxy_type == http_proxy_type::http ? proxy_port : port;
683-
683+
684684
tcp::resolver::query query(tcp_host, utility::conversions::print_string(tcp_port, std::locale::classic()));
685685
auto client = std::static_pointer_cast<asio_client>(ctx->m_http_client);
686686
client->m_resolver.async_resolve(query, boost::bind(&asio_context::handle_resolve, ctx, boost::asio::placeholders::error, boost::asio::placeholders::iterator));
687687
}
688-
688+
689689
// Register for notification on cancellation to abort this request.
690690
if (ctx->m_request._cancellation_token() != pplx::cancellation_token::none())
691691
{
@@ -702,7 +702,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
702702
});
703703
}
704704
};
705-
705+
706706
if (proxy_type == http_proxy_type::ssl_tunnel)
707707
{
708708
// The ssl_tunnel_proxy keeps the context alive and then calls back once the ssl tunnel is established via 'start_http_request_flow'
@@ -733,16 +733,16 @@ class asio_context : public request_context, public std::enable_shared_from_this
733733
utility::string_t generate_basic_proxy_auth_header()
734734
{
735735
utility::string_t header;
736-
736+
737737
header.append(header_names::proxy_authorization);
738738
header.append(": Basic ");
739-
739+
740740
auto credential_str = web::details::plaintext_string(new ::utility::string_t(m_http_client->client_config().proxy().credentials().username()));
741741
credential_str->append(":");
742742
credential_str->append(*m_http_client->client_config().proxy().credentials().decrypt());
743-
743+
744744
std::vector<unsigned char> credentials_buffer(credential_str->begin(), credential_str->end());
745-
745+
746746
header.append(utility::conversions::to_base64(credentials_buffer));
747747
header.append(CRLF);
748748
return header;
@@ -790,7 +790,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
790790

791791
void handle_connect(const boost::system::error_code& ec, tcp::resolver::iterator endpoints)
792792
{
793-
793+
794794
m_timer.reset();
795795
if (!ec)
796796
{
@@ -983,7 +983,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
983983
// Reuse error handling.
984984
return handle_write_body(ec);
985985
}
986-
986+
987987
m_timer.reset();
988988
const auto &progress = m_request._get_impl()->_progress_handler();
989989
if (progress)
@@ -1064,7 +1064,7 @@ class asio_context : public request_context, public std::enable_shared_from_this
10641064
response_stream >> http_version;
10651065
status_code status_code;
10661066
response_stream >> status_code;
1067-
1067+
10681068
std::string status_message;
10691069
std::getline(response_stream, status_message);
10701070

@@ -1123,7 +1123,9 @@ class asio_context : public request_context, public std::enable_shared_from_this
11231123
if (colon != std::string::npos)
11241124
{
11251125
auto name = header.substr(0, colon);
1126-
auto value = header.substr(colon + 2, header.size() - (colon + 3)); // also exclude '\r'
1126+
1127+
//read value from immediately after colon, and exclude \r.
1128+
auto value = header.substr(colon + 1, header.size() - colon - 2));
11271129
boost::algorithm::trim(name);
11281130
boost::algorithm::trim(value);
11291131

@@ -1474,7 +1476,7 @@ void asio_client::send_request(const std::shared_ptr<request_context> &request_c
14741476
{
14751477
client_config().invoke_nativehandle_options(ctx->m_connection->m_ssl_stream.get());
14761478
}
1477-
else
1479+
else
14781480
{
14791481
client_config().invoke_nativehandle_options(&(ctx->m_connection->m_socket));
14801482
}

0 commit comments

Comments
 (0)