highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-09-15 15:58:11 +02:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Make HTTPRequest fields private

Browse Source 
Makes sense since they are only set by methods in the class itself, and already had accessors for most fields.
This commit is contained in:
Hodlinator
2026-08-24 11:25:31 +02:00
parent b8cd77237b
commit 6fec8d6914
3 changed files with 55 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/httpserver.cpp
View File

@@ -1064,8 +1064,8 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr<HTTPRemot
LogDebug(
BCLog::HTTP,
"Received a %s request for %s from %s (id=%llu)",
RequestMethodString(client->m_req->m_method),
client->m_req->m_target,
RequestMethodString(client->m_req->GetRequestMethod()),
client->m_req->GetURI(),
client->m_origin,
client->m_id);

31
src/httpserver.h
View File

@@ -150,18 +150,6 @@ class HTTPRemoteClient;
class HTTPRequest
{
public:
HTTPRequestMethod m_method;
std::string m_target;
HTTPVersion m_version;
HTTPHeaders m_headers;
std::string m_body;
//! Pointer to the client that made the request so we know who to respond to.
std::weak_ptr<HTTPRemoteClient> m_client;
//! Response headers may be set in advance before response body is known
HTTPHeaders m_response_headers;
explicit HTTPRequest(const std::shared_ptr<HTTPRemoteClient>& client) : m_client{client} {}
//! Construct with a null client for unit tests
explicit HTTPRequest() : m_client{} {}
@@ -186,6 +174,9 @@ public:
WriteReply(status, std::as_bytes(std::span{reply_body_view}));
}
const HTTPVersion& GetVersion() const { return m_version; }
std::shared_ptr<HTTPRemoteClient> GetClient() const { return m_client.lock(); }
// These methods reimplement the API from http_libevent::HTTPRequest
// for downstream JSONRPC and REST modules.
std::string GetURI() const { return m_target; }
@@ -195,6 +186,8 @@ public:
std::optional<std::string> GetHeader(std::string_view hdr) const;
std::string ReadBody() const { return m_body; }
void WriteHeader(std::string&& hdr, std::string&& value);
std::optional<uint64_t> GetChunkSize() const { return m_chunk_size; }
uint64_t GetChunkProgress() const { return m_chunk_read; }
enum class State {
Init,
@@ -206,6 +199,19 @@ public:
State GetState() const { return m_state; }
void SetState(State state) { m_state = state; }
private:
HTTPRequestMethod m_method;
std::string m_target;
HTTPVersion m_version;
HTTPHeaders m_headers;
std::string m_body;
//! Pointer to the client that made the request so we know who to respond to.
std::weak_ptr<HTTPRemoteClient> m_client;
//! Response headers may be set in advance before response body is known
HTTPHeaders m_response_headers;
// If a large request is sent with "Transfer-encoding: chunked" we may
// read the chunk size in a separate I/O loop iteration than the chunk
// of data itself. Store the chunk size value here until the chunk is read.
@@ -214,7 +220,6 @@ public:
// Track the progress of the chunk here.
uint64_t m_chunk_read{0};
private:
State m_state = State::Init;
};

73
src/test/httpserver_tests.cpp
View File

@@ -206,19 +206,16 @@ BOOST_AUTO_TEST_CASE(http_request_tests)
BOOST_CHECK(req.LoadControlData(reader));
BOOST_CHECK(req.LoadHeaders(reader));
BOOST_CHECK(req.LoadBody(reader));
BOOST_CHECK_EQUAL(req.m_method, HTTPRequestMethod::POST);
BOOST_CHECK_EQUAL(req.GetRequestMethod(), HTTPRequestMethod::POST);
BOOST_CHECK_EQUAL(req.m_target, "/");
BOOST_CHECK_EQUAL(req.GetURI(), "/");
BOOST_CHECK_EQUAL(req.m_version.major, 1);
BOOST_CHECK_EQUAL(req.m_version.minor, 1);
BOOST_CHECK_EQUAL(req.GetVersion().major, 1);
BOOST_CHECK_EQUAL(req.GetVersion().minor, 1);
BOOST_CHECK_EQUAL(req.GetHeader("Host"), "127.0.0.1");
BOOST_CHECK_EQUAL(req.GetHeader("Connection"), "close");
BOOST_CHECK_EQUAL(req.GetHeader("Content-Type"), "application/json");
BOOST_CHECK_EQUAL(req.GetHeader("Authorization"), "Basic X19jb29raWVfXzo5OGQ5ODQ3MWNmNjg0NzAzYTkzN2EzNzk0ZDFlODQ1NjZmYTRkZjJiMzFkYjhhODI4ZGY4MjVjOTg5ZGI4OTVl");
BOOST_CHECK_EQUAL(req.GetHeader("Content-Length"), "46");
BOOST_CHECK_EQUAL(req.m_body.size(), 46);
BOOST_CHECK_EQUAL(req.m_body, R"({"method":"getblockcount","params":[],"id":1})""\n");
BOOST_CHECK_EQUAL(req.ReadBody(), R"({"method":"getblockcount","params":[],"id":1})""\n");
}
{
// Malformed: no spaces between data
@@ -313,13 +310,13 @@ BOOST_AUTO_TEST_CASE(http_request_tests)
BOOST_CHECK(req.LoadControlData(reader));
BOOST_CHECK(req.LoadHeaders(reader));
BOOST_CHECK(req.LoadBody(reader));
BOOST_CHECK_EQUAL(req.m_method, HTTPRequestMethod::GET);
BOOST_CHECK_EQUAL(req.m_target, "/");
BOOST_CHECK_EQUAL(req.m_version.major, 1);
BOOST_CHECK_EQUAL(req.m_version.minor, 0);
BOOST_CHECK_EQUAL(req.GetRequestMethod(), HTTPRequestMethod::GET);
BOOST_CHECK_EQUAL(req.GetURI(), "/");
BOOST_CHECK_EQUAL(req.GetVersion().major, 1);
BOOST_CHECK_EQUAL(req.GetVersion().minor, 0);
BOOST_CHECK_EQUAL(req.GetHeader("Host"), "127.0.0.1");
// no body is OK
BOOST_CHECK_EQUAL(req.m_body.size(), 0);
BOOST_CHECK_EQUAL(req.ReadBody(), "");
}
{
// Malformed: missing colon
@@ -345,7 +342,7 @@ BOOST_AUTO_TEST_CASE(http_request_tests)
BOOST_CHECK(req.LoadHeaders(reader));
BOOST_CHECK(req.LoadBody(reader));
// Don't try to read request body if Content-Length is missing
BOOST_CHECK_EQUAL(req.m_body.size(), 0);
BOOST_CHECK_EQUAL(req.ReadBody(), "");
}
{
// Malformed: Content-Length is not a number
@@ -409,7 +406,7 @@ BOOST_AUTO_TEST_CASE(http_request_tests)
BOOST_CHECK(req.LoadControlData(reader));
BOOST_CHECK(req.LoadHeaders(reader));
BOOST_CHECK(req.LoadBody(reader));
BOOST_CHECK_EQUAL(req.m_body, R"({"method":"getblockcount"})");
BOOST_CHECK_EQUAL(req.ReadBody(), R"({"method":"getblockcount"})");
}
{
// Prevent "chunked" transfer from exceeding size limit
@@ -445,7 +442,7 @@ BOOST_AUTO_TEST_CASE(http_request_tests)
BOOST_CHECK(req.LoadControlData(reader));
BOOST_CHECK(req.LoadHeaders(reader));
BOOST_CHECK(req.LoadBody(reader));
BOOST_CHECK_EQUAL(req.m_body, R"({"method":"getblockcount"})");
BOOST_CHECK_EQUAL(req.ReadBody(), R"({"method":"getblockcount"})");
// Chunk Trailer was parsed, but ignored
BOOST_CHECK_EQUAL(reader.Remaining(), 0);
BOOST_CHECK(!req.GetHeader("Expires"));
@@ -541,7 +538,7 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
"GET /endpoint HTTP/1.0\n\n");
client->ReadRequest(*client->m_req);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete);
BOOST_CHECK_EQUAL(client->m_req->m_body, "I miss you");
BOOST_CHECK_EQUAL(client->m_req->ReadBody(), "I miss you");
// Next request sitting in buffer
BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 24);
// Complete first request hasn't been moved yet, expect no-op
@@ -555,8 +552,8 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
// Read second request
client->ReadRequest(*client->m_req);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete);
BOOST_CHECK_EQUAL(client->m_req->m_target, "/endpoint");
BOOST_CHECK_EQUAL(client->m_req->m_body.size(), 0);
BOOST_CHECK_EQUAL(client->m_req->GetURI(), "/endpoint");
BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 0);
// Buffer is cleared
BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 0);
}
@@ -579,7 +576,7 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
client->receive(std::string(10000, 'x'));
BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 10000);
client->ReadRequest(*client->m_req);
BOOST_CHECK_EQUAL(client->m_req->m_body.size(), 10000 * i);
BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 10000 * i);
BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 0);
}
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete);
@@ -596,7 +593,7 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
"GET /next HTTP/1.0\n\n");
client->ReadRequest(*client->m_req);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete);
BOOST_CHECK_EQUAL(client->m_req->m_body, "body");
BOOST_CHECK_EQUAL(client->m_req->ReadBody(), "body");
// Only the second request is left over
BOOST_CHECK_EQUAL(client->m_recv_buffer.size(), 20);
}
@@ -605,9 +602,9 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
std::shared_ptr<DummyClient> client{std::make_shared<DummyClient>()};
client->m_req = std::make_unique<HTTPRequest>(client);
BOOST_CHECK(!client->m_req->m_chunk_size);
BOOST_CHECK_EQUAL(client->m_req->m_chunk_read, 0);
BOOST_CHECK_EQUAL(client->m_req->m_body.size(), 0);
BOOST_CHECK(!client->m_req->GetChunkSize());
BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 0);
BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 0);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Init);
// First chunk is incomplete
@@ -617,40 +614,40 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
"10\n"
R"({"method)");
client->ReadRequest(*client->m_req);
BOOST_CHECK(client->m_req->m_chunk_size);
BOOST_CHECK_EQUAL(*client->m_req->m_chunk_size, 16);
BOOST_CHECK_EQUAL(client->m_req->m_chunk_read, 8);
BOOST_CHECK_EQUAL(client->m_req->m_body.size(), 8);
BOOST_CHECK(client->m_req->GetChunkSize());
BOOST_CHECK_EQUAL(*client->m_req->GetChunkSize(), 16);
BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 8);
BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 8);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody);
// More data arrives, chunk is completed.
client->receive(R"(":"getbl)""\n");
client->ReadRequest(*client->m_req);
// State is reset
BOOST_CHECK(!client->m_req->m_chunk_size);
BOOST_CHECK_EQUAL(client->m_req->m_chunk_read, 0);
BOOST_CHECK(!client->m_req->GetChunkSize());
BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 0);
// New data is added to body but body is still incomplete
BOOST_CHECK_EQUAL(client->m_req->m_body.size(), 16);
BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 16);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody);
// Next chunk arrives without terminal CRLF
client->receive("a\n"
R"(ockcount"})");
client->ReadRequest(*client->m_req);
BOOST_CHECK(client->m_req->m_chunk_size);
BOOST_CHECK_EQUAL(*client->m_req->m_chunk_size, 10);
BOOST_CHECK_EQUAL(client->m_req->m_chunk_read, 10);
BOOST_CHECK_EQUAL(client->m_req->m_body.size(), 26);
BOOST_CHECK(client->m_req->GetChunkSize());
BOOST_CHECK_EQUAL(*client->m_req->GetChunkSize(), 10);
BOOST_CHECK_EQUAL(client->m_req->GetChunkProgress(), 10);
BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 26);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::NeedsBody);
// Chunk terminal CRLF arrives with final (size 0) chunk
client->receive("\n0\n\n");
client->ReadRequest(*client->m_req);
// Body size hasn't changed
BOOST_CHECK_EQUAL(client->m_req->m_body.size(), 26);
BOOST_CHECK_EQUAL(client->m_req->ReadBody().size(), 26);
// We're done
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete);
BOOST_CHECK_EQUAL(client->m_req->m_body, R"({"method":"getblockcount"})");
BOOST_CHECK_EQUAL(client->m_req->ReadBody(), R"({"method":"getblockcount"})");
}
{
// Invalid headers: error state stops reading
@@ -772,7 +769,7 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
client->receive("\n");
client->ReadRequest(*client->m_req);
BOOST_CHECK_EQUAL(client->m_req->GetState(), HTTPRequest::State::Complete);
BOOST_CHECK_EQUAL(client->m_req->m_body, "x");
BOOST_CHECK_EQUAL(client->m_req->ReadBody(), "x");
}
{
// Ensure chunk trailer counts towards the headers size limit
@@ -868,11 +865,11 @@ BOOST_AUTO_TEST_CASE(http_server_socket_tests)
// Connected client should have one request already from the static content.
if (requests.size() == 1) {
// Check the received request
BOOST_CHECK_EQUAL(requests.front()->m_body, R"({"method":"getblockcount","params":[],"id":1})""\n");
BOOST_CHECK_EQUAL(requests.front()->ReadBody(), R"({"method":"getblockcount","params":[],"id":1})""\n");
BOOST_CHECK_EQUAL(requests.front()->GetPeer().ToStringAddrPort(), "5.5.5.5:6789");
// Inspect the connection pointed to from the request
client = requests.front()->m_client.lock();
client = requests.front()->GetClient();
BOOST_REQUIRE(client);
BOOST_CHECK_EQUAL(client->m_origin, "5.5.5.5:6789");