fix: large https request body could not be sent (#1352)

.clang-tidy

@@ -1,2 +1,2 @@
 # TODO: Discuss about -readability-identifier-length, -readability-avoid-const-params-in-decls
-Checks: "-*,bugprone-*,cert-*,clang-analyzer-*,concurrency-*,cppcoreguidelines-*,llvm-namespace-comment,modernize-*,performance-*,portability-*,readability-*,-bugprone-implicit-widening-of-multiplication-result,-bugprone-easily-swappable-parameters,-readability-identifier-length,-portability-restrict-system-includes,-modernize-use-trailing-return-type,-cppcoreguidelines-non-private-member-variables-in-classes,-readability-avoid-const-params-in-decls,-cppcoreguidelines-owning-memory,-readability-function-cognitive-complexity,-cppcoreguidelines-avoid-do-while"
+Checks: "-*,bugprone-*,cert-*,clang-analyzer-*,concurrency-*,cppcoreguidelines-*,llvm-namespace-comment,modernize-*,performance-*,portability-*,readability-*,-bugprone-implicit-widening-of-multiplication-result,-bugprone-easily-swappable-parameters,-readability-identifier-length,-portability-restrict-system-includes,-modernize-use-trailing-return-type,-cppcoreguidelines-non-private-member-variables-in-classes,-readability-avoid-const-params-in-decls,-cppcoreguidelines-owning-memory,-readability-function-cognitive-complexity,-cppcoreguidelines-avoid-do-while,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-hicpp-no-array-decay"

.github/workflows/ci.yml

@@ -96,7 +96,7 @@ jobs:
 
       - name: Run unit tests
         if: ${{ matrix.cfg.ctest == 'yes' }}
-        run: cd build && ctest -VV
+        run: cd build/library && ./unittest
         env:
           DPP_UNIT_TEST_TOKEN: ${{secrets.DPP_UNIT_TEST_TOKEN}}
           TEST_GUILD_ID: ${{secrets.TEST_GUILD_ID}}
@@ -161,7 +161,7 @@ jobs:
           DONT_RUN_VCPKG: true
 
       - name: Run offline unit tests
-        run: cd build && ctest -VV
+        run: cd build/library && ./unittest
 
   windows: # Windows x64 and x86 build matrix
     permissions:

include/dpp/sslclient.h

@@ -96,6 +96,11 @@ class DPP_EXPORT ssl_client
 	 */
 	std::mutex ssl_mutex;
 
+	/**
+	 * @brief Mutex for output buffer
+	 */
+	std::mutex out_mutex;
+
 	/**
 	 * @brief Start offset into internal ring buffer for client to server IO
 	 */
@@ -217,7 +222,30 @@ class DPP_EXPORT ssl_client
 	 */
 	virtual void connect();
 
+	/**
+	 * @brief Set this to true to log all IO to debug for this connection.
+	 * This is an internal developer facility. Do not enable it unless you
+	 * need to, as it will be very noisy.
+	 */
+	bool raw_trace{false};
+
+	/**
+	 * @brief If raw_trace is set to true, log a debug message for this connection
+	 * @param message debug message
+	 */
+	void do_raw_trace(const std::string& message) const;
+
 public:
+	/**
+	 * @brief For low-level debugging, calling this function will
+	 * enable low level I/O logging for this connection to the logger.
+	 * This can be very loud, and output a lot of data, so only enable it
+	 * selectively where you need it.
+	 *
+	 * Generally, you won't need this, it is a library development utility.
+	 */
+	void enable_raw_tracing();
+
 	/**
 	 * @brief Get the bytes out objectGet total bytes sent
 	 * @return uint64_t bytes sent

src/dpp/cluster.cpp

@@ -382,12 +382,22 @@ void cluster::start(start_type return_after) {
 	}
 
 	if (return_after == st_return) {
-		engine_thread = std::thread([event_loop]() {
-			dpp::utility::set_thread_name("event_loop");
-			event_loop();
+		engine_thread = std::thread([this, event_loop]() {
+			try {
+				dpp::utility::set_thread_name("event_loop");
+				event_loop();
+			}
+			catch (const std::exception& e) {
+				log(ll_critical, "Event loop unhandled exception: " + std::string(e.what()));
+			}
 		});
 	} else {
-		event_loop();
+		try {
+			event_loop();
+		}
+		catch (const std::exception& e) {
+			log(ll_critical, "Event loop unhandled exception: " + std::string(e.what()));
+		}
 	}
 }
 

src/dpp/discordclient.cpp

@@ -602,19 +602,15 @@ voiceconn::~voiceconn() {
 
 voiceconn& voiceconn::connect(snowflake guild_id) {
 	if (this->is_ready() && !this->is_active()) {
-		/* This is wrapped in a thread because instantiating discord_voice_client can initiate a blocking SSL_connect() */
-		auto t = std::thread([guild_id, this]() {
-			try {
-				this->creator->log(ll_debug, "Connecting voice for guild " + std::to_string(guild_id) + " channel " + std::to_string(this->channel_id));
-				this->voiceclient = new discord_voice_client(creator->creator, this->channel_id, guild_id, this->token, this->session_id, this->websocket_hostname, this->dave);
-				/* Note: Spawns thread! */
-				this->voiceclient->run();
-			}
-			catch (std::exception &e) {
-				this->creator->log(ll_debug, "Can't connect to voice websocket (guild_id: " + std::to_string(guild_id) + ", channel_id: " + std::to_string(this->channel_id) + ": " + std::string(e.what()));
-			}
-		});
-		t.detach();
+		try {
+			this->creator->log(ll_debug, "Connecting voice for guild " + std::to_string(guild_id) + " channel " + std::to_string(this->channel_id));
+			this->voiceclient = new discord_voice_client(creator->creator, this->channel_id, guild_id, this->token, this->session_id, this->websocket_hostname, this->dave);
+			/* Note: Spawns thread! */
+			this->voiceclient->run();
+		}
+		catch (std::exception &e) {
+			this->creator->log(ll_debug, "Can't connect to voice websocket (guild_id: " + std::to_string(guild_id) + ", channel_id: " + std::to_string(this->channel_id) + "): " + std::string(e.what()));
+		}
 	}
 	return *this;
 }

src/dpp/dispatcher.cpp

@@ -271,11 +271,11 @@ command_value interaction_create_t::get_parameter(const std::string& name) const
 	return {};
 }
 
-voice_receive_t::voice_receive_t(dpp::cluster* creator, uint32_t shard_id, const std::string& raw, discord_voice_client* vc, snowflake _user_id, const uint8_t* pcm, size_t length) : event_dispatch_t(owner, shard_id, std::move(raw)), voice_client(vc), user_id(_user_id) {
+voice_receive_t::voice_receive_t(dpp::cluster* creator, uint32_t shard_id, const std::string& raw, discord_voice_client* vc, snowflake _user_id, const uint8_t* pcm, size_t length) : event_dispatch_t(creator, shard_id, std::move(raw)), voice_client(vc), user_id(_user_id) {
 	reassign(vc, _user_id, pcm, length);
 }
 
-voice_receive_t::voice_receive_t(dpp::cluster* creator, uint32_t shard_id, std::string&& raw, discord_voice_client* vc, snowflake _user_id, const uint8_t* pcm, size_t length) : event_dispatch_t(owner, shard_id, std::move(raw)), voice_client(vc), user_id(_user_id) {
+voice_receive_t::voice_receive_t(dpp::cluster* creator, uint32_t shard_id, std::string&& raw, discord_voice_client* vc, snowflake _user_id, const uint8_t* pcm, size_t length) : event_dispatch_t(creator, shard_id, std::move(raw)), voice_client(vc), user_id(_user_id) {
 	reassign(vc, _user_id, pcm, length);
 }
 

src/dpp/events/ready.cpp

@@ -56,8 +56,13 @@ void ready::handle(discord_client* client, json &j, const std::string &raw) {
 		client->resume_gateway_url = ugly;
 	}
 	/* Pre-resolve it into our cache so that we aren't waiting on this when we need it later */
-	static_cast<void>(resolve_hostname(client->resume_gateway_url, "443"));
-	client->log(ll_debug, "Resume URL for session " + client->sessionid + " is " + ugly + " (host: " + client->resume_gateway_url + ")");
+	try {
+		static_cast<void>(resolve_hostname(client->resume_gateway_url, "443"));
+		client->log(ll_debug, "Resume URL for session " + client->sessionid + " is " + ugly + " (host: " + client->resume_gateway_url + ")");
+	}
+	catch (std::exception& e) {
+		client->log(ll_warning, "Resume URL " + client->resume_gateway_url + " does not resolve: " + std::string(e.what()));
+	}
 
 	client->ready = true;
 

src/dpp/httpsclient.cpp

@@ -27,6 +27,7 @@
 #include <climits>
 #include <dpp/httpsclient.h>
 #include <dpp/utility.h>
+#include <dpp/cluster.h>
 
 namespace dpp {
 
@@ -54,6 +55,7 @@ void https_client::connect()
 	for (auto& [k,v] : request_headers) {
 		map_headers += k + ": " + v + "\r\n";
 	}
+
 	if (this->sfd != SOCKET_ERROR) {
 		this->socket_write(
 			this->request_type + " " + this->path + " HTTP/" + http_protocol + "\r\n"
@@ -72,43 +74,44 @@ void https_client::connect()
 }
 
 multipart_content https_client::build_multipart(const std::string &json, const std::vector<std::string>& filenames, const std::vector<std::string>& contents, const std::vector<std::string>& mimetypes) {
+
 	if (filenames.empty() && contents.empty()) {
+		/* If there are no files to upload, there is no need to build a multipart body */
 		if (!json.empty()) {
 			return { json, "application/json" };
-		} else {
-			return {json, ""};
 		}
-	} else {
-		/* Note: loss of upper 32 bits on this value is INTENTIONAL */
-		uint32_t dummy1 = (uint32_t)time(nullptr) + (uint32_t)time(nullptr);
-		time_t dummy2 = time(nullptr) * time(nullptr);
-		const std::string two_cr("\r\n\r\n");
-		const std::string boundary("-------------" + to_hex(dummy1) + to_hex(dummy2));
-		const std::string part_start("--" + boundary + "\r\nContent-Disposition: form-data; ");
-		const std::string mime_type_start("\r\nContent-Type: ");
-		const std::string default_mime_type("application/octet-stream");
-
-		std::string content("--" + boundary);
+		return {json, ""};
+	}
 
-		/* Special case, single file */
-		content += "\r\nContent-Type: application/json\r\nContent-Disposition: form-data; name=\"payload_json\"" + two_cr;
-		content += json + "\r\n";
-		if (filenames.size() == 1 && contents.size() == 1) {
-			content += part_start + "name=\"file\"; filename=\"" + filenames[0] + "\"";
-			content += mime_type_start + (mimetypes.empty() || mimetypes[0].empty() ? default_mime_type : mimetypes[0]) + two_cr;
-			content += contents[0];
-		} else {
-			/* Multiple files */
-			for (size_t i = 0; i < filenames.size(); ++i) {
-				content += part_start + "name=\"files[" + std::to_string(i) + "]\"; filename=\"" + filenames[i] + "\"";
-				content += "\r\nContent-Type: " + (mimetypes.size() <= i || mimetypes[i].empty() ? default_mime_type : mimetypes[i]) + two_cr;
-				content += contents[i];
-				content += "\r\n";
-			}
+	/* Note: loss of upper 32 bits on this value is INTENTIONAL */
+	uint32_t dummy1 = (uint32_t)time(nullptr) + (uint32_t)time(nullptr);
+	time_t dummy2 = time(nullptr) * time(nullptr);
+	const std::string two_cr("\r\n\r\n");
+	const std::string boundary("-------------" + to_hex(dummy1) + to_hex(dummy2));
+	const std::string part_start("--" + boundary + "\r\nContent-Disposition: form-data; ");
+	const std::string mime_type_start("\r\nContent-Type: ");
+	const std::string default_mime_type("application/octet-stream");
+
+	std::string content("--" + boundary);
+
+	/* Special case, single file */
+	content += "\r\nContent-Type: application/json\r\nContent-Disposition: form-data; name=\"payload_json\"" + two_cr;
+	content += json + "\r\n";
+	if (filenames.size() == 1 && contents.size() == 1) {
+		content += part_start + "name=\"file\"; filename=\"" + filenames[0] + "\"";
+		content += mime_type_start + (mimetypes.empty() || mimetypes[0].empty() ? default_mime_type : mimetypes[0]) + two_cr;
+		content += contents[0];
+	} else {
+		/* Multiple files */
+		for (size_t i = 0; i < filenames.size(); ++i) {
+			content += part_start + "name=\"files[" + std::to_string(i) + "]\"; filename=\"" + filenames[i] + "\"";
+			content += "\r\nContent-Type: " + (mimetypes.size() <= i || mimetypes[i].empty() ? default_mime_type : mimetypes[i]) + two_cr;
+			content += contents[i];
+			content += "\r\n";
 		}
-		content += "\r\n--" + boundary + "--";
-		return { content, "multipart/form-data; boundary=" + boundary };
 	}
+	content += "\r\n--" + boundary + "--";
+	return { content, "multipart/form-data; boundary=" + boundary };
 }
 
 const std::string https_client::get_header(std::string header_name) const {