Skip to content

Commit 29d2b2c

Browse files
ShadowNinjaShadowNinja
authoredJan 1, 2022
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
·
5.11.05.5.0
1 parent 544b9d5 commit 29d2b2c

File tree

6 files changed

+73
-36
lines changed

6 files changed

+73
-36
lines changed
 

Diff for: ‎src/client/clientmedia.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,12 @@ class ClientMediaDownloader : public IClientMediaDownloader
174174
s32 m_uncached_received_count = 0;
175175

176176
// Status of remote transfers
177-
unsigned long m_httpfetch_caller;
178-
unsigned long m_httpfetch_next_id = 0;
177+
u64 m_httpfetch_caller;
178+
u64 m_httpfetch_next_id = 0;
179179
s32 m_httpfetch_active = 0;
180180
s32 m_httpfetch_active_limit = 0;
181181
s32 m_outstanding_hash_sets = 0;
182-
std::unordered_map<unsigned long, std::string> m_remote_file_transfers;
182+
std::unordered_map<u64, std::string> m_remote_file_transfers;
183183

184184
// All files up to this name have either been received from a
185185
// remote server or failed on all remote servers, so those files

Diff for: ‎src/httpfetch.cpp

+28-23
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
3838
#include "noise.h"
3939

4040
static std::mutex g_httpfetch_mutex;
41-
static std::unordered_map<unsigned long, std::queue<HTTPFetchResult>>
41+
static std::unordered_map<u64, std::queue<HTTPFetchResult>>
4242
g_httpfetch_results;
4343
static PcgRandom g_callerid_randomness;
4444

@@ -52,22 +52,21 @@ HTTPFetchRequest::HTTPFetchRequest() :
5252

5353
static void httpfetch_deliver_result(const HTTPFetchResult &fetch_result)
5454
{
55-
unsigned long caller = fetch_result.caller;
55+
u64 caller = fetch_result.caller;
5656
if (caller != HTTPFETCH_DISCARD) {
5757
MutexAutoLock lock(g_httpfetch_mutex);
5858
g_httpfetch_results[caller].emplace(fetch_result);
5959
}
6060
}
6161

62-
static void httpfetch_request_clear(unsigned long caller);
62+
static void httpfetch_request_clear(u64 caller);
6363

64-
unsigned long httpfetch_caller_alloc()
64+
u64 httpfetch_caller_alloc()
6565
{
6666
MutexAutoLock lock(g_httpfetch_mutex);
6767

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

8180
FATAL_ERROR("httpfetch_caller_alloc: ran out of caller IDs");
82-
return discard;
8381
}
8482

85-
unsigned long httpfetch_caller_alloc_secure()
83+
u64 httpfetch_caller_alloc_secure()
8684
{
8785
MutexAutoLock lock(g_httpfetch_mutex);
8886

8987
// Generate random caller IDs and make sure they're not
90-
// already used or equal to HTTPFETCH_DISCARD
88+
// already used or reserved.
9189
// Give up after 100 tries to prevent infinite loop
92-
u8 tries = 100;
93-
unsigned long caller;
90+
size_t tries = 100;
91+
u64 caller;
9492

9593
do {
9694
caller = (((u64) g_callerid_randomness.next()) << 32) |
@@ -100,7 +98,8 @@ unsigned long httpfetch_caller_alloc_secure()
10098
FATAL_ERROR("httpfetch_caller_alloc_secure: ran out of caller IDs");
10199
return HTTPFETCH_DISCARD;
102100
}
103-
} while (g_httpfetch_results.find(caller) != g_httpfetch_results.end());
101+
} while (caller >= HTTPFETCH_CID_START &&
102+
g_httpfetch_results.find(caller) != g_httpfetch_results.end());
104103

105104
verbosestream << "httpfetch_caller_alloc_secure: allocating "
106105
<< caller << std::endl;
@@ -110,7 +109,7 @@ unsigned long httpfetch_caller_alloc_secure()
110109
return caller;
111110
}
112111

113-
void httpfetch_caller_free(unsigned long caller)
112+
void httpfetch_caller_free(u64 caller)
114113
{
115114
verbosestream<<"httpfetch_caller_free: freeing "
116115
<<caller<<std::endl;
@@ -122,7 +121,7 @@ void httpfetch_caller_free(unsigned long caller)
122121
}
123122
}
124123

125-
bool httpfetch_async_get(unsigned long caller, HTTPFetchResult &fetch_result)
124+
bool httpfetch_async_get(u64 caller, HTTPFetchResult &fetch_result)
126125
{
127126
MutexAutoLock lock(g_httpfetch_mutex);
128127

@@ -242,7 +241,6 @@ HTTPFetchOngoing::HTTPFetchOngoing(const HTTPFetchRequest &request_,
242241

243242
// Set static cURL options
244243
curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1);
245-
curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1);
246244
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
247245
curl_easy_setopt(curl, CURLOPT_MAXREDIRS, 3);
248246
curl_easy_setopt(curl, CURLOPT_ENCODING, "gzip");
@@ -393,10 +391,17 @@ const HTTPFetchResult * HTTPFetchOngoing::complete(CURLcode res)
393391
}
394392

395393
if (res != CURLE_OK) {
396-
errorstream << request.url << " not found ("
397-
<< curl_easy_strerror(res) << ")"
398-
<< " (response code " << result.response_code << ")"
394+
errorstream << "HTTPFetch for " << request.url << " failed ("
395+
<< curl_easy_strerror(res) << ")" << std::endl;
396+
} else if (result.response_code >= 400) {
397+
errorstream << "HTTPFetch for " << request.url
398+
<< " returned response code " << result.response_code
399399
<< std::endl;
400+
if (result.caller == HTTPFETCH_PRINT_ERR && !result.data.empty()) {
401+
errorstream << "Response body:" << std::endl;
402+
safe_print_string(errorstream, result.data);
403+
errorstream << std::endl;
404+
}
400405
}
401406

402407
return &result;
@@ -474,7 +479,7 @@ class CurlFetchThread : public Thread
474479
m_requests.push_back(req);
475480
}
476481

477-
void requestClear(unsigned long caller, Event *event)
482+
void requestClear(u64 caller, Event *event)
478483
{
479484
Request req;
480485
req.type = RT_CLEAR;
@@ -505,7 +510,7 @@ class CurlFetchThread : public Thread
505510

506511
}
507512
else if (req.type == RT_CLEAR) {
508-
unsigned long caller = req.fetch_request.caller;
513+
u64 caller = req.fetch_request.caller;
509514

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

781-
static void httpfetch_request_clear(unsigned long caller)
786+
static void httpfetch_request_clear(u64 caller)
782787
{
783788
if (g_httpfetch_thread->isRunning()) {
784789
Event event;
@@ -827,7 +832,7 @@ void httpfetch_async(const HTTPFetchRequest &fetch_request)
827832
httpfetch_deliver_result(fetch_result);
828833
}
829834

830-
static void httpfetch_request_clear(unsigned long caller)
835+
static void httpfetch_request_clear(u64 caller)
831836
{
832837
}
833838

Diff for: ‎src/httpfetch.h

+17-10
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,17 @@ with this program; if not, write to the Free Software Foundation, Inc.,
2323
#include "util/string.h"
2424
#include "config.h"
2525

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

3138
// Methods
3239
enum HttpMethod : u8
@@ -43,11 +50,11 @@ struct HTTPFetchRequest
4350

4451
// Identifies the caller (for asynchronous requests)
4552
// Ignored by httpfetch_sync
46-
unsigned long caller = HTTPFETCH_DISCARD;
53+
u64 caller = HTTPFETCH_DISCARD;
4754

4855
// Some number that identifies the request
4956
// (when the same caller issues multiple httpfetch_async calls)
50-
unsigned long request_id = 0;
57+
u64 request_id = 0;
5158

5259
// Timeout for the whole transfer, in milliseconds
5360
long timeout;
@@ -85,8 +92,8 @@ struct HTTPFetchResult
8592
long response_code = 0;
8693
std::string data = "";
8794
// The caller and request_id from the corresponding HTTPFetchRequest.
88-
unsigned long caller = HTTPFETCH_DISCARD;
89-
unsigned long request_id = 0;
95+
u64 caller = HTTPFETCH_DISCARD;
96+
u64 request_id = 0;
9097

9198
HTTPFetchResult() = default;
9299

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

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

112119
// Allocates a caller ID for httpfetch_async
113120
// Not required if you want to set caller = HTTPFETCH_DISCARD
114-
unsigned long httpfetch_caller_alloc();
121+
u64 httpfetch_caller_alloc();
115122

116123
// Allocates a non-predictable caller ID for httpfetch_async
117-
unsigned long httpfetch_caller_alloc_secure();
124+
u64 httpfetch_caller_alloc_secure();
118125

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

124131
// Performs a synchronous HTTP request. This blocks and therefore should
125132
// only be used from background threads.

Diff for: ‎src/serverlist.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ void sendAnnounce(AnnounceAction action,
9797
}
9898

9999
HTTPFetchRequest fetch_request;
100+
fetch_request.caller = HTTPFETCH_PRINT_ERR;
100101
fetch_request.url = g_settings->get("serverlist_url") + std::string("/announce");
101102
fetch_request.method = HTTP_POST;
102103
fetch_request.fields["json"] = fastWriteJson(server);

Diff for: ‎src/util/string.cpp

+16
Original file line numberDiff line numberDiff line change
@@ -887,3 +887,19 @@ std::string sanitizeDirName(const std::string &str, const std::string &optional_
887887

888888
return wide_to_utf8(safe_name);
889889
}
890+
891+
892+
void safe_print_string(std::ostream &os, const std::string &str)
893+
{
894+
std::ostream::fmtflags flags = os.flags();
895+
os << std::hex;
896+
for (const char c : str) {
897+
if (IS_ASCII_PRINTABLE_CHAR(c) || IS_UTF8_MULTB_START(c) ||
898+
IS_UTF8_MULTB_INNER(c) || c == '\n' || c == '\t') {
899+
os << c;
900+
} else {
901+
os << '<' << std::setw(2) << (int)c << '>';
902+
}
903+
}
904+
os.setf(flags);
905+
}

Diff for: ‎src/util/string.h

+8
Original file line numberDiff line numberDiff line change
@@ -753,3 +753,11 @@ inline irr::core::stringw utf8_to_stringw(const std::string &input)
753753
* 2. Remove 'unsafe' characters from the name by replacing them with '_'
754754
*/
755755
std::string sanitizeDirName(const std::string &str, const std::string &optional_prefix);
756+
757+
/**
758+
* Prints a sanitized version of a string without control characters.
759+
* '\t' and '\n' are allowed, as are UTF-8 control characters (e.g. RTL).
760+
* ASCII control characters are replaced with their hex encoding in angle
761+
* brackets (e.g. "a\x1eb" -> "a<1e>b").
762+
*/
763+
void safe_print_string(std::ostream &os, const std::string &str);

0 commit comments

Comments
 (0)
Please sign in to comment.