Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Print announce error response (#11878)
Fix HTTPFetch caller and request ID to 64 bits
Check that allocated caller ID is not DISCARD
Print body if serverlist request returns error
Don't print control characters from HTTP responses
Document special HTTPFetch caller IDs
Allow unicode to be printed
  • Loading branch information
ShadowNinja committed Jan 1, 2022
1 parent 544b9d5 commit 29d2b2c
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 36 deletions.
6 changes: 3 additions & 3 deletions src/client/clientmedia.h
Expand Up @@ -174,12 +174,12 @@ class ClientMediaDownloader : public IClientMediaDownloader
s32 m_uncached_received_count = 0;

// Status of remote transfers
unsigned long m_httpfetch_caller;
unsigned long m_httpfetch_next_id = 0;
u64 m_httpfetch_caller;
u64 m_httpfetch_next_id = 0;
s32 m_httpfetch_active = 0;
s32 m_httpfetch_active_limit = 0;
s32 m_outstanding_hash_sets = 0;
std::unordered_map<unsigned long, std::string> m_remote_file_transfers;
std::unordered_map<u64, std::string> m_remote_file_transfers;

// All files up to this name have either been received from a
// remote server or failed on all remote servers, so those files
Expand Down
51 changes: 28 additions & 23 deletions src/httpfetch.cpp
Expand Up @@ -38,7 +38,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#include "noise.h"

static std::mutex g_httpfetch_mutex;
static std::unordered_map<unsigned long, std::queue<HTTPFetchResult>>
static std::unordered_map<u64, std::queue<HTTPFetchResult>>
g_httpfetch_results;
static PcgRandom g_callerid_randomness;

Expand All @@ -52,22 +52,21 @@ HTTPFetchRequest::HTTPFetchRequest() :

static void httpfetch_deliver_result(const HTTPFetchResult &fetch_result)
{
unsigned long caller = fetch_result.caller;
u64 caller = fetch_result.caller;
if (caller != HTTPFETCH_DISCARD) {
MutexAutoLock lock(g_httpfetch_mutex);
g_httpfetch_results[caller].emplace(fetch_result);
}
}

static void httpfetch_request_clear(unsigned long caller);
static void httpfetch_request_clear(u64 caller);

unsigned long httpfetch_caller_alloc()
u64 httpfetch_caller_alloc()
{
MutexAutoLock lock(g_httpfetch_mutex);

// Check each caller ID except HTTPFETCH_DISCARD
const unsigned long discard = HTTPFETCH_DISCARD;
for (unsigned long caller = discard + 1; caller != discard; ++caller) {
// Check each caller ID except reserved ones
for (u64 caller = HTTPFETCH_CID_START; caller != 0; ++caller) {
auto it = g_httpfetch_results.find(caller);
if (it == g_httpfetch_results.end()) {
verbosestream << "httpfetch_caller_alloc: allocating "
Expand All @@ -79,18 +78,17 @@ unsigned long httpfetch_caller_alloc()
}

FATAL_ERROR("httpfetch_caller_alloc: ran out of caller IDs");
return discard;
}

unsigned long httpfetch_caller_alloc_secure()
u64 httpfetch_caller_alloc_secure()
{
MutexAutoLock lock(g_httpfetch_mutex);

// Generate random caller IDs and make sure they're not
// already used or equal to HTTPFETCH_DISCARD
// already used or reserved.
// Give up after 100 tries to prevent infinite loop
u8 tries = 100;
unsigned long caller;
size_t tries = 100;
u64 caller;

do {
caller = (((u64) g_callerid_randomness.next()) << 32) |
Expand All @@ -100,7 +98,8 @@ unsigned long httpfetch_caller_alloc_secure()
FATAL_ERROR("httpfetch_caller_alloc_secure: ran out of caller IDs");
return HTTPFETCH_DISCARD;
}
} while (g_httpfetch_results.find(caller) != g_httpfetch_results.end());
} while (caller >= HTTPFETCH_CID_START &&
g_httpfetch_results.find(caller) != g_httpfetch_results.end());

verbosestream << "httpfetch_caller_alloc_secure: allocating "
<< caller << std::endl;
Expand All @@ -110,7 +109,7 @@ unsigned long httpfetch_caller_alloc_secure()
return caller;
}

void httpfetch_caller_free(unsigned long caller)
void httpfetch_caller_free(u64 caller)
{
verbosestream<<"httpfetch_caller_free: freeing "
<<caller<<std::endl;
Expand All @@ -122,7 +121,7 @@ void httpfetch_caller_free(unsigned long caller)
}
}

bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetch_result)
bool httpfetch_async_get(u64 caller, HTTPFetchResult &fetch_result)
{
MutexAutoLock lock(g_httpfetch_mutex);

Expand Down Expand Up @@ -242,7 +241,6 @@ HTTPFetchOngoing::HTTPFetchOngoing(const HTTPFetchRequest &request_,

// Set static cURL options
curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1);
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 3);
curl_easy_setopt(curl, CURLOPT_ENCODING, "gzip");
Expand Down Expand Up @@ -393,10 +391,17 @@ const HTTPFetchResult * HTTPFetchOngoing::complete(CURLcode res)
}

if (res != CURLE_OK) {
errorstream << request.url << " not found ("
<< curl_easy_strerror(res) << ")"
<< " (response code " << result.response_code << ")"
errorstream << "HTTPFetch for " << request.url << " failed ("
<< curl_easy_strerror(res) << ")" << std::endl;
} else if (result.response_code >= 400) {
errorstream << "HTTPFetch for " << request.url
<< " returned response code " << result.response_code
<< std::endl;
if (result.caller == HTTPFETCH_PRINT_ERR && !result.data.empty()) {
errorstream << "Response body:" << std::endl;
safe_print_string(errorstream, result.data);
errorstream << std::endl;
}
}

return &result;
Expand Down Expand Up @@ -474,7 +479,7 @@ class CurlFetchThread : public Thread
m_requests.push_back(req);
}

void requestClear(unsigned long caller, Event *event)
void requestClear(u64 caller, Event *event)
{
Request req;
req.type = RT_CLEAR;
Expand Down Expand Up @@ -505,7 +510,7 @@ class CurlFetchThread : public Thread

}
else if (req.type == RT_CLEAR) {
unsigned long caller = req.fetch_request.caller;
u64 caller = req.fetch_request.caller;

// Abort all ongoing fetches for the caller
for (std::vector<HTTPFetchOngoing*>::iterator
Expand Down Expand Up @@ -778,7 +783,7 @@ void httpfetch_async(const HTTPFetchRequest &fetch_request)
g_httpfetch_thread->start();
}

static void httpfetch_request_clear(unsigned long caller)
static void httpfetch_request_clear(u64 caller)
{
if (g_httpfetch_thread->isRunning()) {
Event event;
Expand Down Expand Up @@ -827,7 +832,7 @@ void httpfetch_async(const HTTPFetchRequest &fetch_request)
httpfetch_deliver_result(fetch_result);
}

static void httpfetch_request_clear(unsigned long caller)
static void httpfetch_request_clear(u64 caller)
{
}

Expand Down
27 changes: 17 additions & 10 deletions src/httpfetch.h
Expand Up @@ -23,10 +23,17 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#include "util/string.h"
#include "config.h"

// Can be used in place of "caller" in asynchronous transfers to discard result
// (used as default value of "caller")
// These can be used in place of "caller" in to specify special handling.
// Discard result (used as default value of "caller").
#define HTTPFETCH_DISCARD 0
// Indicates that the result should not be discarded when performing a
// synchronous request (since a real caller ID is not needed for synchronous
// requests because the result does not have to be retrieved later).
#define HTTPFETCH_SYNC 1
// Print response body to console if the server returns an error code.
#define HTTPFETCH_PRINT_ERR 2
// Start of regular allocated caller IDs.
#define HTTPFETCH_CID_START 3

// Methods
enum HttpMethod : u8
Expand All @@ -43,11 +50,11 @@ struct HTTPFetchRequest

// Identifies the caller (for asynchronous requests)
// Ignored by httpfetch_sync
unsigned long caller = HTTPFETCH_DISCARD;
u64 caller = HTTPFETCH_DISCARD;

// Some number that identifies the request
// (when the same caller issues multiple httpfetch_async calls)
unsigned long request_id = 0;
u64 request_id = 0;

// Timeout for the whole transfer, in milliseconds
long timeout;
Expand Down Expand Up @@ -85,8 +92,8 @@ struct HTTPFetchResult
long response_code = 0;
std::string data = "";
// The caller and request_id from the corresponding HTTPFetchRequest.
unsigned long caller = HTTPFETCH_DISCARD;
unsigned long request_id = 0;
u64 caller = HTTPFETCH_DISCARD;
u64 request_id = 0;

HTTPFetchResult() = default;

Expand All @@ -107,19 +114,19 @@ void httpfetch_async(const HTTPFetchRequest &fetch_request);

// If any fetch for the given caller ID is complete, removes it from the
// result queue, sets the fetch result and returns true. Otherwise returns false.
bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetch_result);
bool httpfetch_async_get(u64 caller, HTTPFetchResult &fetch_result);

// Allocates a caller ID for httpfetch_async
// Not required if you want to set caller = HTTPFETCH_DISCARD
unsigned long httpfetch_caller_alloc();
u64 httpfetch_caller_alloc();

// Allocates a non-predictable caller ID for httpfetch_async
unsigned long httpfetch_caller_alloc_secure();
u64 httpfetch_caller_alloc_secure();

// Frees a caller ID allocated with httpfetch_caller_alloc
// Note: This can be expensive, because the httpfetch thread is told
// to stop any ongoing fetches for the given caller.
void httpfetch_caller_free(unsigned long caller);
void httpfetch_caller_free(u64 caller);

// Performs a synchronous HTTP request. This blocks and therefore should
// only be used from background threads.
Expand Down
1 change: 1 addition & 0 deletions src/serverlist.cpp
Expand Up @@ -97,6 +97,7 @@ void sendAnnounce(AnnounceAction action,
}

HTTPFetchRequest fetch_request;
fetch_request.caller = HTTPFETCH_PRINT_ERR;
fetch_request.url = g_settings->get("serverlist_url") + std::string("/announce");
fetch_request.method = HTTP_POST;
fetch_request.fields["json"] = fastWriteJson(server);
Expand Down
16 changes: 16 additions & 0 deletions src/util/string.cpp
Expand Up @@ -887,3 +887,19 @@ std::string sanitizeDirName(const std::string &str, const std::string &optional_

return wide_to_utf8(safe_name);
}


void safe_print_string(std::ostream &os, const std::string &str)
{
std::ostream::fmtflags flags = os.flags();
os << std::hex;
for (const char c : str) {
if (IS_ASCII_PRINTABLE_CHAR(c) || IS_UTF8_MULTB_START(c) ||
IS_UTF8_MULTB_INNER(c) || c == '\n' || c == '\t') {
os << c;
} else {
os << '<' << std::setw(2) << (int)c << '>';
}
}
os.setf(flags);
}
8 changes: 8 additions & 0 deletions src/util/string.h
Expand Up @@ -753,3 +753,11 @@ inline irr::core::stringw utf8_to_stringw(const std::string &input)
* 2. Remove 'unsafe' characters from the name by replacing them with '_'
*/
std::string sanitizeDirName(const std::string &str, const std::string &optional_prefix);

/**
* Prints a sanitized version of a string without control characters.
* '\t' and '\n' are allowed, as are UTF-8 control characters (e.g. RTL).
* ASCII control characters are replaced with their hex encoding in angle
* brackets (e.g. "a\x1eb" -> "a<1e>b").
*/
void safe_print_string(std::ostream &os, const std::string &str);

0 comments on commit 29d2b2c

Please sign in to comment.