Merge pull request #4056 from sysown/v2.x-4055

Fix crash on malformed HTTP request to the REST API - Closes #4055

deps/Makefile

@@ -89,11 +89,12 @@ libssl/openssl/libssl.a:
 	cd libssl/openssl && ln -s . lib # curl wants this path
 libssl: libssl/openssl/libssl.a
 
-GCC_VERSION := $(shell gcc -dumpversion)
 MIN_VERSION := 4.9.0
-REQUIRE_PATCH = false
+GCC_VERSION := $(shell gcc -dumpversion)
+SORTED_VERSIONS := $(shell echo -e "$(GCC_VERSION)\n$(MIN_VERSION)" | sort -V)
 
-ifeq ($(MIN_VERSION),$(lastword $(sort $(GCC_VERSION) $(MIN_VERSION))))
+REQUIRE_PATCH = false
+ifeq ($(MIN_VERSION),$(lastword $(SORTED_VERSIONS)))
 	REQUIRE_PATCH = true
 endif
 
@@ -111,8 +112,9 @@ ifeq ($(REQUIRE_PATCH), true)
 	cd libhttpserver/libhttpserver && patch src/httpserver/http_response.hpp < ../http_response.hpp.patch
 	cd libhttpserver/libhttpserver && patch src/httpserver/string_response.hpp < ../string_response.hpp.patch
 	cd libhttpserver/libhttpserver && patch -p0 < ../re2_regex.patch
-	cd libhttpserver/libhttpserver && patch -p0 < ../final_val_post_process.patch
 endif
+	cd libhttpserver/libhttpserver && patch -p0 < ../final_val_post_process.patch
+	cd libhttpserver/libhttpserver && patch -p0 < ../empty_uri_log_crash.patch
 ifeq ($(UNAME_S),FreeBSD)
 	sed -i -e 's/\/bin\/bash/\/usr\/local\/bin\/bash/' libhttpserver/libhttpserver/bootstrap
 endif

deps/libhttpserver/empty_uri_log_crash.patch

@@ -0,0 +1,13 @@
+diff --git src/webserver.cpp src/webserver.cpp
+index 5ae7381..04a5a28 100644
+--- src/webserver.cpp
++++ src/webserver.cpp
+@@ -443,7 +443,7 @@ int policy_callback (void *cls, const struct sockaddr* addr, socklen_t addrlen)
+ void* uri_log(void* cls, const char* uri)
+ {
+     struct details::modded_request* mr = new details::modded_request();
+-    mr->complete_uri = new string(uri);
++    mr->complete_uri = new string(uri == NULL ? "" : uri);
+     mr->second = false;
+     return ((void*)mr);
+ }

test/tap/tap/utils.cpp

@@ -25,6 +25,18 @@
 using std::string;
 using std::vector;
 
+std::size_t count_matches(const string& str, const string& substr) {
+	std::size_t result = 0;
+	std::size_t pos = 0;
+
+	while ((pos = str.find(substr, pos)) != string::npos) {
+		result += 1;
+		pos += substr.length();
+	}
+
+	return result;
+}
+
 int mysql_query_t(MYSQL* mysql, const char* query) {
 	diag("%s: Issuing query '%s' to ('%s':%d)", get_formatted_time().c_str(), query, mysql->host, mysql->port);
 	return mysql_query(mysql, query);

test/tap/tap/utils.h

@@ -431,4 +431,14 @@ int configure_endpoints(
 	bool prevent_dups = true
 );
 
+/**
+ * @brief Returns the matches found of the 'substr' provided in the provided string.
+ *
+ * @param str String from which to count the matches.
+ * @param substr The substring which matches needs to be counted.
+ *
+ * @return Number of matches of the 'substr' in the provided string.
+ */
+std::size_t count_matches(const std::string& str, const std::string& substr);
+
 #endif // #define UTILS_H

test/tap/tests/reg_test_4055_restapi-t.cpp

@@ -0,0 +1,123 @@
+/**
+ * @file reg_test_4055_restapi-t.cpp
+ * @brief Simple regression test sending malformed query to RESTAPI.
+ * @details This test performs the following actions:
+ *   - Issue a malformed request to the RESTAPI.
+ *   - Checks that Admin interface is still responsive.
+ *   - Checks that the 'metrics' endpoint from the RESTAPI is still responsive.
+ *   - Perform minor correctness check on the 'metrics' endpoint response.
+ * @date 2022-12-15
+ */
+
+#include <cstring>
+#include <unistd.h>
+#include <vector>
+#include <string>
+#include <stdio.h>
+
+#include <arpa/inet.h>
+#include <sys/socket.h>
+
+#include "mysql.h"
+
+#include "tap.h"
+#include "command_line.h"
+#include "utils.h"
+
+using std::string;
+
+/* This is an estimation of the supported number of metrics as for '2022-12-15' */
+uint32_t SUPPORTED_METRICS = 148;
+
+int main(int argc, char** argv) {
+	plan(5);
+
+	CommandLine cl;
+
+	if (cl.getEnv()) {
+		diag("Failed to get the required environmental variables.");
+		return EXIT_FAILURE;
+	}
+
+	MYSQL* admin = mysql_init(NULL);
+
+	if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) {
+		fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin));
+		return EXIT_FAILURE;
+	}
+
+	// Enable 'RESTAPI'
+	MYSQL_QUERY(admin, "SET admin-restapi_enabled='true'");
+	MYSQL_QUERY(admin, "SET admin-restapi_port=6070");
+	MYSQL_QUERY(admin, "LOAD ADMIN VARIABLES TO RUNTIME");
+
+	int socket_desc;
+	struct sockaddr_in server;
+
+	socket_desc = socket(AF_INET , SOCK_STREAM , 0);
+	if (socket_desc == -1) {
+		return errno;
+	}
+
+	server.sin_addr.s_addr = inet_addr("127.0.0.1");
+	server.sin_family = AF_INET;
+	server.sin_port = htons(6070);
+
+	if (connect(socket_desc , (struct sockaddr *)&server , sizeof(server)) < 0) {
+		return errno;
+	}
+
+	// Perform the invalid request, and add a sleep to let ProxySQL process the data
+	{
+		ssize_t n = write(socket_desc, static_cast<const void*>(" \n"), strlen(" \n"));
+		diag("Written '%lu' bytes into socket", n);
+		sleep(1);
+	}
+
+	int myrc = mysql_query(admin, "SELECT version()");
+	ok(myrc == EXIT_SUCCESS, "ProxySQL is still up and reachable");
+
+	MYSQL_RES* myres = mysql_store_result(admin);
+	MYSQL_ROW myrow = mysql_fetch_row(myres);
+
+	string recv_version {};
+	if (myrow && myrow[0]) { recv_version = myrow[0]; }
+	mysql_free_result(myres);
+
+	ok(recv_version.empty() == false, "Received non empty ProxySQL version '%s'", recv_version.c_str());
+
+	uint64_t curl_res_code = 0;
+	string curl_res_data {};
+
+	CURLcode code = perform_simple_get("http://localhost:6070/metrics/", curl_res_code, curl_res_data);
+
+	ok(
+		code == CURLE_OK && curl_res_code == 200,
+		"RESTAPI still up and responding to requests - curl_code: %d, res_code: %lu",
+		code, curl_res_code
+	);
+
+	size_t matches = count_matches(curl_res_data, "# ");
+	const uint32_t min_exp_metrics = SUPPORTED_METRICS - 20;
+
+	ok(
+		matches % 2 == 0,
+		"Response from endpoint is well-formed (even number of '# ' metrics descriptions) - matches: '%ld'",
+		matches
+	);
+
+	ok(
+		min_exp_metrics < (matches / 2),
+		"Response contains more than a minimum of expected metrics - min: %d, act: %lu",
+		min_exp_metrics, matches / 2
+	);
+
+	if (tests_failed()) {
+		diag("Failed! Received GET response: \n\n%s", curl_res_data.c_str());
+	}
+
+	close(socket_desc);
+	mysql_close(admin);
+
+	return exit_status();
+}

test/tap/tests/test_admin_prometheus_metrics_dump-t_disabled_gh3571.cpp

@@ -21,18 +21,6 @@ using std::string;
 
 std::size_t supported_metrics = 121;
 
-std::size_t count_matches(const std::string& str, const std::string& substr) {
-	std::size_t result = 0;
-	std::size_t pos = 0;
-
-	while ((pos = str.find(substr, pos)) != std::string::npos) {
-		result += 1;
-		pos += substr.length();
-	}
-
-	return result;
-}
-
 int main(int argc, char** argv) {
 	CommandLine cl;