Skip to content

Commit 75ad3e1

Browse files
author
Ognjen Sobajic
committed
handshake_fail test enabled TFS 747982
1 parent 94a438c commit 75ad3e1

File tree

9 files changed

+100
-72
lines changed

9 files changed

+100
-72
lines changed

Release/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ release: MODE = Release$(shell getconf LONG_BIT)
1616
release: OUTPUT_DIR = $(PWD)/../Binaries/$(MODE)
1717
release: mk_out_dir tests samples
1818

19-
debug: OPTIMIZATION_LEVEL = -O0 -g -gdwarf-2
19+
debug: OPTIMIZATION_LEVEL = -O0 -g -ggdb
2020
debug: MODE = Debug$(shell getconf LONG_BIT)
2121
debug: OUTPUT_DIR = $(PWD)/../Binaries/$(MODE)
2222
debug: mk_out_dir tests samples

Release/include/cpprest/http_client_impl.h

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -62,41 +62,9 @@ using namespace web::http::details;
6262

6363
namespace web { namespace http { namespace client { namespace details
6464
{
65-
const bool valid_chars [] =
66-
{
67-
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0-15
68-
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, //16-31
69-
0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, //32-47
70-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, //48-63
71-
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, //64-79
72-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, //80-95
73-
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, //96-111
74-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0 //112-127
75-
};
76-
7765
#ifdef _MS_WINDOWS
7866
static const utility::char_t * get_with_body = _XPLATSTR("A GET or HEAD request should not have an entity body.");
7967
#endif
80-
81-
#if (!defined(_MS_WINDOWS) || defined(__cplusplus_winrt))
82-
// Checks if the method contains any invalid characters
83-
static bool validate_method(const utility::string_t& method)
84-
{
85-
for (auto iter = method.begin(); iter != method.end(); iter++)
86-
{
87-
char_t ch = *iter;
88-
89-
if (size_t(ch) >= 128)
90-
return false;
91-
92-
if (!valid_chars[ch])
93-
return false;
94-
}
95-
96-
return true;
97-
}
98-
#endif
99-
10068
// Helper function to trim leading and trailing null characters from a string.
10169
static void trim_nulls(utility::string_t &str)
10270
{

Release/include/cpprest/http_helpers.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ namespace details
120120
void ltrim_whitespace(utility::string_t &str);
121121
void rtrim_whitespace(utility::string_t &str);
122122
void trim_whitespace(utility::string_t &str);
123+
124+
bool validate_method(const utility::string_t& method);
125+
123126

124127
namespace chunked_encoding
125128
{

Release/include/cpprest/http_linux_server.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class connection
9696
void handle_chunked_body(const boost::system::error_code& ec, int toWrite);
9797
void dispatch_request_to_listener();
9898
void request_data_avail(size_t size);
99-
void do_response();
99+
void do_response(bool bad_reqiest=false);
100100
template <typename ReadHandler>
101101
void async_read_until_buffersize(size_t size, ReadHandler handler);
102102
void async_process_response(http_response response);

Release/src/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ PKGCONFIG_LIBS = $(shell pkg-config libxml++-2.6 --libs) $(shell pkg-config libs
7575
#-fpch-deps -MMD not useful with a single monolithic PCH
7676

7777
CXXFLAGS = -fPIC -std=c++11 $(STRICT_BASE_CXXFLAGS) -I../include -I./pch $(WARNINGS) $(PKGCONFIG_CFLAGS)
78-
LIBS = $(PKGCONFIG_LIBS) -lboost_system -lboost_thread -lboost_locale -pthread -lstdc++ -lm # these are explicit for clang
78+
LIBS = $(PKGCONFIG_LIBS) -lboost_system -lboost_thread -lboost_locale -lboost_regex -pthread -lstdc++ -lm # these are explicit for clang
7979
LDFLAGS = $(BASE_LDFLAGS)
8080

8181
ifeq ($(OUTPUT_DIR),)

Release/src/http/client/http_linux.cpp

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,17 @@ namespace web { namespace http
4141
class linux_client;
4242
struct client;
4343

44-
enum class httpclient_errorcode_context
45-
{
46-
none = 0,
47-
connect,
48-
writeheader,
49-
writebody,
50-
readheader,
51-
readbody,
52-
close
53-
};
44+
enum class httpclient_errorcode_context
45+
{
46+
none = 0,
47+
connect,
48+
handshake,
49+
writeheader,
50+
writebody,
51+
readheader,
52+
readbody,
53+
close
54+
};
5455
class linux_request_context : public request_context
5556
{
5657
public:
@@ -61,28 +62,28 @@ namespace web { namespace http
6162

6263
void report_error(const utility::string_t &message, boost::system::error_code ec, httpclient_errorcode_context context = httpclient_errorcode_context::none)
6364
{
64-
// By default, errorcodeValue don't need to converted
65-
long errorcodeValue = ec.value();
66-
67-
// map timer cancellation to time_out
65+
// By default, errorcodeValue don't need to converted
66+
long errorcodeValue = ec.value();
67+
68+
// map timer cancellation to time_out
6869
if (ec == boost::system::errc::operation_canceled && m_timedout)
6970
errorcodeValue = make_error_code(std::errc::timed_out).value();
7071
else
71-
{
72-
// We need to correct inaccurate ASIO error code base on context information
73-
switch (context)
74-
{
75-
case httpclient_errorcode_context::connect:
76-
if (ec == boost::system::errc::connection_refused)
77-
errorcodeValue = make_error_code(std::errc::host_unreachable).value();
78-
break;
79-
case httpclient_errorcode_context::readheader:
80-
if (ec.default_error_condition().value() == boost::system::errc::no_such_file_or_directory) // bug in boost error_code mapping
81-
errorcodeValue = make_error_code(std::errc::connection_aborted).value();
82-
break;
83-
}
84-
}
85-
request_context::report_error(errorcodeValue, message);
72+
{
73+
// We need to correct inaccurate ASIO error code base on context information
74+
switch (context)
75+
{
76+
case httpclient_errorcode_context::connect:
77+
if (ec == boost::system::errc::connection_refused)
78+
errorcodeValue = make_error_code(std::errc::host_unreachable).value();
79+
break;
80+
case httpclient_errorcode_context::readheader:
81+
if (ec.default_error_condition().value() == boost::system::errc::no_such_file_or_directory) // bug in boost error_code mapping
82+
errorcodeValue = make_error_code(std::errc::connection_aborted).value();
83+
break;
84+
}
85+
}
86+
request_context::report_error(errorcodeValue, message);
8687
}
8788

8889
std::unique_ptr<tcp::socket> m_socket;
@@ -363,7 +364,7 @@ namespace web { namespace http
363364
if (!ec)
364365
boost::asio::async_write(*ctx->m_ssl_stream, ctx->m_request_buf, boost::bind(&client::handle_write_request, this, boost::asio::placeholders::error, ctx));
365366
else
366-
ctx->report_error("Error code in handle_handshake is ", ec);
367+
ctx->report_error("Error code in handle_handshake is ", ec, httpclient_errorcode_context::handshake);
367368
}
368369

369370
void handle_write_chunked_body(const boost::system::error_code& ec, linux_request_context* ctx)

Release/src/http/common/http_helpers.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,37 @@ size_t chunked_encoding::add_chunked_delimiters(_Out_writes_ (buffer_size) uint8
542542
return offset;
543543
}
544544

545+
#if (!defined(_MS_WINDOWS) || defined(__cplusplus_winrt))
546+
const bool valid_chars [] =
547+
{
548+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0-15
549+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, //16-31
550+
0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, //32-47
551+
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, //48-63
552+
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, //64-79
553+
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, //80-95
554+
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, //96-111
555+
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0 //112-127
556+
};
557+
558+
// Checks if the method contains any invalid characters
559+
bool validate_method(const utility::string_t& method)
560+
{
561+
for (auto iter = method.begin(); iter != method.end(); iter++)
562+
{
563+
char_t ch = *iter;
564+
565+
if (size_t(ch) >= 128)
566+
return false;
567+
568+
if (!valid_chars[ch])
569+
return false;
570+
}
571+
572+
return true;
573+
}
574+
#endif
575+
545576

546577
} // namespace details
547578
}} // namespace web::http

Release/src/http/listener/http_linux_server.cpp

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
#include "stdafx.h"
2020
#include <boost/algorithm/string/find.hpp>
21+
#include <boost/regex.hpp>
2122
#include "cpprest/http_helpers.h"
2223
#include "cpprest/http_server_api.h"
2324
#include "cpprest/http_server.h"
@@ -75,7 +76,10 @@ void connection::start_request_response()
7576
{
7677
m_read_size = 0; m_read = 0;
7778
m_request_buf.consume(m_request_buf.size()); // clear the buffer
78-
async_read_until(*m_socket, m_request_buf, CRLF + CRLF, boost::bind(&connection::handle_http_line, this, placeholders::error));
79+
80+
// Wait for either double newline or a char which is not in the range [32-127] which suggests SSL handshaking.
81+
// For the SSL server support this line might need to be changed. Now, this prevents from hanging when SSL client tries to connect.
82+
async_read_until(*m_socket, m_request_buf, boost::regex(CRLF+CRLF+"|[\\x00-\\x1F]|[\\x80-\\xFF]"), boost::bind(&connection::handle_http_line, this, placeholders::error));
7983
}
8084

8185
void hostport_listener::on_accept(ip::tcp::socket* socket, const boost::system::error_code& ec)
@@ -114,6 +118,7 @@ void connection::handle_http_line(const boost::system::error_code& ec)
114118
}
115119
else
116120
{
121+
std::cout << "BAD REQUEST1" << std::endl;
117122
m_request.reply(status_codes::BadRequest);
118123
do_response();
119124
}
@@ -125,7 +130,7 @@ void connection::handle_http_line(const boost::system::error_code& ec)
125130
std::istream request_stream(&m_request_buf);
126131
std::skipws(request_stream);
127132

128-
std::string http_verb;
133+
std::string http_verb = "";
129134
request_stream >> http_verb;
130135

131136
if (boost::iequals(http_verb, http::methods::GET)) http_verb = http::methods::GET;
@@ -136,6 +141,18 @@ void connection::handle_http_line(const boost::system::error_code& ec)
136141
else if (boost::iequals(http_verb, http::methods::TRCE)) http_verb = http::methods::TRCE;
137142
else if (boost::iequals(http_verb, http::methods::CONNECT)) http_verb = http::methods::CONNECT;
138143
else if (boost::iequals(http_verb, http::methods::OPTIONS)) http_verb = http::methods::OPTIONS;
144+
145+
// Check to see if there is not allowed character on the input
146+
147+
if (!web::http::details::validate_method(http_verb))
148+
{
149+
std::cout << "BAD REQUEST" << std::endl;
150+
m_request.reply(status_codes::BadRequest);
151+
do_response(true);
152+
finish_request_response();
153+
return;
154+
}
155+
139156
m_request.set_method(http_verb);
140157

141158
std::string http_path_and_version;
@@ -184,6 +201,7 @@ void connection::handle_headers()
184201
}
185202
else
186203
{
204+
std::cout << "BAD REQUEST" << std::endl;
187205
m_request.reply(status_codes::BadRequest);
188206
do_response();
189207
return;
@@ -397,7 +415,7 @@ void connection::request_data_avail(size_t size)
397415
m_request._get_impl()->_complete(size);
398416
}
399417

400-
void connection::do_response()
418+
void connection::do_response(bool bad_request)
401419
{
402420
++m_refs;
403421
pplx::task<http_response> response_task = m_request.get_response();
@@ -423,9 +441,16 @@ void connection::do_response()
423441
response = http::http_response(status_codes::InternalError);
424442
}
425443
// before sending response, the full incoming message need to be processed.
426-
m_request.content_ready().then([=](pplx::task<http::http_request>) {
427-
async_process_response(response);
428-
});
444+
if (bad_request)
445+
{
446+
async_process_response(response);
447+
}
448+
else
449+
{
450+
m_request.content_ready().then([=](pplx::task<http::http_request>) {
451+
async_process_response(response);
452+
});
453+
}
429454
});
430455
}
431456

Release/tests/Functional/http/client/connections_and_errors.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ TEST_FIXTURE(uri_address, invalid_method)
146146
}
147147

148148
// This test sends an SSL request to a non-SSL server and should fail on handshaking
149-
TEST_FIXTURE(uri_address, handshake_fail, "Ignore:Linux", "TFS#747982")
149+
TEST_FIXTURE(uri_address, handshake_fail)
150150
{
151151
web::http::uri ssl_uri(U("https://localhost:34568/"));
152152

0 commit comments

Comments
 (0)