From db7b65ed4267106d9a9a6703c08ab1a74720e3c7 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Wed, 27 Apr 2016 07:02:51 -0400 Subject: [PATCH] Various fixes, warnings: * Fix sig_wait * Fix websocket strict aliasing warning * Fix invokable strict aliasing * Silence fread warning in examples * Silence integer conversion warnings * Build parser-bench as test * Disable unused variable warning for asio: Caused by static variables declared in No known workaround. --- Jamroot | 1 + examples/file_body.h | 3 +- examples/sig_wait.h | 9 +- include/beast/http/basic_parser.hpp | 207 ++++++++++++++++++- include/beast/http/detail/basic_parser.hpp | 185 ----------------- include/beast/http/impl/basic_parser.ipp | 4 + include/beast/http/parser.hpp | 3 +- include/beast/websocket/detail/endian.hpp | 60 ++++++ include/beast/websocket/detail/frame.hpp | 22 ++ include/beast/websocket/detail/invokable.hpp | 43 ++-- test/Jamfile | 2 +- test/http/basic_parser.cpp | 123 +++++++++++ test/http/parser.cpp | 60 ++++++ test/http/parser_bench.cpp | 10 +- 14 files changed, 495 insertions(+), 237 deletions(-) create mode 100644 include/beast/websocket/detail/endian.hpp diff --git a/Jamroot b/Jamroot index 248851163..8d8f8c604 100644 --- a/Jamroot +++ b/Jamroot @@ -62,6 +62,7 @@ project beast shared on gcc:-std=c++11 + gcc:-Wno-unused-variable clang:-std=c++11 msvc:_SCL_SECURE_NO_WARNINGS=1 msvc:_CRT_SECURE_NO_WARNINGS=1 diff --git a/examples/file_body.h b/examples/file_body.h index 763c5a436..d6719d5ae 100644 --- a/examples/file_body.h +++ b/examples/file_body.h @@ -80,7 +80,8 @@ struct file_body operator()(resume_context&&, error_code&, Write&& write) { buf_len_ = std::min(size_ - offset_, sizeof(buf_)); - fread(buf_, 1, sizeof(buf_), file_); + auto const nread = fread(buf_, 1, sizeof(buf_), file_); + (void)nread; offset_ += buf_len_; write(boost::asio::buffer(buf_, buf_len_)); return offset_ >= size_; diff --git a/examples/sig_wait.h b/examples/sig_wait.h index f2bd7b55d..34ff1efd5 100644 --- a/examples/sig_wait.h +++ b/examples/sig_wait.h @@ -32,18 +32,11 @@ sig_wait() boost::asio::io_service ios; boost::asio::signal_set signals( ios, SIGINT, SIGTERM); - std::mutex m; - bool stop = false; - std::condition_variable cv; signals.async_wait( [&](boost::system::error_code const&, int) { - std::lock_guard lock(m); - stop = true; - cv.notify_one(); }); - std::unique_lock lock(m); - cv.wait(lock, [&]{ return stop; }); + ios.run(); } #endif diff --git a/include/beast/http/basic_parser.hpp b/include/beast/http/basic_parser.hpp index 20465e227..027f3a8a1 100644 --- a/include/beast/http/basic_parser.hpp +++ b/include/beast/http/basic_parser.hpp @@ -408,6 +408,191 @@ private: bool needs_eof(std::false_type) const; + template + class has_on_method_t + { + template().on_method( + std::declval(), + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_method = + std::integral_constant::value>; + + template + class has_on_uri_t + { + template().on_uri( + std::declval(), + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_uri = + std::integral_constant::value>; + + template + class has_on_reason_t + { + template().on_reason( + std::declval(), + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_reason = + std::integral_constant::value>; + + template + class has_on_request_t + { + template().on_request( + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_request = + std::integral_constant::value>; + + template + class has_on_response_t + { + template().on_response( + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_response = + std::integral_constant::value>; + + template + class has_on_field_t + { + template().on_uri( + std::declval(), + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_field = + std::integral_constant::value>; + + template + class has_on_value_t + { + template().on_uri( + std::declval(), + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_value = + std::integral_constant::value>; + + template + class has_on_headers_t + { + template().on_headers( + std::declval()))>> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_headers = + std::integral_constant::value>; + + template + class has_on_body_t + { + template().on_body( + std::declval(), + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_body = + std::integral_constant::value>; + + template + class has_on_complete_t + { + template().on_complete( + std::declval()), + std::true_type{})> + static R check(int); + template + static std::false_type check(...); + using type = decltype(check(0)); + public: + static bool const value = type::value; + }; + template + using has_on_complete = + std::integral_constant::value>; + void call_on_method(error_code& ec, boost::string_ref const& s, std::true_type) { @@ -423,7 +608,7 @@ private: boost::string_ref const& s) { call_on_method(ec, s, std::integral_constant::value>{}); + isRequest && has_on_method::value>{}); } void call_on_uri(error_code& ec, @@ -440,7 +625,7 @@ private: void call_on_uri(error_code& ec, boost::string_ref const& s) { call_on_uri(ec, s, std::integral_constant::value>{}); + isRequest && has_on_uri::value>{}); } void call_on_reason(error_code& ec, @@ -457,7 +642,7 @@ private: void call_on_reason(error_code& ec, boost::string_ref const& s) { call_on_reason(ec, s, std::integral_constant::value>{}); + ! isRequest && has_on_reason::value>{}); } void call_on_request(error_code& ec, std::true_type) @@ -471,7 +656,8 @@ private: void call_on_request(error_code& ec) { - call_on_request(ec, detail::has_on_request{}); + call_on_request(ec, std::integral_constant::value>{}); } void call_on_response(error_code& ec, std::true_type) @@ -485,7 +671,8 @@ private: void call_on_response(error_code& ec) { - call_on_response(ec, detail::has_on_response{}); + call_on_response(ec, std::integral_constant::value>{}); } void call_on_field(error_code& ec, @@ -501,7 +688,7 @@ private: void call_on_field(error_code& ec, boost::string_ref const& s) { - call_on_field(ec, s, detail::has_on_field{}); + call_on_field(ec, s, has_on_field{}); } void call_on_value(error_code& ec, @@ -517,7 +704,7 @@ private: void call_on_value(error_code& ec, boost::string_ref const& s) { - call_on_value(ec, s, detail::has_on_value{}); + call_on_value(ec, s, has_on_value{}); } int call_on_headers(error_code& ec, std::true_type) @@ -532,7 +719,7 @@ private: int call_on_headers(error_code& ec) { - return call_on_headers(ec, detail::has_on_headers{}); + return call_on_headers(ec, has_on_headers{}); } void call_on_body(error_code& ec, @@ -548,7 +735,7 @@ private: void call_on_body(error_code& ec, boost::string_ref const& s) { - call_on_body(ec, s, detail::has_on_body{}); + call_on_body(ec, s, has_on_body{}); } void call_on_complete(error_code& ec, std::true_type) @@ -562,7 +749,7 @@ private: void call_on_complete(error_code& ec) { - call_on_complete(ec, detail::has_on_complete{}); + call_on_complete(ec, has_on_complete{}); } }; diff --git a/include/beast/http/detail/basic_parser.hpp b/include/beast/http/detail/basic_parser.hpp index 79de107e9..27ee8e261 100644 --- a/include/beast/http/detail/basic_parser.hpp +++ b/include/beast/http/detail/basic_parser.hpp @@ -196,191 +196,6 @@ parser_str_t<_>::transfer_encoding[18]; using parser_str = parser_str_t<>; -template -class has_on_method_t -{ - template().on_method( - std::declval(), - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_method = - std::integral_constant::value>; - -template -class has_on_uri_t -{ - template().on_uri( - std::declval(), - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_uri = - std::integral_constant::value>; - -template -class has_on_reason_t -{ - template().on_reason( - std::declval(), - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_reason = - std::integral_constant::value>; - -template -class has_on_request_t -{ - template().on_request( - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_request = - std::integral_constant::value>; - -template -class has_on_response_t -{ - template().on_response( - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_response = - std::integral_constant::value>; - -template -class has_on_field_t -{ - template().on_uri( - std::declval(), - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_field = - std::integral_constant::value>; - -template -class has_on_value_t -{ - template().on_uri( - std::declval(), - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_value = - std::integral_constant::value>; - -template -class has_on_headers_t -{ - template().on_headers( - std::declval()))>> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_headers = - std::integral_constant::value>; - -template -class has_on_body_t -{ - template().on_body( - std::declval(), - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_body = - std::integral_constant::value>; - -template -class has_on_complete_t -{ - template().on_complete( - std::declval()), - std::true_type{})> - static R check(int); - template - static std::false_type check(...); - using type = decltype(check(0)); -public: - static bool const value = type::value; -}; -template -using has_on_complete = - std::integral_constant::value>; - } // detail } // http } // beast diff --git a/include/beast/http/impl/basic_parser.ipp b/include/beast/http/impl/basic_parser.ipp index ffa22786c..c63549f98 100644 --- a/include/beast/http/impl/basic_parser.ipp +++ b/include/beast/http/impl/basic_parser.ipp @@ -42,6 +42,9 @@ write(void const* data, std::size_t size, error_code& ec) using beast::http::detail::to_field_char; using beast::http::detail::to_value_char; using beast::http::detail::unhex; + + if(size == 0 && s_ != s_closed) + return 0; auto begin = reinterpret_cast(data); @@ -338,6 +341,7 @@ write(void const* data, std::size_t size, error_code& ec) if(cb(&self::call_on_reason)) return used(); pos_ = 0; + s_ = s_res_status; break; case s_res_status: diff --git a/include/beast/http/parser.hpp b/include/beast/http/parser.hpp index a85ef39bd..3dcc6d615 100644 --- a/include/beast/http/parser.hpp +++ b/include/beast/http/parser.hpp @@ -118,7 +118,7 @@ private: } return false; }; - while(false) + do { if(m("DELETE", method_t::http_delete)) break; @@ -187,6 +187,7 @@ private: if(m("UNLINK", method_t::http_unlink)) break; } + while(false); m_.url = std::move(this->uri_); diff --git a/include/beast/websocket/detail/endian.hpp b/include/beast/websocket/detail/endian.hpp new file mode 100644 index 000000000..a92b062a9 --- /dev/null +++ b/include/beast/websocket/detail/endian.hpp @@ -0,0 +1,60 @@ +// +// Copyright (c) 2013-2016 Vinnie Falco (vinnie dot falco at gmail dot com) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#ifndef BEAST_WEBSOCKET_DETAIL_ENDIAN_HPP +#define BEAST_WEBSOCKET_DETAIL_ENDIAN_HPP + +#include + +namespace beast { +namespace websocket { +namespace detail { + +inline +std::uint16_t +big_uint16_to_native(void const* buf) +{ + auto const p = reinterpret_cast< + std::uint8_t const*>(buf); + return (p[0]<<8) + p[1]; +} + +inline +std::uint64_t +big_uint64_to_native(void const* buf) +{ + auto const p = reinterpret_cast< + std::uint8_t const*>(buf); + return + (static_cast(p[0])<<56) + + (static_cast(p[1])<<48) + + (static_cast(p[2])<<40) + + (static_cast(p[3])<<32) + + (static_cast(p[4])<<24) + + (static_cast(p[5])<<16) + + (static_cast(p[6])<< 8) + + p[7]; +} + +inline +std::uint32_t +little_uint32_to_native(void const* buf) +{ + auto const p = reinterpret_cast< + std::uint8_t const*>(buf); + return + p[0] + + (static_cast(p[1])<< 8) + + (static_cast(p[2])<<16) + + (static_cast(p[3])<<24); +} + +} // detail +} // websocket +} // beast + +#endif diff --git a/include/beast/websocket/detail/frame.hpp b/include/beast/websocket/detail/frame.hpp index 3ec1bfc63..6ddbf1ac6 100644 --- a/include/beast/websocket/detail/frame.hpp +++ b/include/beast/websocket/detail/frame.hpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -244,8 +245,13 @@ read_fh2(frame_header& fh, Streambuf& sb, std::uint8_t b[2]; assert(buffer_size(sb.data()) >= sizeof(b)); sb.consume(buffer_copy(buffer(b), sb.data())); + #if 0 + // Causes strict-aliasing warning in gcc fh.len = reinterpret_cast< big_uint16_buf_t const*>(&b[0])->value(); + #else + fh.len = big_uint16_to_native(&b[0]); + #endif // length not canonical if(fh.len < 126) { @@ -259,8 +265,13 @@ read_fh2(frame_header& fh, Streambuf& sb, std::uint8_t b[8]; assert(buffer_size(sb.data()) >= sizeof(b)); sb.consume(buffer_copy(buffer(b), sb.data())); + #if 0 + // Causes strict-aliasing warning in gcc fh.len = reinterpret_cast< big_uint64_buf_t const*>(&b[0])->value(); + #else + fh.len = big_uint64_to_native(&b[0]); + #endif // length not canonical if(fh.len < 65536) { @@ -275,8 +286,13 @@ read_fh2(frame_header& fh, Streambuf& sb, std::uint8_t b[4]; assert(buffer_size(sb.data()) >= sizeof(b)); sb.consume(buffer_copy(buffer(b), sb.data())); + #if 0 + // Causes strict-aliasing warning in gcc fh.key = reinterpret_cast< little_uint32_buf_t const*>(&b[0])->value(); + #else + fh.key = little_uint32_to_native(&b[0]); + #endif } code = close_code::none; } @@ -327,9 +343,15 @@ read(close_reason& cr, { std::uint8_t b[2]; buffer_copy(buffer(b), cb); + #if 0 + // Causes strict-aliasing warning in gcc cr.code = static_cast( reinterpret_cast< big_uint16_buf_t const*>(&b[0])->value()); + #else + cr.code = static_cast( + big_uint16_to_native(&b[0])); + #endif cb.consume(2); n -= 2; if(! is_valid(cr.code)) diff --git a/include/beast/websocket/detail/invokable.hpp b/include/beast/websocket/detail/invokable.hpp index d049df65b..6fd24d29c 100644 --- a/include/beast/websocket/detail/invokable.hpp +++ b/include/beast/websocket/detail/invokable.hpp @@ -68,10 +68,9 @@ class invokable void operator()(){} }; - using buf_type = std::uint8_t[ - sizeof(holder)]; + using buf_type = char[sizeof(holder)]; - bool b_ = false; + base* base_ = nullptr; alignas(holder) buf_type buf_; public: @@ -81,7 +80,7 @@ public: // Engaged invokables must be invoked before // destruction otherwise the io_service // invariants are broken w.r.t completions. - assert(! b_); + assert(! base_); } #endif @@ -90,12 +89,12 @@ public: invokable& operator=(invokable const&) = delete; invokable(invokable&& other) - : b_(other.b_) { - if(other.b_) + if(other.base_) { - other.get().move(buf_); - other.b_ = false; + base_ = reinterpret_cast(&buf_[0]); + other.base_->move(buf_); + other.base_ = nullptr; } } @@ -105,13 +104,13 @@ public: // Engaged invokables must be invoked before // assignment otherwise the io_service // invariants are broken w.r.t completions. - assert(! b_); + assert(! base_); - if(other.b_) + if(other.base_) { - b_ = true; - other.get().move(buf_); - other.b_ = false; + base_ = reinterpret_cast(&buf_[0]); + other.base_->move(buf_); + other.base_ = nullptr; } return *this; } @@ -123,19 +122,13 @@ public: void maybe_invoke() { - if(b_) + if(base_) { - b_ = false; - get()(); + auto const basep = base_; + base_ = nullptr; + (*basep)(); } } - -private: - base& - get() - { - return *reinterpret_cast(&buf_[0]); - } }; template @@ -144,9 +137,9 @@ invokable::emplace(F&& f) { static_assert(sizeof(buf_type) >= sizeof(holder), "buffer too small"); - assert(! b_); + assert(! base_); ::new(buf_) holder(std::forward(f)); - b_ = true; + base_ = reinterpret_cast(&buf_[0]); } } // detail diff --git a/test/Jamfile b/test/Jamfile index cb947c2bf..275e5a788 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -58,7 +58,7 @@ unit-test websocket-tests : websocket/teardown.cpp ; -exe parser-bench : +unit-test parser-bench : main.cpp http/nodejs_parser.cpp http/parser_bench.cpp diff --git a/test/http/basic_parser.cpp b/test/http/basic_parser.cpp index 48627f864..e1cfc1e3a 100644 --- a/test/http/basic_parser.cpp +++ b/test/http/basic_parser.cpp @@ -33,6 +33,80 @@ class basic_parser_test : public beast::detail::unit_test::suite std::mt19937 rng_; public: + struct cb_req_checker + { + bool method = false; + bool uri = false; + bool request = false; + }; + + struct cb_res_checker + { + bool reason = false; + bool response = false; + }; + + template + struct cb_checker + : public basic_parser> + , std::conditional::type + + { + bool field = false; + bool value = false; + bool headers = false; + bool body = false; + bool complete = false; + + private: + friend class basic_parser>; + + void on_method(boost::string_ref const&, error_code&) + { + this->method = true; + } + void on_uri(boost::string_ref const&, error_code&) + { + this->uri = true; + } + void on_reason(boost::string_ref const&, error_code&) + { + this->reason = true; + } + void on_request(error_code&) + { + this->request = true; + } + void on_response(error_code&) + { + this->response = true; + } + void on_field(boost::string_ref const&, error_code&) + { + field = true; + } + void on_value(boost::string_ref const&, error_code&) + { + value = true; + } + int on_headers(error_code&) + { + headers = true; + return 0; + } + void on_body(boost::string_ref const&, error_code&) + { + body = true; + } + void on_complete(error_code&) + { + complete = true; + } + }; + + //-------------------------------------------------------------------------- + static std::string escaped_string(boost::string_ref const& s) @@ -106,6 +180,54 @@ public: } }; + void + testCallbacks() + { + { + cb_checker p; + error_code ec; + std::string const s = + "GET / HTTP/1.1\r\n" + "User-Agent: test\r\n" + "Content-Length: 1\r\n" + "\r\n" + "*"; + p.write(s.data(), s.size(), ec); + if( expect(! ec)) + { + expect(p.method); + expect(p.uri); + expect(p.request); + expect(p.field); + expect(p.value); + expect(p.headers); + expect(p.body); + expect(p.complete); + } + } + { + cb_checker p; + error_code ec; + std::string const s = + "HTTP/1.1 200 OK\r\n" + "Server: test\r\n" + "Content-Length: 1\r\n" + "\r\n" + "*"; + p.write(s.data(), s.size(), ec); + if( expect(! ec)) + { + expect(p.reason); + expect(p.response); + expect(p.field); + expect(p.value); + expect(p.headers); + expect(p.body); + expect(p.complete); + } + } + } + // Parse the entire input buffer as a valid message, // then parse in two pieces of all possible lengths. // @@ -497,6 +619,7 @@ public: void run() override { + testCallbacks(); testVersion(); testFlags(); testUpgrade(); diff --git a/test/http/parser.cpp b/test/http/parser.cpp index 6bbdeff1b..e9ee2b708 100644 --- a/test/http/parser.cpp +++ b/test/http/parser.cpp @@ -7,3 +7,63 @@ // Test that header file is self-contained. #include + +#include +#include +#include + +namespace beast { +namespace http { + +class parser_test : public beast::detail::unit_test::suite +{ +public: + void run() override + { + { + error_code ec; + parser>> p; + std::string const s = + "GET / HTTP/1.1\r\n" + "User-Agent: test\r\n" + "Content-Length: 1\r\n" + "\r\n" + "*"; + p.write(s.data(), s.size(), ec); + expect(! ec); + expect(p.complete()); + auto m = p.release(); + expect(m.method == method_t::http_get); + expect(m.url == "/"); + expect(m.version == 11); + expect(m.headers["User-Agent"] == "test"); + expect(m.body == "*"); + } + { + error_code ec; + parser>> p; + std::string const s = + "HTTP/1.1 200 OK\r\n" + "Server: test\r\n" + "Content-Length: 1\r\n" + "\r\n" + "*"; + p.write(s.data(), s.size(), ec); + expect(! ec); + expect(p.complete()); + auto m = p.release(); + expect(m.status == 200); + expect(m.reason == "OK"); + expect(m.version == 11); + expect(m.headers["Server"] == "test"); + expect(m.body == "*"); + } + } +}; + +BEAST_DEFINE_TESTSUITE(parser,http,beast); + +} // http +} // beast diff --git a/test/http/parser_bench.cpp b/test/http/parser_bench.cpp index 32cea58f0..0df290cfe 100644 --- a/test/http/parser_bench.cpp +++ b/test/http/parser_bench.cpp @@ -36,30 +36,28 @@ public: } corpus - build_corpus(std::size_t N, std::true_type) + build_corpus(std::size_t n, std::true_type) { corpus v; v.resize(N); message_fuzz mg; - for(std::size_t i = 0; i < N; ++i) + for(std::size_t i = 0; i < n; ++i) { mg.request(v[i]); -//log << debug::buffers_to_string(v[i].data()) << "\r"; size_ += v[i].size(); } return v; } corpus - build_corpus(std::size_t N, std::false_type) + build_corpus(std::size_t n, std::false_type) { corpus v; v.resize(N); message_fuzz mg; - for(std::size_t i = 0; i < N; ++i) + for(std::size_t i = 0; i < n; ++i) { mg.response(v[i]); -//log << debug::buffers_to_string(v[i].data()) << "\r"; size_ += v[i].size(); } return v;