Skip to content

Commit

Permalink
fix http(s) requests with 0 content length timing out
Browse files Browse the repository at this point in the history
  • Loading branch information
braindigitalis committed Nov 17, 2024
1 parent 5ae9b8b commit 192b8ae
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 87 deletions.
6 changes: 0 additions & 6 deletions src/dpp/httpsclient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,6 @@ bool https_client::handle_buffer(std::string &buffer)
} else {
content_length = ULLONG_MAX;
}
auto it_conn = response_headers.find("connection");
if (it_conn != response_headers.end() && it_conn->second == "close") {
keepalive = false;
}
chunked = false;
auto it_txenc = response_headers.find("transfer-encoding");
if (it_txenc != response_headers.end()) {
Expand All @@ -218,11 +214,9 @@ bool https_client::handle_buffer(std::string &buffer)
return true;
} else {
/* Non-HTTP-like response with invalid headers. Go no further. */
keepalive = false;
return false;
}
} else {
keepalive = false;
return false;
}

Expand Down
33 changes: 9 additions & 24 deletions src/dpp/socketengines/epoll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,42 +87,27 @@ struct socket_engine_epoll : public socket_engine_base {
}

if ((ev.events & EPOLLHUP) != 0U) {
//eh->flags = modify_event(epoll_handle, eh, eh->flags & ~WANT_ERROR);
//pool->enqueue([this, eh, fd]() {
eh->on_error(fd, *eh, 0);
// eh->flags = modify_event(epoll_handle, eh, eh->flags | WANT_ERROR);
//});
eh->on_error(fd, *eh, EPIPE);
continue;
}

if ((ev.events & EPOLLERR) != 0U) {
/* Get error number */
//eh->flags = modify_event(epoll_handle, eh, eh->flags & ~WANT_ERROR);
//pool->enqueue([this, eh, fd]() {
socklen_t codesize = sizeof(int);
int errcode{};
if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &errcode, &codesize) < 0) {
errcode = errno;
}
eh->on_error(fd, *eh, errcode);
// eh->flags = modify_event(epoll_handle, eh, eh->flags | WANT_ERROR);
//});
socklen_t codesize = sizeof(int);
int errcode{};
if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &errcode, &codesize) < 0) {
errcode = errno;
}
eh->on_error(fd, *eh, errcode);
continue;
}

if ((ev.events & EPOLLOUT) != 0U) {
eh->flags = modify_event(epoll_handle, eh, eh->flags & ~WANT_WRITE);
//pool->enqueue([eh, fd]() {
eh->on_write(fd, *eh);
//});
eh->on_write(fd, *eh);
}

if ((ev.events & EPOLLIN) != 0U) {
//eh->flags = modify_event(epoll_handle, eh, eh->flags & ~WANT_READ);
//pool->enqueue([this, eh, fd]() {
eh->on_read(fd, *eh);
// eh->flags = modify_event(epoll_handle, eh, eh->flags | WANT_READ);
//});
eh->on_read(fd, *eh);
}
}
prune();
Expand Down
8 changes: 4 additions & 4 deletions src/dpp/socketengines/kqueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ struct socket_engine_kqueue : public socket_engine_base {
std::vector<struct kevent> change_list;
std::vector<struct kevent> ke_list;

socket_engine_kqueue(const socket_engine_kqueue&) = default;
socket_engine_kqueue(socket_engine_kqueue&&) = default;
socket_engine_kqueue& operator=(const socket_engine_kqueue&) = default;
socket_engine_kqueue& operator=(socket_engine_kqueue&&) = default;
socket_engine_kqueue(const socket_engine_kqueue&) = delete;
socket_engine_kqueue(socket_engine_kqueue&&) = delete;
socket_engine_kqueue& operator=(const socket_engine_kqueue&) = delete;
socket_engine_kqueue& operator=(socket_engine_kqueue&&) = delete;

explicit socket_engine_kqueue(cluster* creator) : socket_engine_base(creator), kqueue_handle(kqueue()) {
change_list.resize(8);
Expand Down
28 changes: 10 additions & 18 deletions src/dpp/sslclient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,8 @@
#include <WinSock2.h>
#include <WS2tcpip.h>
#include <io.h>
/* Windows doesn't have standard poll(), it has WSAPoll.
* It's the same thing with different symbol names.
* Microsoft gotta be different.
*/
#define poll(fds, nfds, timeout) WSAPoll(fds, nfds, timeout)
#define pollfd WSAPOLLFD
/* Windows sockets library */
#pragma comment(lib, "ws2_32")
#else
/* Anyting other than Windows (e.g. sane OSes) */
#include <poll.h>
/* Anything other than Windows (e.g. sane OSes) */
#include <sys/socket.h>
#include <unistd.h>
#endif
Expand Down Expand Up @@ -110,11 +101,6 @@ class openssl_context_deleter {
*/
thread_local std::unique_ptr<SSL_CTX, openssl_context_deleter> openssl_context;

/**
* @brief Keepalive sessions, per-thread
*/
thread_local std::unordered_map<std::string, keepalive_cache_t> keepalives;

bool close_socket(dpp::socket sfd)
{
/* close_socket on an error socket is a non-op */
Expand Down Expand Up @@ -158,7 +144,7 @@ bool set_nonblocking(dpp::socket sockfd, bool non_blocking)
* @param addr address to connect to
* @param addrlen address length
* @param timeout_ms timeout in milliseconds
* @return int -1 on error, 0 on succcess just like POSIX connect()
* @return int -1 on error, 0 on success just like POSIX connect()
* @throw dpp::connection_exception on failure
*/
int start_connecting(dpp::socket sockfd, const struct sockaddr *addr, socklen_t addrlen, unsigned int timeout_ms) {
Expand Down Expand Up @@ -246,7 +232,6 @@ void ssl_client::connect()
void ssl_client::socket_write(const std::string_view data)
{
obuffer += data;
return;
}

void ssl_client::one_second_timer()
Expand Down Expand Up @@ -307,6 +292,7 @@ void ssl_client::on_read(socket fd, const struct socket_events& ev) {
}
buffer.append(server_to_client_buffer, r);
if (!this->handle_buffer(buffer)) {
this->close();
return;
}
bytes_in += r;
Expand Down Expand Up @@ -375,6 +361,12 @@ void ssl_client::on_read(socket fd, const struct socket_events& ev) {
}

void ssl_client::on_write(socket fd, const struct socket_events& e) {

if (!connected && plaintext) {
/* Plaintext sockets connect immediately on first write event */
connected = true;
}

if (connected) {
if (obuffer.length() && client_to_server_length == 0) {
memcpy(&client_to_server_buffer, obuffer.data(), obuffer.length() > DPP_BUFSIZE ? DPP_BUFSIZE : obuffer.length());
Expand All @@ -395,7 +387,7 @@ void ssl_client::on_write(socket fd, const struct socket_events& e) {
bytes_out += r;
if (client_to_server_length > 0) {
socket_events se{e};
se.flags = WANT_READ | WANT_ERROR;
se.flags = WANT_READ | WANT_WRITE | WANT_ERROR;
owner->socketengine->update_socket(se);
}
} else {
Expand Down
8 changes: 7 additions & 1 deletion src/soaktest/soak.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include <dpp/dpp.h>
#include <iostream>
#include <thread>
#include <chrono>

int main() {
using namespace std::chrono_literals;
Expand All @@ -35,6 +34,13 @@ int main() {
std::cout << "[" << dpp::utility::current_date_time() << "] " << dpp::utility::loglevel(log.severity) << ": " << log.message << std::endl;
});
soak_test.start(dpp::st_return);

dpp::https_client c2(&soak_test, "github.com", 80, "/", "GET", "", {}, true, 2, "1.1", [](dpp::https_client* c2) {
std::string hdr2 = c2->get_header("location");
std::string content2 = c2->get_content();
std::cout << "hdr2 == " << hdr2 << " ? https://github.com/ status = " << c2->get_status() << "\n";
});

while (true) {
std::this_thread::sleep_for(60s);
dpp::discord_client* dc = soak_test.get_shard(0);
Expand Down
50 changes: 25 additions & 25 deletions src/unittest/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2309,35 +2309,35 @@ Markdown lol \\|\\|spoiler\\|\\| \\~\\~strikethrough\\~\\~ \\`small \\*code\\* b
std::cout << e.what() << "\n";
set_test(HTTPS, false);
}
}

set_test(HTTP, false);
try {
dpp::https_client c2(&bot, "github.com", 80, "/", "GET", "", {}, true, 5, "1.1", [](dpp::https_client* c2) {
std::string hdr2 = c2->get_header("location");
std::string content2 = c2->get_content();
set_test(HTTP, hdr2 == "https://github.com/" && c2->get_status() == 301);
});
std::this_thread::sleep_for(std::chrono::seconds(6));
}
catch (const dpp::exception& e) {
std::cout << e.what() << "\n";
set_test(HTTP, false);
}
try {
dpp::https_client c2(&bot, "github.com", 80, "/", "GET", "", {}, true, 5, "1.1", [](dpp::https_client* c2) {
std::string hdr2 = c2->get_header("location");
std::string content2 = c2->get_content();
set_test(HTTP, hdr2 == "https://github.com/" && c2->get_status() == 301);
});
std::this_thread::sleep_for(std::chrono::seconds(6));
}
catch (const dpp::exception& e) {
std::cout << e.what() << "\n";
set_test(HTTP, false);
}

set_test(MULTIHEADER, false);
try {
dpp::https_client c2(&bot, "dl.dpp.dev", 443, "/cookietest.php", "GET", "", {}, true, 5, "1.1", [](dpp::https_client* c2) {
size_t count = c2->get_header_count("set-cookie");
size_t count_list = c2->get_header_list("set-cookie").size();
// This test script sets a bunch of cookies when we request it.
set_test(MULTIHEADER, c2->get_status() == 200 && count > 1 && count == count_list);
});
std::this_thread::sleep_for(std::chrono::seconds(6));
}
catch (const dpp::exception& e) {
std::cout << e.what() << "\n";
set_test(MULTIHEADER, false);
try {
dpp::https_client c2(&bot, "dl.dpp.dev", 443, "/cookietest.php", "GET", "", {}, true, 5, "1.1", [](dpp::https_client* c2) {
size_t count = c2->get_header_count("set-cookie");
size_t count_list = c2->get_header_list("set-cookie").size();
// This test script sets a bunch of cookies when we request it.
set_test(MULTIHEADER, c2->get_status() == 200 && count > 1 && count == count_list);
});
std::this_thread::sleep_for(std::chrono::seconds(6));
}
catch (const dpp::exception& e) {
std::cout << e.what() << "\n";
set_test(MULTIHEADER, false);
}
}

set_test(TIMERSTART, false);
Expand Down
18 changes: 9 additions & 9 deletions src/unittest/test.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ DPP_TEST(OPTCHOICE_SNOWFLAKE, "command_option_choice::fill_from_json: snowflake"
DPP_TEST(OPTCHOICE_STRING, "command_option_choice::fill_from_json: string", tf_offline);
DPP_TEST(HOSTINFO, "https_client::get_host_info()", tf_offline);
DPP_TEST(HTTPS, "https_client HTTPS request", tf_online);
DPP_TEST(HTTP, "https_client HTTP request", tf_offline);
DPP_TEST(HTTP, "https_client HTTP request", tf_online);
DPP_TEST(RUNONCE, "run_once<T>", tf_offline);
DPP_TEST(WEBHOOK, "webhook construct from URL", tf_offline);
DPP_TEST(MD_ESC_1, "Markdown escaping (ignore code block contents)", tf_offline);
Expand Down Expand Up @@ -228,9 +228,9 @@ DPP_TEST(INVITE_CREATE, "cluster::channel_invite_create", tf_online);
DPP_TEST(INVITE_GET, "cluster::invite_get", tf_online);
DPP_TEST(INVITE_DELETE, "cluster::invite_delete", tf_online);

/* Extended set -- Less important, skipped on the master branch due to rate limits and GitHub actions limitations*/
/* Extended set -- Less important, skipped on the master branch due to rate limits and GitHub actions limitations */
/* To execute, run unittests with "full" command line argument */
DPP_TEST(MULTIHEADER, "multiheader cookie test", tf_offline | tf_extended); // Fails in the EU as cookies are not sent without acceptance
DPP_TEST(MULTIHEADER, "multiheader cookie test", tf_online | tf_extended);

DPP_TEST(VOICECONN, "Connect to voice channel", tf_online | tf_extended);
DPP_TEST(VOICESEND, "Send audio to voice channel", tf_online | tf_extended); // udp unreliable on gitbub
Expand Down Expand Up @@ -290,7 +290,7 @@ extern dpp::snowflake TEST_EVENT_ID;

/* True if we skip tt_online tests */
extern bool offline;
/* True if we skip tt_extended tests*/
/* True if we skip tt_extended tests */
extern bool extended;
#ifdef DPP_CORO
inline constexpr bool coro = true;
Expand Down Expand Up @@ -576,9 +576,9 @@ inline constexpr auto is_owner = [](auto &&user) noexcept {
#endif

#define DPP_CHECK_CONSTRUCT_ASSIGN(test, type, var) do { \
DPP_CHECK(test, std::is_default_constructible_v<type>, var); \
DPP_CHECK(test, std::is_copy_constructible_v<type>, var); \
DPP_CHECK(test, std::is_move_constructible_v<type>, var); \
DPP_CHECK(test, std::is_copy_assignable_v<type>, var); \
DPP_CHECK(test, std::is_default_constructible_v<type>, var); \
DPP_CHECK(test, std::is_copy_constructible_v<type>, var); \
DPP_CHECK(test, std::is_move_constructible_v<type>, var); \
DPP_CHECK(test, std::is_copy_assignable_v<type>, var); \
DPP_CHECK(test, std::is_move_assignable_v<type>, var); \
} while(0)
} while(0)

0 comments on commit 192b8ae

Please sign in to comment.