Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
@@ -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"
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down Expand Up @@ -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:
Expand Down
28 changes: 28 additions & 0 deletions include/dpp/sslclient.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
Expand Down
18 changes: 14 additions & 4 deletions src/dpp/cluster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
}
}

Expand Down
22 changes: 9 additions & 13 deletions src/dpp/discordclient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/dpp/dispatcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
9 changes: 7 additions & 2 deletions src/dpp/events/ready.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
63 changes: 33 additions & 30 deletions src/dpp/httpsclient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <climits>
#include <dpp/httpsclient.h>
#include <dpp/utility.h>
#include <dpp/cluster.h>

namespace dpp {

Expand Down Expand Up @@ -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"
Expand All @@ -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 {
Expand Down
Loading
Loading