Change HTTPCallback::OnReceiveData to use UniqueBuffer

See: https://github.com/OpenTTD/OpenTTD/issues/11636
This commit is contained in:
Jonathan G Rennison 2023-12-30 19:51:58 +00:00
parent f457f306ce
commit f8085683fb
8 changed files with 23 additions and 37 deletions

View File

@ -13,6 +13,7 @@
#define NETWORK_CORE_HTTP_H
#include "tcp.h"
#include "../../src/core/alloc_type.hpp"
constexpr int HTTP_429_TOO_MANY_REQUESTS = 429;
@ -26,11 +27,11 @@ struct HTTPCallback {
/**
* We're receiving data.
* @param data the received data, nullptr when all data has been received. The implementation is responsible for freeing the data.
* @param data the received data, nullptr when all data has been received.
* @param length the amount of received data, 0 when all data has been received.
* @note When nullptr is sent the HTTP socket handler is closed/freed.
*/
virtual void OnReceiveData(const char *data, size_t length) = 0;
virtual void OnReceiveData(UniqueBuffer<char> data) = 0;
/**
* Check if there is a request to cancel the transfer.

View File

@ -195,11 +195,11 @@ void HttpThread()
HTTPThreadSafeCallback *callback = static_cast<HTTPThreadSafeCallback *>(userdata);
/* Copy the buffer out of CURL. OnReceiveData() will free it when done. */
char *buffer = size * nmemb == 0 ? nullptr : MallocT<char>(size * nmemb);
UniqueBuffer<char> buffer(size * nmemb);
if (buffer != nullptr) {
memcpy(buffer, ptr, size * nmemb);
memcpy(buffer.get(), ptr, size * nmemb);
}
callback->OnReceiveData(buffer, size * nmemb);
callback->OnReceiveData(std::move(buffer));
return size * nmemb;
});
@ -223,7 +223,7 @@ void HttpThread()
if (res == CURLE_OK) {
Debug(net, 1, "HTTP request succeeded");
request->callback.OnReceiveData(nullptr, 0);
request->callback.OnReceiveData({});
} else {
long status_code = 0;
curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &status_code);

View File

@ -24,11 +24,10 @@ private:
/** Entries on the queue for later handling. */
class Callback {
public:
Callback(const char *data, size_t length) : data(data), length(length), failure(false) {}
Callback() : data(nullptr), length(0), failure(true) {}
Callback(UniqueBuffer<char> data) : data(std::move(data)), failure(false) {}
Callback() : data({}), failure(true) {}
const char *data;
size_t length;
UniqueBuffer<char> data;
bool failure;
};
@ -45,10 +44,10 @@ public:
/**
* Similar to HTTPCallback::OnReceiveData, but thread-safe.
*/
void OnReceiveData(const char *data, size_t length)
void OnReceiveData(UniqueBuffer<char> data)
{
std::lock_guard<std::mutex> lock(this->mutex);
this->queue.emplace_back(data, length);
this->queue.emplace_back(std::move(data));
}
/**
@ -66,7 +65,7 @@ public:
if (item.failure) {
this->callback->OnFailure();
} else {
this->callback->OnReceiveData(item.data, item.length);
this->callback->OnReceiveData(std::move(item.data));
}
}
@ -101,13 +100,6 @@ public:
{
std::lock_guard<std::mutex> lock(this->mutex);
/* Free all data that was not handled. */
for (auto &item : this->queue) {
if (!item.failure) {
free(item.data);
}
}
queue.clear();
queue_cv.notify_all();
}

View File

@ -159,14 +159,14 @@ void NetworkHTTPRequest::WinHttpCallback(DWORD code, void *info, DWORD length)
/* Next step: read the data in a temporary allocated buffer.
* The buffer will be free'd by OnReceiveData() in the next step. */
char *buffer = size == 0 ? nullptr : MallocT<char>(size);
char *buffer = size == 0 ? nullptr : new char[size];
WinHttpReadData(this->request, buffer, size, 0);
} break;
case WINHTTP_CALLBACK_STATUS_READ_COMPLETE:
Debug(net, 4, "HTTP callback: {} bytes", length);
this->callback.OnReceiveData(static_cast<char *>(info), length);
this->callback.OnReceiveData(UniqueBuffer<char>(std::unique_ptr<char[]>(static_cast<char *>(info)), length));
if (length == 0) {
/* Next step: no more data available: request is finished. */

View File

@ -608,23 +608,19 @@ void ClientNetworkContentSocketHandler::OnFailure()
}
}
void ClientNetworkContentSocketHandler::OnReceiveData(const char *data, size_t length)
void ClientNetworkContentSocketHandler::OnReceiveData(UniqueBuffer<char> data)
{
assert(data == nullptr || length != 0);
assert(data.get() == nullptr || data.size() != 0);
/* Ignore any latent data coming from a connection we closed. */
if (this->http_response_index == -2) {
if (data != nullptr) {
free(data);
}
return;
}
if (this->http_response_index == -1) {
if (data != nullptr) {
/* Append the rest of the response. */
this->http_response.insert(this->http_response.end(), data, data + length);
free(data);
this->http_response.insert(this->http_response.end(), data.get(), data.get() + data.size());
return;
} else {
/* Make sure the response is properly terminated. */
@ -637,16 +633,15 @@ void ClientNetworkContentSocketHandler::OnReceiveData(const char *data, size_t l
if (data != nullptr) {
/* We have data, so write it to the file. */
if (fwrite(data, 1, length, this->curFile) != length) {
if (fwrite(data.get(), 1, data.size(), this->curFile) != data.size()) {
/* Writing failed somehow, let try via the old method. */
this->OnFailure();
} else {
/* Just received the data. */
this->OnDownloadProgress(this->curInfo, (int)length);
this->OnDownloadProgress(this->curInfo, (int)data.size());
}
/* Nothing more to do now. */
free(data);
return;
}

View File

@ -97,7 +97,7 @@ protected:
void OnDownloadComplete(ContentID cid) override;
void OnFailure() override;
void OnReceiveData(const char *data, size_t length) override;
void OnReceiveData(UniqueBuffer<char> data) override;
bool IsCancelled() const override;
bool BeforeDownload();

View File

@ -389,12 +389,10 @@ void NetworkSurveyHandler::OnFailure()
this->loaded.notify_all();
}
void NetworkSurveyHandler::OnReceiveData(const char *data, size_t)
void NetworkSurveyHandler::OnReceiveData(UniqueBuffer<char> data)
{
if (data == nullptr) {
Debug(net, 1, "Survey: survey results sent");
this->loaded.notify_all();
} else {
free(data);
}
}

View File

@ -20,7 +20,7 @@
class NetworkSurveyHandler : public HTTPCallback {
protected:
void OnFailure() override;
void OnReceiveData(const char *data, size_t length) override;
void OnReceiveData(UniqueBuffer<char> data) override;
bool IsCancelled() const override { return false; }
public: