From 9a7677bf1aec06879417166d249f7aeff890b5d6 Mon Sep 17 00:00:00 2001 From: The-EDev Date: Fri, 11 Feb 2022 00:56:30 +0300 Subject: [PATCH] Applied changes from review Also moved builtin_expect to utility.h (for use in sanitizer function) --- include/crow/common.h | 9 --------- include/crow/http_connection.h | 10 +++------- include/crow/http_parser_merged.h | 1 - include/crow/json.h | 2 +- include/crow/multipart.h | 5 +++-- include/crow/parser.h | 26 ++++++++++++++++---------- include/crow/utility.h | 16 ++++++++++++---- 7 files changed, 35 insertions(+), 34 deletions(-) diff --git a/include/crow/common.h b/include/crow/common.h index bdb1e9225..379c4f0ef 100644 --- a/include/crow/common.h +++ b/include/crow/common.h @@ -6,15 +6,6 @@ #include #include "crow/utility.h" -// TODO(EDev): Adding C++20's [[likely]] and [[unlikely]] attributes might be useful -#if defined(__GNUG__) || defined(__clang__) -#define CROW_LIKELY(X) __builtin_expect(!!(X), 1) -#define CROW_UNLIKELY(X) __builtin_expect(!!(X), 0) -#else -#define CROW_LIKELY(X) (X) -#define CROW_UNLIKELY(X) (X) -#endif - namespace crow { const char cr = '\r'; diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 466221e9a..c42dc469f 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -302,7 +302,7 @@ namespace crow } } - CROW_LOG_INFO << "Request: " << boost::lexical_cast(adaptor_.remote_endpoint()) << " " << this << " HTTP/" << (char)(req.http_ver_major + 48) << "." << (char)(req.http_ver_minor + 48) << ' ' << method_name(req.method) << " " << req.url; + CROW_LOG_INFO << "Request: " << boost::lexical_cast(adaptor_.remote_endpoint()) << " " << this << " HTTP/" << (char)(req.http_ver_major + '0') << "." << (char)(req.http_ver_minor + '0') << ' ' << method_name(req.method) << " " << req.url; need_to_call_after_handlers_ = false; @@ -539,11 +539,9 @@ namespace crow is_writing = false; if (close_connection_) { - //boost::asio::socket_base::linger option(true, 30); - //adaptor_.raw_socket().set_option(option); adaptor_.shutdown_readwrite(); adaptor_.close(); - //CROW_LOG_DEBUG << this << " from write (sync)(1)"; + CROW_LOG_DEBUG << this << " from write (static)"; check_destroy(); } @@ -598,11 +596,9 @@ namespace crow is_writing = false; if (close_connection_) { - //boost::asio::socket_base::linger option(true, 30); - //adaptor_.raw_socket().set_option(option); adaptor_.shutdown_readwrite(); adaptor_.close(); - //CROW_LOG_DEBUG << this << " from write (sync)(1)"; + CROW_LOG_DEBUG << this << " from write (res_stream)"; check_destroy(); } diff --git a/include/crow/http_parser_merged.h b/include/crow/http_parser_merged.h index 7ccde4571..3fa3e5658 100644 --- a/include/crow/http_parser_merged.h +++ b/include/crow/http_parser_merged.h @@ -125,7 +125,6 @@ enum http_connection_flags // This is basically 7 booleans placed into 1 integer CROW_XX(INVALID_CONSTANT, "invalid constant string") \ CROW_XX(INVALID_INTERNAL_STATE, "encountered unexpected internal state") \ CROW_XX(STRICT, "strict mode assertion failed") \ - /*CROW_XX(PAUSED, "parser is paused")*/ /*There's no need to pause the parser */ \ CROW_XX(UNKNOWN, "an unknown error occurred") \ CROW_XX(INVALID_TRANSFER_ENCODING, "request has invalid transfer-encoding") \ diff --git a/include/crow/json.h b/include/crow/json.h index 10109fdbf..928b4541e 100644 --- a/include/crow/json.h +++ b/include/crow/json.h @@ -18,7 +18,7 @@ #include #include -#include "crow/common.h" +#include "crow/utility.h" #include "crow/settings.h" #include "crow/returnable.h" #include "crow/logging.h" diff --git a/include/crow/multipart.h b/include/crow/multipart.h index 5d352732e..dd7ff0865 100644 --- a/include/crow/multipart.h +++ b/include/crow/multipart.h @@ -93,10 +93,11 @@ namespace crow private: std::string get_boundary(const std::string& header) const { - size_t found = header.find("boundary="); + constexpr char boundary_text[] = "boundary="; + size_t found = header.find(boundary_text); if (found) { - std::string to_return(header.substr(found + 9)); + std::string to_return(header.substr(found + strlen(boundary_text))); if (to_return[0] == '\"') { to_return = to_return.substr(1, to_return.length() - 2); diff --git a/include/crow/parser.h b/include/crow/parser.h index 0dc89f540..e038f2408 100644 --- a/include/crow/parser.h +++ b/include/crow/parser.h @@ -70,17 +70,8 @@ namespace crow { self->headers.emplace(std::move(self->header_field), std::move(self->header_value)); } - //NOTE(EDev): it seems that the problem is with crow's policy on closing the connection for HTTP_VERSION < 1.0, the behaviour for that in crow is "don't close the connection, but don't send a keep-alive either" - // HTTP1.1 = always send keep_alive, HTTP1.0 = only send if header exists, HTTP?.? = never send - self->keep_alive = (self->http_major == 1 && self->http_minor == 0) ? - ((self->flags & F_CONNECTION_KEEP_ALIVE) ? true : false) : - ((self->http_major == 1 && self->http_minor == 1) ? true : false); - - // HTTP1.1 = only close if close header exists, HTTP1.0 = always close unless keep_alive header exists, HTTP?.?= never close - self->close_connection = (self->http_major == 1 && self->http_minor == 0) ? - ((self->flags & F_CONNECTION_KEEP_ALIVE) ? false : true) : - ((self->http_major == 1 && self->http_minor == 1) ? ((self->flags & F_CONNECTION_CLOSE) ? true : false) : false); + self->set_connection_parameters(); self->process_header(); return 0; @@ -157,6 +148,21 @@ namespace crow handler_->handle(); } + inline void set_connection_parameters() + { + //NOTE(EDev): it seems that the problem is with crow's policy on closing the connection for HTTP_VERSION < 1.0, the behaviour for that in crow is "don't close the connection, but don't send a keep-alive either" + + // HTTP1.1 = always send keep_alive, HTTP1.0 = only send if header exists, HTTP?.? = never send + keep_alive = (http_major == 1 && http_minor == 0) ? + ((flags & F_CONNECTION_KEEP_ALIVE) ? true : false) : + ((http_major == 1 && http_minor == 1) ? true : false); + + // HTTP1.1 = only close if close header exists, HTTP1.0 = always close unless keep_alive header exists, HTTP?.?= never close + close_connection = (http_major == 1 && http_minor == 0) ? + ((flags & F_CONNECTION_KEEP_ALIVE) ? false : true) : + ((http_major == 1 && http_minor == 1) ? ((flags & F_CONNECTION_CLOSE) ? true : false) : false); + } + /// Take the parsed HTTP request data and convert it to a \ref crow.request request to_request() const { diff --git a/include/crow/utility.h b/include/crow/utility.h index 3dd63f9a3..223586e3e 100644 --- a/include/crow/utility.h +++ b/include/crow/utility.h @@ -11,6 +11,15 @@ #include "crow/settings.h" +// TODO(EDev): Adding C++20's [[likely]] and [[unlikely]] attributes might be useful +#if defined(__GNUG__) || defined(__clang__) +#define CROW_LIKELY(X) __builtin_expect(!!(X), 1) +#define CROW_UNLIKELY(X) __builtin_expect(!!(X), 0) +#else +#define CROW_LIKELY(X) (X) +#define CROW_UNLIKELY(X) (X) +#endif + namespace crow { namespace black_magic @@ -693,9 +702,8 @@ namespace crow data[i] = replacement; } else if ((c == '/') || (c == '\\')) - { - //TODO(EDev): uncomment below once #332 is merged - if (/*CROW_UNLIKELY(*/ i == 0 /*)*/) //Prevent Unix Absolute Paths (Windows Absolute Paths are prevented with `(c == ':')`) + { + if (CROW_UNLIKELY( i == 0 )) //Prevent Unix Absolute Paths (Windows Absolute Paths are prevented with `(c == ':')`) { data[i] = replacement; } @@ -703,9 +711,9 @@ namespace crow { checkForSpecialEntries = true; } + } } } - } } // namespace utility } // namespace crow