Use string_view in API methods where applicable (#288)

* Issue #286: use string_view in API methods where applicable

This change reduces required copying of immutable data primarily in the
HTTP request API. Methods returning strings now return string_view to
avoid copying immutable data from the underlying MHD impl.
get_*s methods still return a std::map which must be allocated
and constructed, but does not copy underlying strings.

Fix a few instances of these method calls in the code base.

Introduce some type aliases for maps from string_view to string_view,
string to string to reduce code verbosity.
Overload the comparator operator()s as required.

Provide overloads for dump_*_map functions and a template impl to avoid
code duplication.

Bump C++ version to 17 required to use string_view. Remove CI
configurations using ancient compilers that don't support string_view
or C++17

* Issue #286: use string_view in API methods where applicable

This change reduces required copying of immutable data primarily in the
HTTP request API. Methods returning strings now return string_view to
avoid copying immutable data from the underlying MHD impl.
get_*s methods still return a std::map which must be allocated
and constructed, but does not copy underlying strings.

Fix a few instances of these method calls in the code base.

Introduce some type aliases for maps from string_view to string_view,
string to string to reduce code verbosity.
Overload the comparator operator()s as required.

Provide overloads for dump_*_map functions and a template impl to avoid
code duplication.

Bump C++ version to 17 required to use string_view. Remove CI
configurations using ancient compilers that don't support string_view
or C++17

* Issue #286: use string_view in API methods where applicable

This change reduces required copying of immutable data primarily in the
HTTP request API. Methods returning strings now return string_view to
avoid copying immutable data from the underlying MHD impl.
get_*s methods still return a std::map which must be allocated
and constructed, but does not copy underlying strings.

Fix a few instances of these method calls in the code base.

Introduce some type aliases for maps from string_view to string_view,
string to string to reduce code verbosity.
Overload the comparator operator()s as required.

Bump C++ version to 17 required to use string_view. Remove CI
configurations using ancient compilers that don't support string_view
or C++17

* remove accidentally added line of code

* fix data scope issue

In the file upload test, it was saving the arg_view_map from the
request, which went out of scope, leaving dangling refs.

Fix by making a deep copy.

* undo change from arg_map to arg_view_map

Because build_request_args makes a temp copy of underlying data and
mutates it, it is not feasible currently to return a view over that data.

* fix readme after changing get_args signature

* cache transformed data to return views over data

Add a data_cache type to the http request type, and a unique_ptr
instance member. This allows caching of transformed and/or copied data
from MHD, while preserving const-ness of all accessor methods.

Change all applicable methods to string_view, fix a few call sites.

* cache transformed data to return views over data

Add a data_cache type to the http request type, and a unique_ptr
instance member. This allows caching of transformed and/or copied data
from MHD, while preserving const-ness of all accessor methods.

Change all applicable methods to string_view, fix a few call sites.

* cache transformed data to return views over data

Add a data_cache type to the http request type, and a unique_ptr
instance member. This allows caching of transformed and/or copied data
from MHD, while preserving const-ness of all accessor methods.

Change all applicable methods to string_view, fix a few call sites.

* cache transformed data to return views over data

Add a data_cache type to the http request type, and a unique_ptr
instance member. This allows caching of transformed and/or copied data
from MHD, while preserving const-ness of all accessor methods.

Change all applicable methods to string_view, fix a few call sites.

* cache transformed data to return views over data

Add a http_request_data_cache type to the http request type, and a unique_ptr
instance member. This allows caching of transformed and/or copied data
from MHD, while preserving const-ness of all accessor methods.

Change all applicable methods to string_view, fix a few call sites.

Remove the existing http_request::args member, use the cache instead.

Make the http_request class move-only.

* cache transformed data to return views over data

Add a http_request_data_cache type to the http request type, and a unique_ptr
instance member. This allows caching of transformed and/or copied data
from MHD, while preserving const-ness of all accessor methods.

Change all applicable methods to string_view, fix a few call sites.

Remove the existing http_request::args member, use the cache instead.

Make the http_request class move-only.

* cache transformed data to return views over data

Add a http_request_data_cache type to the http request type, and a unique_ptr
instance member. This allows caching of transformed and/or copied data
from MHD, while preserving const-ness of all accessor methods.

Change all applicable methods to string_view, fix a few call sites.

Remove the existing http_request::args member, use the cache instead.

Make the http_request class move-only.

Author: stuart-byma

.github/workflows/verify-build.yml

@@ -98,24 +98,6 @@ jobs:
             cc-compiler: clang++-6.0
             debug: debug
             coverage: nocoverage
-          - test-group: extra
-            os: ubuntu-18.04
-            os-type: ubuntu
-            build-type: none
-            compiler-family: gcc
-            c-compiler: gcc-5
-            cc-compiler: g++-5
-            debug: nodebug
-            coverage: nocoverage
-          - test-group: extra
-            os: ubuntu-18.04
-            os-type: ubuntu
-            build-type: none
-            compiler-family: gcc
-            c-compiler: gcc-6
-            cc-compiler: g++-6
-            debug: nodebug
-            coverage: nocoverage
           - test-group: extra
             os: ubuntu-latest
             os-type: ubuntu
@@ -152,24 +134,6 @@ jobs:
             cc-compiler: g++-10
             debug: nodebug
             coverage: nocoverage
-          - test-group: extra
-            os: ubuntu-18.04
-            os-type: ubuntu
-            build-type: none
-            compiler-family: clang
-            c-compiler: clang-3.9
-            cc-compiler: clang++-3.9
-            debug: nodebug
-            coverage: nocoverage
-          - test-group: extra
-            os: ubuntu-18.04
-            os-type: ubuntu
-            build-type: none
-            compiler-family: clang
-            c-compiler: clang-4.0
-            cc-compiler: clang++-4.0
-            debug: nodebug
-            coverage: nocoverage
           - test-group: extra
             os: ubuntu-18.04
             os-type: ubuntu

README.md

@@ -511,7 +511,9 @@ There are essentially four ways to specify an endpoint string:
       class url_args_resource : public http_resource {
       public:
           std::shared_ptr<http_response> render(const http_request& req) {
-              return std::shared_ptr<http_response>(new string_response("ARGS: " + req.get_arg("arg1") + " and " + req.get_arg("arg2")));
+              std::string arg1(req.get_arg("arg1"));
+              std::string arg2(req.get_arg("arg2"));
+              return std::shared_ptr<http_response>(new string_response("ARGS: " + arg1 + " and " + arg2));
           }
       };
 
@@ -559,13 +561,13 @@ The `http_request` class has a set of methods you will have access to when imple
 * _**const std::vector\<std::string\>&** get_path_pieces() **const**:_ Returns the components of the path requested by the HTTP client (each piece of the path split by `'/'`.
 * _**const std::string&** get_path_piece(int index) **const**:_ Returns one piece of the path requested by the HTTP client. The piece is selected through the `index` parameter (0-indexed). 
 * _**const std::string&** get_method() **const**:_ Returns the method requested by the HTTP client.
-* _**const std::string** get_header(**const std::string&** key) **const**:_ Returns the header with name equal to `key` if present in the HTTP request. Returns an `empty string` otherwise.
-* _**const std::string** get_cookie(**const std::string&** key) **const**:_ Returns the cookie with name equal to `key` if present in the HTTP request. Returns an `empty string` otherwise.
-* _**const std::string** get_footer(**const std::string&** key) **const**:_ Returns the footer with name equal to `key` if present in the HTTP request (only for http 1.1 chunked encodings). Returns an `empty string` otherwise.
-* _**const std::string** get_arg(**const std::string&** key) **const**:_ Returns the argument with name equal to `key` if present in the HTTP request. Arguments can be (1) querystring parameters, (2) path argument (in case of parametric endpoint, (3) parameters parsed from the HTTP request body if the body is in `application/x-www-form-urlencoded` or `multipart/form-data` formats and the postprocessor is enabled in the webserver (enabled by default).
-* _**const std::map<std::string, std::string, http::header_comparator>** get_headers() **const**:_ Returns a map containing all the headers present in the HTTP request.
-* _**const std::map<std::string, std::string, http::header_comparator>** get_cookies() **const**:_ Returns a map containing all the cookies present in the HTTP request.
-* _**const std::map<std::string, std::string, http::header_comparator>** get_footers() **const**:_ Returns a map containing all the footers present in the HTTP request (only for http 1.1 chunked encodings).
+* _**std::string_view** get_header(**std::string_view** key) **const**:_ Returns the header with name equal to `key` if present in the HTTP request. Returns an `empty string` otherwise.
+* _**std::string_view** get_cookie(**std::string_view** key) **const**:_ Returns the cookie with name equal to `key` if present in the HTTP request. Returns an `empty string` otherwise.
+* _**std::string_view** get_footer(**std::string_view** key) **const**:_ Returns the footer with name equal to `key` if present in the HTTP request (only for http 1.1 chunked encodings). Returns an `empty string` otherwise.
+* _**std::string_view** get_arg(**std::string_view** key) **const**:_ Returns the argument with name equal to `key` if present in the HTTP request. Arguments can be (1) querystring parameters, (2) path argument (in case of parametric endpoint, (3) parameters parsed from the HTTP request body if the body is in `application/x-www-form-urlencoded` or `multipart/form-data` formats and the postprocessor is enabled in the webserver (enabled by default).
+* _**const std::map<std::string_view, std::string_view, http::header_comparator>** get_headers() **const**:_ Returns a map containing all the headers present in the HTTP request.
+* _**const std::map<std::string_view, std::string_view, http::header_comparator>** get_cookies() **const**:_ Returns a map containing all the cookies present in the HTTP request.
+* _**const std::map<std::string_view, std::string_view, http::header_comparator>** get_footers() **const**:_ Returns a map containing all the footers present in the HTTP request (only for http 1.1 chunked encodings).
 * _**const std::map<std::string, std::string, http::arg_comparator>** get_args() **const**:_ Returns all the arguments present in the HTTP request. Arguments can be (1) querystring parameters, (2) path argument (in case of parametric endpoint, (3) parameters parsed from the HTTP request body if the body is in `application/x-www-form-urlencoded` or `multipart/form-data` formats and the postprocessor is enabled in the webserver (enabled by default).
 * _**const std::map<std::string, std::map<std::string, http::file_info>>** get_files() **const**:_ Returns information about all the uploaded files (if the files are stored to disk). This information includes the key (as identifier of the outer map), the original file name (as identifier of the inner map) and a class `file_info`, which includes the size of the file and the path to the file in the file system.
 * _**const std::string&** get_content() **const**:_ Returns the body of the HTTP request.
@@ -597,7 +599,7 @@ Details on the `http::file_info` structure.
     class hello_world_resource : public http_resource {
     public:
         std::shared_ptr<http_response> render(const http_request& req) {
-            return std::shared_ptr<http_response>(new string_response("Hello: " + req.get_arg("name")));
+            return std::shared_ptr<http_response>(new string_response("Hello: " + std::string(req.get_arg("name"))));
         }
     };
 

configure.ac

@@ -44,7 +44,7 @@ AC_LANG([C++])
 AC_SYS_LARGEFILE
 
 # Minimal feature-set required
-AX_CXX_COMPILE_STDCXX([14])
+AX_CXX_COMPILE_STDCXX([17])
 
 native_srcdir=$srcdir
 
@@ -197,7 +197,7 @@ AM_LDFLAGS="-lstdc++"
 
 if test x"$debugit" = x"yes"; then
     AC_DEFINE([DEBUG],[],[Debug Mode])
-    AM_CXXFLAGS="$AM_CXXFLAGS -DDEBUG -g -Wall -Wextra -Werror -pedantic -std=c++14 -Wno-unused-command-line-argument -O0"
+    AM_CXXFLAGS="$AM_CXXFLAGS -DDEBUG -g -Wall -Wextra -Werror -pedantic -std=c++17 -Wno-unused-command-line-argument -O0"
     AM_CFLAGS="$AM_CXXFLAGS -DDEBUG -g -Wall -Wextra -Werror -pedantic -Wno-unused-command-line-argument -O0"
 else
     AC_DEFINE([NDEBUG],[],[No-debug Mode])

examples/basic_authentication.cpp

@@ -27,7 +27,7 @@ class user_pass_resource : public httpserver::http_resource {
              return std::shared_ptr<httpserver::basic_auth_fail_response>(new httpserver::basic_auth_fail_response("FAIL", "test@example.com"));
          }
 
-         return std::shared_ptr<httpserver::string_response>(new httpserver::string_response(req.get_user() + " " + req.get_pass(), 200, "text/plain"));
+         return std::shared_ptr<httpserver::string_response>(new httpserver::string_response(std::string(req.get_user()) + " " + std::string(req.get_pass()), 200, "text/plain"));
      }
 };
 

examples/hello_with_get_arg.cpp

@@ -23,7 +23,7 @@
 class hello_world_resource : public httpserver::http_resource {
  public:
      std::shared_ptr<httpserver::http_response> render(const httpserver::http_request& req) {
-         return std::shared_ptr<httpserver::http_response>(new httpserver::string_response("Hello: " + req.get_arg("name")));
+         return std::shared_ptr<httpserver::http_response>(new httpserver::string_response("Hello: " + std::string(req.get_arg("name"))));
      }
 };
 

examples/hello_world.cpp

@@ -33,8 +33,8 @@ class hello_world_resource : public httpserver::http_resource {
 std::shared_ptr<httpserver::http_response> hello_world_resource::render(const httpserver::http_request& req) {
     // It is possible to store data inside the resource object that can be altered through the requests
     std::cout << "Data was: " << data << std::endl;
-    std::string datapar = req.get_arg("data");
-    set_some_data(datapar == "" ? "no data passed!!!" : datapar);
+    std::string_view datapar = req.get_arg("data");
+    set_some_data(datapar == "" ? "no data passed!!!" : std::string(datapar));
     std::cout << "Now data is:" << data << std::endl;
 
     // It is possible to send a response initializing an http_string_response that reads the content to send in response from a string.

examples/url_registration.cpp

@@ -30,14 +30,14 @@ class hello_world_resource : public httpserver::http_resource {
 class handling_multiple_resource : public httpserver::http_resource {
  public:
      std::shared_ptr<httpserver::http_response> render(const httpserver::http_request& req) {
-         return std::shared_ptr<httpserver::http_response>(new httpserver::string_response("Your URL: " + req.get_path()));
+         return std::shared_ptr<httpserver::http_response>(new httpserver::string_response("Your URL: " + std::string(req.get_path())));
      }
 };
 
 class url_args_resource : public httpserver::http_resource {
  public:
      std::shared_ptr<httpserver::http_response> render(const httpserver::http_request& req) {
-         return std::shared_ptr<httpserver::http_response>(new httpserver::string_response("ARGS: " + req.get_arg("arg1") + " and " + req.get_arg("arg2")));
+         return std::shared_ptr<httpserver::http_response>(new httpserver::string_response("ARGS: " + std::string(req.get_arg("arg1")) + " and " + std::string(req.get_arg("arg2"))));
      }
 };
 

src/http_request.cpp

@@ -32,17 +32,17 @@ const char http_request::EMPTY[] = "";
 
 struct arguments_accumulator {
     unescaper_ptr unescaper;
-    std::map<std::string, std::string, http::arg_comparator>* arguments;
+    http::arg_map* arguments;
 };
 
 void http_request::set_method(const std::string& method) {
     this->method = string_utilities::to_upper_copy(method);
 }
 
 bool http_request::check_digest_auth(const std::string& realm, const std::string& password, int nonce_timeout, bool* reload_nonce) const {
-    std::string digested_user = get_digested_user();
+    std::string_view digested_user = get_digested_user();
 
-    int val = MHD_digest_auth_check(underlying_connection, realm.c_str(), digested_user.c_str(), password.c_str(), nonce_timeout);
+    int val = MHD_digest_auth_check(underlying_connection, realm.c_str(), digested_user.data(), password.c_str(), nonce_timeout);
 
     if (val == MHD_INVALID_NONCE) {
         *reload_nonce = true;
@@ -55,8 +55,8 @@ bool http_request::check_digest_auth(const std::string& realm, const std::string
     return true;
 }
 
-const std::string http_request::get_connection_value(const std::string& key, enum MHD_ValueKind kind) const {
-    const char* header_c = MHD_lookup_connection_value(underlying_connection, kind, key.c_str());
+std::string_view http_request::get_connection_value(std::string_view key, enum MHD_ValueKind kind) const {
+    const char* header_c = MHD_lookup_connection_value(underlying_connection, kind, key.data());
 
     if (header_c == nullptr) return EMPTY;
 
@@ -67,76 +67,84 @@ MHD_Result http_request::build_request_header(void *cls, enum MHD_ValueKind kind
     // Parameters needed to respect MHD interface, but not used in the implementation.
     std::ignore = kind;
 
-    std::map<std::string, std::string, http::header_comparator>* dhr = static_cast<std::map<std::string, std::string, http::header_comparator>*>(cls);
+    http::header_view_map* dhr = static_cast<http::header_view_map*>(cls);
     (*dhr)[key] = value;
     return MHD_YES;
 }
 
-const std::map<std::string, std::string, http::header_comparator> http_request::get_headerlike_values(enum MHD_ValueKind kind) const {
-    std::map<std::string, std::string, http::header_comparator> headers;
+const http::header_view_map http_request::get_headerlike_values(enum MHD_ValueKind kind) const {
+    http::header_view_map headers;
 
     MHD_get_connection_values(underlying_connection, kind, &build_request_header, reinterpret_cast<void*>(&headers));
 
     return headers;
 }
 
-const std::string http_request::get_header(const std::string& key) const {
+std::string_view http_request::get_header(std::string_view key) const {
     return get_connection_value(key, MHD_HEADER_KIND);
 }
 
-const std::map<std::string, std::string, http::header_comparator> http_request::get_headers() const {
+const http::header_view_map http_request::get_headers() const {
     return get_headerlike_values(MHD_HEADER_KIND);
 }
 
-const std::string http_request::get_footer(const std::string& key) const {
+std::string_view http_request::get_footer(std::string_view key) const {
     return get_connection_value(key, MHD_FOOTER_KIND);
 }
 
-const std::map<std::string, std::string, http::header_comparator> http_request::get_footers() const {
+const http::header_view_map http_request::get_footers() const {
     return get_headerlike_values(MHD_FOOTER_KIND);
 }
 
-const std::string http_request::get_cookie(const std::string& key) const {
+std::string_view http_request::get_cookie(std::string_view key) const {
     return get_connection_value(key, MHD_COOKIE_KIND);
 }
 
-const std::map<std::string, std::string, http::header_comparator> http_request::get_cookies() const {
+const http::header_view_map http_request::get_cookies() const {
     return get_headerlike_values(MHD_COOKIE_KIND);
 }
 
-const std::string http_request::get_arg(const std::string& key) const {
-    std::map<std::string, std::string>::const_iterator it = args.find(key);
+std::string_view http_request::get_arg(std::string_view key) const {
+    std::map<std::string, std::string>::const_iterator it = cache->unescaped_args.find(std::string(key));
 
-    if (it != args.end()) {
+    if (it != cache->unescaped_args.end()) {
         return it->second;
     }
 
     return get_connection_value(key, MHD_GET_ARGUMENT_KIND);
 }
 
-const std::map<std::string, std::string, http::arg_comparator> http_request::get_args() const {
-    std::map<std::string, std::string, http::arg_comparator> arguments;
-    arguments.insert(args.begin(), args.end());
+const http::arg_view_map http_request::get_args() const {
+    http::arg_view_map arguments;
+
+    if (!cache->unescaped_args.empty()) {
+        arguments.insert(cache->unescaped_args.begin(), cache->unescaped_args.end());
+        return arguments;
+    }
 
     arguments_accumulator aa;
     aa.unescaper = unescaper;
-    aa.arguments = &arguments;
+    aa.arguments = &cache->unescaped_args;
 
     MHD_get_connection_values(underlying_connection, MHD_GET_ARGUMENT_KIND, &build_request_args, reinterpret_cast<void*>(&aa));
 
+    arguments.insert(cache->unescaped_args.begin(), cache->unescaped_args.end());
+
     return arguments;
 }
 
 http::file_info& http_request::get_or_create_file_info(const std::string& key, const std::string& upload_file_name) {
     return files[key][upload_file_name];
 }
 
-const std::string http_request::get_querystring() const {
-    std::string querystring = "";
+std::string_view http_request::get_querystring() const {
+    if (!cache->querystring.empty()) {
+        return cache->querystring;
+    }
 
-    MHD_get_connection_values(underlying_connection, MHD_GET_ARGUMENT_KIND, &build_request_querystring, reinterpret_cast<void*>(&querystring));
+    MHD_get_connection_values(underlying_connection, MHD_GET_ARGUMENT_KIND, &build_request_querystring, reinterpret_cast<void*>(&cache->querystring));
 
-    return querystring;
+    return cache->querystring;
 }
 
 MHD_Result http_request::build_request_args(void *cls, enum MHD_ValueKind kind, const char *key, const char *arg_value) {
@@ -173,47 +181,50 @@ MHD_Result http_request::build_request_querystring(void *cls, enum MHD_ValueKind
     return MHD_YES;
 }
 
-const std::string http_request::get_user() const {
-    char* username = nullptr;
+void http_request::fetch_user_pass() const {
     char* password = nullptr;
+    auto* username = MHD_basic_auth_get_username_password(underlying_connection, &password);
 
-    username = MHD_basic_auth_get_username_password(underlying_connection, &password);
-    if (password != nullptr) free(password);
-
-    std::string user;
-    if (username != nullptr) user = username;
-
-    free(username);
-
-    return user;
+    if (username != nullptr) {
+        cache->username = username;
+        MHD_free(username);
+    }
+    if (password != nullptr) {
+        cache->password = password;
+        MHD_free(password);
+    }
 }
 
-const std::string http_request::get_pass() const {
-    char* username = nullptr;
-    char* password = nullptr;
-
-    username = MHD_basic_auth_get_username_password(underlying_connection, &password);
-    if (username != nullptr) free(username);
-
-    std::string pass;
-    if (password != nullptr) pass = password;
-
-    free(password);
+std::string_view http_request::get_user() const {
+    if (!cache->username.empty()) {
+        return cache->username;
+    }
+    fetch_user_pass();
+    return cache->username;
+}
 
-    return pass;
+std::string_view http_request::get_pass() const {
+    if (!cache->password.empty()) {
+        return cache->password;
+    }
+    fetch_user_pass();
+    return cache->password;
 }
 
-const std::string http_request::get_digested_user() const {
-    char* digested_user_c = nullptr;
-    digested_user_c = MHD_digest_auth_get_username(underlying_connection);
+std::string_view http_request::get_digested_user() const {
+    if (!cache->digested_user.empty()) {
+        return cache->digested_user;
+    }
+
+    char* digested_user_c = MHD_digest_auth_get_username(underlying_connection);
 
-    std::string digested_user = EMPTY;
+    cache->digested_user = EMPTY;
     if (digested_user_c != nullptr) {
-        digested_user = digested_user_c;
+        cache->digested_user = digested_user_c;
         free(digested_user_c);
     }
 
-    return digested_user;
+    return cache->digested_user;
 }
 
 #ifdef HAVE_GNUTLS
@@ -229,10 +240,15 @@ gnutls_session_t http_request::get_tls_session() const {
 }
 #endif  // HAVE_GNUTLS
 
-const std::string http_request::get_requestor() const {
+std::string_view http_request::get_requestor() const {
+    if (!cache->requestor_ip.empty()) {
+        return cache->requestor_ip;
+    }
+
     const MHD_ConnectionInfo * conninfo = MHD_get_connection_info(underlying_connection, MHD_CONNECTION_INFO_CLIENT_ADDRESS);
 
-    return http::get_ip_str(conninfo->client_addr);
+    cache->requestor_ip = http::get_ip_str(conninfo->client_addr);
+    return cache->requestor_ip;
 }
 
 uint16_t http_request::get_requestor_port() const {