Test shared_ptr before accessing members (etr#239)

LeSpocky · web-flow · commit 263f2bdef4d5 · 2021-11-08T16:02:07.000-08:00
* Add check for empty shared_ptr<http_response> Users can register arbitrary resources and could technically return empty shared_ptr without managed http_response object. This case should not crash the library. Signed-off-by: Alexander Dahl <ada@thorsis.com> References: etr#238 * Test shared_ptr before accessing members Avoid possible segfault if someone returns an empty shared_ptr<http_response> in her render method. Signed-off-by: Alexander Dahl <ada@thorsis.com> Fixes: etr#238
diff --git a/src/webserver.cpp b/src/webserver.cpp
@@ -596,7 +596,7 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct details
         try {
             if (hrm->is_allowed(method)) {
                 mr->dhrs = ((hrm)->*(mr->callback))(*mr->dhr);  // copy in memory (move in case)
-                if (mr->dhrs->get_response_code() == -1) {
+                if (mr->dhrs.get() == nullptr || mr->dhrs->get_response_code() == -1) {
                     mr->dhrs = internal_error_page(mr);
                 }
             } else {
diff --git a/test/integ/basic.cpp b/test/integ/basic.cpp
@@ -193,6 +193,13 @@ class no_response_resource : public http_resource {
      }
 };
 
+class empty_response_resource : public http_resource {
+ public:
+     const shared_ptr<http_response> render_GET(const http_request&) {
+         return shared_ptr<http_response>(nullptr);
+     }
+};
+
 class file_response_resource : public http_resource {
  public:
      const shared_ptr<http_response> render_GET(const http_request&) {
@@ -629,6 +636,22 @@ LT_BEGIN_AUTO_TEST(basic_suite, no_response)
     curl_easy_cleanup(curl);
 LT_END_AUTO_TEST(no_response)
 
+LT_BEGIN_AUTO_TEST(basic_suite, empty_response)
+    empty_response_resource resource;
+    ws->register_resource("base", &resource);
+    curl_global_init(CURL_GLOBAL_ALL);
+
+    CURL* curl = curl_easy_init();
+    curl_easy_setopt(curl, CURLOPT_URL, "localhost:8080/base");
+    curl_easy_setopt(curl, CURLOPT_HTTPGET, 1L);
+    CURLcode res = curl_easy_perform(curl);
+    LT_ASSERT_EQ(res, 0);
+    int64_t http_code = 0;
+    curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_code);
+    LT_ASSERT_EQ(http_code, 500);
+    curl_easy_cleanup(curl);
+LT_END_AUTO_TEST(empty_response)
+
 LT_BEGIN_AUTO_TEST(basic_suite, regex_matching)
     simple_resource resource;
     ws->register_resource("regex/matching/number/[0-9]+", &resource);

Original file line number	Diff line number	Diff line change
`@@ -596,7 +596,7 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct details`
`596`	`596`	`try {`
`597`	`597`	`if (hrm->is_allowed(method)) {`
`598`	`598`	`mr->dhrs = ((hrm)->(mr->callback))(mr->dhr); // copy in memory (move in case)`
`599`		`- if (mr->dhrs->get_response_code() == -1) {`
	`599`	`+ if (mr->dhrs.get() == nullptr \|\| mr->dhrs->get_response_code() == -1) {`
`600`	`600`	`mr->dhrs = internal_error_page(mr);`
`601`	`601`	`}`
`602`	`602`	`} else {`