From 103a86fdd49ed6f943a5442e4965e9f777a9fb9a Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 09:24:51 -0800 Subject: [PATCH 01/20] Start hacking everything up. --- BedrockCommand.cpp | 13 +++++++++++++ BedrockCommand.h | 3 +++ BedrockServer.cpp | 8 ++++++++ libstuff/SHTTPSManager.cpp | 9 --------- libstuff/SHTTPSManager.h | 9 --------- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 931874684..86258cf54 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -303,3 +303,16 @@ void BedrockCommand::setTimeout(uint64_t timeoutDurationMS) { bool BedrockCommand::shouldCommitEmptyTransactions() const { return _commitEmptyTransactions; } + +void BedrockCommand::addHTTPSRequests(const string& serializedHTTPSRequests) { + + if (serializedHTTPSRequests.empty()) { + return; + } + + list requests = SParseJSONArray(serializedHTTPSRequests); + for (const string& requestStr : requests) { + STable request = SParseJSONObject(requestStr); + + } +} diff --git a/BedrockCommand.h b/BedrockCommand.h index bcc5ad3f5..8cd0df04f 100644 --- a/BedrockCommand.h +++ b/BedrockCommand.h @@ -80,6 +80,9 @@ class BedrockCommand : public SQLiteCommand { // Return the name of the plugin for this command. const string& getName() const; + // Take a serialized list of HTTPS requests and apply it to the command. + void addHTTPSRequests(const string& serializedHTTPSRequests); + // Bedrock will call this before each `processCommand` (note: not `peekCommand`) for each plugin to allow it to // enable query rewriting. If a plugin would like to enable query rewriting, this should return true, and it should // set the rewriteHandler it would like to use. diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 18c6239fa..5b5e9cf44 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2170,8 +2170,16 @@ unique_ptr BedrockServer::buildCommandFromRequest(SData&& reques request["_source"] = ip; } + // Pull any serialized https requests off the requests object to apply tothe command. + string serializedHTTPSRequests = request["httpsRequests"]; + request.erase("httpsRequests"); + // Create a command. unique_ptr command = getCommandFromPlugins(move(request)); + + // Apply HTTPS requests. + command->addHTTPSRequests(serializedHTTPSRequests); + SDEBUG("Deserialized command " << command->request.methodLine); command->socket = fireAndForget ? nullptr : &socket; diff --git a/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index 9f3fbe349..3223e37f4 100644 --- a/libstuff/SHTTPSManager.cpp +++ b/libstuff/SHTTPSManager.cpp @@ -17,13 +17,6 @@ SHTTPSManager::SHTTPSManager(BedrockPlugin& plugin_, const string& pem, const st plugin.httpsManagers.push_back(this); } -void SHTTPSManager::validate() { - // These can only be created on a leader node. - if (plugin.server.getState() != SQLiteNodeState::LEADING && plugin.server.getState() != SQLiteNodeState::STANDINGDOWN) { - throw NotLeading(); - } -} - SStandaloneHTTPSManager::SStandaloneHTTPSManager() { } @@ -143,7 +136,6 @@ SStandaloneHTTPSManager::Transaction::Transaction(SStandaloneHTTPSManager& manag sentTime(0), requestID(SThreadLogPrefix) { - manager.validate(); } SStandaloneHTTPSManager::Transaction::~Transaction() { @@ -171,7 +163,6 @@ SStandaloneHTTPSManager::Transaction* SStandaloneHTTPSManager::_httpsSend(const host += ":443"; } - // Create a new transaction. This can throw if `validate` fails. We explicitly do this *before* creating a socket. Transaction* transaction = new Transaction(*this); // If this is going to be an https transaction, create a certificate and give it to the socket. diff --git a/libstuff/SHTTPSManager.h b/libstuff/SHTTPSManager.h index 9d2a1d1a3..48db3a870 100644 --- a/libstuff/SHTTPSManager.h +++ b/libstuff/SHTTPSManager.h @@ -42,13 +42,6 @@ class SStandaloneHTTPSManager : public STCPManager { static int getHTTPResponseCode(const string& methodLine); - virtual void validate() { - // The constructor for a transaction needs to call this on it's manager. It can then throw in cases where this - // manager should not be allowed to create transactions. This lets us have different validation behavior for - // different managers without needing to subclass Transaction. - // The default implementation does nothing. - } - protected: // Child API // Used to create the signing certificate. @@ -76,8 +69,6 @@ class SHTTPSManager : public SStandaloneHTTPSManager { } }; - virtual void validate() override; - protected: // Reference to the plugin that owns this object. BedrockPlugin& plugin; From 55d92880ea5966b99d7559c1e9b09df5fc9459f1 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 09:32:47 -0800 Subject: [PATCH 02/20] remove some cruft --- BedrockPlugin.cpp | 3 --- BedrockPlugin.h | 4 ---- libstuff/SHTTPSManager.cpp | 2 -- libstuff/SHTTPSManager.h | 1 + 4 files changed, 1 insertion(+), 9 deletions(-) diff --git a/BedrockPlugin.cpp b/BedrockPlugin.cpp index 007261550..007e568d1 100644 --- a/BedrockPlugin.cpp +++ b/BedrockPlugin.cpp @@ -8,9 +8,6 @@ BedrockPlugin::BedrockPlugin(BedrockServer& s) : server(s) { } BedrockPlugin::~BedrockPlugin() { - for (auto httpsManager : httpsManagers) { - delete httpsManager; - } } bool BedrockPlugin::isValidDate(const string& date) diff --git a/BedrockPlugin.h b/BedrockPlugin.h index 6fe838056..f81f5d289 100644 --- a/BedrockPlugin.h +++ b/BedrockPlugin.h @@ -38,10 +38,6 @@ class BedrockPlugin { // Called at some point during initiation to allow the plugin to verify/change the database schema. virtual void upgradeDatabase(SQLite& db); - // A list of SHTTPSManagers that the plugin would like the server to watch for activity. It is only guaranteed to - // be safe to modify this list during `initialize`. - list httpsManagers; - // The plugin can register any number of timers it wants. When any of them `ding`, then the `timerFired` // function will be called, and passed the timer that is dinging. set timers; diff --git a/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index 3223e37f4..9af93b428 100644 --- a/libstuff/SHTTPSManager.cpp +++ b/libstuff/SHTTPSManager.cpp @@ -8,13 +8,11 @@ SHTTPSManager::SHTTPSManager(BedrockPlugin& plugin_) : plugin(plugin_) { - plugin.httpsManagers.push_back(this); } SHTTPSManager::SHTTPSManager(BedrockPlugin& plugin_, const string& pem, const string& srvCrt, const string& caCrt) : SStandaloneHTTPSManager(pem, srvCrt, caCrt), plugin(plugin_) { - plugin.httpsManagers.push_back(this); } SStandaloneHTTPSManager::SStandaloneHTTPSManager() diff --git a/libstuff/SHTTPSManager.h b/libstuff/SHTTPSManager.h index 48db3a870..bab21c4b2 100644 --- a/libstuff/SHTTPSManager.h +++ b/libstuff/SHTTPSManager.h @@ -63,6 +63,7 @@ class SHTTPSManager : public SStandaloneHTTPSManager { SHTTPSManager(BedrockPlugin& plugin_); SHTTPSManager(BedrockPlugin& plugin_, const string& pem, const string& srvCrt, const string& caCrt); + // TODO: Remove this once AUth no longer checks for it. class NotLeading : public exception { const char* what() { return "Can't create SHTTPSManager::Transaction when not leading"; From 97aacfdf48032c5c00d3db4c07b4c80c948f17ce Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 10:01:08 -0800 Subject: [PATCH 03/20] There's a lot of obsolete junk in here. --- BedrockCommand.cpp | 2 +- BedrockCommand.h | 7 +++++-- BedrockServer.cpp | 2 +- libstuff/SHTTPSManager.cpp | 2 +- libstuff/SHTTPSManager.h | 3 --- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 86258cf54..22e6003c9 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -304,7 +304,7 @@ bool BedrockCommand::shouldCommitEmptyTransactions() const { return _commitEmptyTransactions; } -void BedrockCommand::addHTTPSRequests(const string& serializedHTTPSRequests) { +void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSRequests) { if (serializedHTTPSRequests.empty()) { return; diff --git a/BedrockCommand.h b/BedrockCommand.h index 8cd0df04f..860bac4ec 100644 --- a/BedrockCommand.h +++ b/BedrockCommand.h @@ -80,8 +80,11 @@ class BedrockCommand : public SQLiteCommand { // Return the name of the plugin for this command. const string& getName() const; - // Take a serialized list of HTTPS requests and apply it to the command. - void addHTTPSRequests(const string& serializedHTTPSRequests); + // Take all of the HTTPS requests attached to this object, and serialize them into an `httpsRequests` header field in the response. + void serializeHTTPSRequests(); + + // Take a serialized list of HTTPS requests as from `serializeHTTPSRequests` and deserialize them into the `httpsRequests` object. + void deserializeHTTPSRequests(const string& serializedHTTPSRequests); // Bedrock will call this before each `processCommand` (note: not `peekCommand`) for each plugin to allow it to // enable query rewriting. If a plugin would like to enable query rewriting, this should return true, and it should diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 5b5e9cf44..0eaca206a 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2178,7 +2178,7 @@ unique_ptr BedrockServer::buildCommandFromRequest(SData&& reques unique_ptr command = getCommandFromPlugins(move(request)); // Apply HTTPS requests. - command->addHTTPSRequests(serializedHTTPSRequests); + command->deserializeHTTPSRequests(serializedHTTPSRequests); SDEBUG("Deserialized command " << command->request.methodLine); command->socket = fireAndForget ? nullptr : &socket; diff --git a/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index 9af93b428..d4a04ffb1 100644 --- a/libstuff/SHTTPSManager.cpp +++ b/libstuff/SHTTPSManager.cpp @@ -93,7 +93,7 @@ void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Tra // content. Why this is the what constitutes a valid response is lost to time. Any well-formed response should // be valid here, and this should get cleaned up. However, this requires testing anything that might rely on // the existing behavior, which is an exercise for later. - if (handleAllResponses() || SContains(transaction.fullResponse.methodLine, " 200 ") || transaction.fullResponse.content.size()) { + if (SContains(transaction.fullResponse.methodLine, " 200 ") || transaction.fullResponse.content.size()) { // Pass the transaction down to the subclass. _onRecv(&transaction); } else { diff --git a/libstuff/SHTTPSManager.h b/libstuff/SHTTPSManager.h index bab21c4b2..6d2e1c3df 100644 --- a/libstuff/SHTTPSManager.h +++ b/libstuff/SHTTPSManager.h @@ -53,9 +53,6 @@ class SStandaloneHTTPSManager : public STCPManager { Transaction* _httpsSend(const string& url, const SData& request); Transaction* _createErrorTransaction(); virtual bool _onRecv(Transaction* transaction); - - // Historically we only call _onRecv for `200 OK` responses. This allows manangers to handle all responses. - virtual bool handleAllResponses() { return false; } }; class SHTTPSManager : public SStandaloneHTTPSManager { From f565a219763dd57b69d7dd7da2c053ec75683cc6 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 10:10:23 -0800 Subject: [PATCH 04/20] Add a default manager for deserialized https requests --- BedrockCommand.cpp | 2 ++ BedrockCommand.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 22e6003c9..45ebca746 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -4,6 +4,8 @@ atomic BedrockCommand::_commandCount(0); +SStandaloneHTTPSManager BedrockCommand::_noopHTTPSManager; + const string BedrockCommand::defaultPluginName("NO_PLUGIN"); BedrockCommand::BedrockCommand(SQLiteCommand&& baseCommand, BedrockPlugin* plugin, bool escalateImmediately_) : diff --git a/BedrockCommand.h b/BedrockCommand.h index 860bac4ec..4ff8bdb5a 100644 --- a/BedrockCommand.h +++ b/BedrockCommand.h @@ -237,5 +237,7 @@ class BedrockCommand : public SQLiteCommand { static atomic _commandCount; + static SStandaloneHTTPSManager _noopHTTPSManager; + static const string defaultPluginName; }; From e213a25251e0dc86d4f5230dca0dad07e2bb2385 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 10:32:06 -0800 Subject: [PATCH 05/20] We can deserialize, but not serialize. --- BedrockCommand.cpp | 21 +++++++++++++++++++-- libstuff/SHTTPSManager.cpp | 4 ++-- libstuff/SHTTPSManager.h | 2 +- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 45ebca746..8da47fa74 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -1,4 +1,5 @@ #include +#include #include "BedrockCommand.h" #include "BedrockPlugin.h" @@ -314,7 +315,23 @@ void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSReque list requests = SParseJSONArray(serializedHTTPSRequests); for (const string& requestStr : requests) { - STable request = SParseJSONObject(requestStr); - + STable requestMap = SParseJSONObject(requestStr); + + SHTTPSManager::Transaction* httpsRequest = new SHTTPSManager::Transaction(_noopHTTPSManager, request["requestID"]); + httpsRequest->s = nullptr; + httpsRequest->created = SToUInt64(requestMap["created"]); + httpsRequest->finished = SToUInt64(requestMap["finished"]); + httpsRequest->timeoutAt = SToUInt64(requestMap["timeoutAt"]); + httpsRequest->sentTime = SToUInt64(requestMap["sentTime"]); + httpsRequest->response = SToInt(requestMap["response"]); + httpsRequest->fullRequest = SDecodeBase64(requestMap["fullRequest"]); + httpsRequest->fullResponse = SDecodeBase64(requestMap["fullResponse"]); + + httpsRequests.push_back(httpsRequest); + + // These should never be incomplete when passed with a serialized command. + if (!httpsRequest->response) { + SWARN("Received incomplete HTTPS request."); + } } } diff --git a/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index d4a04ffb1..2c1cc4f33 100644 --- a/libstuff/SHTTPSManager.cpp +++ b/libstuff/SHTTPSManager.cpp @@ -124,7 +124,7 @@ void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Tra } } -SStandaloneHTTPSManager::Transaction::Transaction(SStandaloneHTTPSManager& manager_) : +SStandaloneHTTPSManager::Transaction::Transaction(SStandaloneHTTPSManager& manager_, const string& requestID) : s(nullptr), created(STimeNow()), finished(0), @@ -132,7 +132,7 @@ SStandaloneHTTPSManager::Transaction::Transaction(SStandaloneHTTPSManager& manag response(0), manager(manager_), sentTime(0), - requestID(SThreadLogPrefix) + requestID(requestID.empty() ? SThreadLogPrefix : requestID) { } diff --git a/libstuff/SHTTPSManager.h b/libstuff/SHTTPSManager.h index 6d2e1c3df..87e725953 100644 --- a/libstuff/SHTTPSManager.h +++ b/libstuff/SHTTPSManager.h @@ -9,7 +9,7 @@ class SStandaloneHTTPSManager : public STCPManager { public: struct Transaction { // Constructor/Destructor - Transaction(SStandaloneHTTPSManager& manager_); + Transaction(SStandaloneHTTPSManager& manager_, const string& requestID = ""); ~Transaction(); // Attributes From 5d6831f7d12d6016420efad94c4ce5f8d10a4525 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 11:07:11 -0800 Subject: [PATCH 06/20] Add serialization. --- BedrockCommand.cpp | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 8da47fa74..ca166bc9c 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -308,7 +308,6 @@ bool BedrockCommand::shouldCommitEmptyTransactions() const { } void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSRequests) { - if (serializedHTTPSRequests.empty()) { return; } @@ -324,8 +323,8 @@ void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSReque httpsRequest->timeoutAt = SToUInt64(requestMap["timeoutAt"]); httpsRequest->sentTime = SToUInt64(requestMap["sentTime"]); httpsRequest->response = SToInt(requestMap["response"]); - httpsRequest->fullRequest = SDecodeBase64(requestMap["fullRequest"]); - httpsRequest->fullResponse = SDecodeBase64(requestMap["fullResponse"]); + httpsRequest->fullRequest.deserialize(SDecodeBase64(requestMap["fullRequest"])); + httpsRequest->fullResponse.deserialize(SDecodeBase64(requestMap["fullResponse"])); httpsRequests.push_back(httpsRequest); @@ -335,3 +334,24 @@ void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSReque } } } + +void BedrockCommand::serializeHTTPSRequests() { + if (!httpsRequests.size()) { + return; + } + + list requests; + for (const auto& httpsRequest : httpsRequests) { + STable data; + data["created"] = to_string(httpsRequest->created); + data["finished"] = to_string(httpsRequest->finished); + data["timeoutAt"] = to_string(httpsRequest->timeoutAt); + data["sentTime"] = to_string(httpsRequest->sentTime); + data["response"] = to_string(httpsRequest->response); + data["fullRequest"] = SEncodeBase64(httpsRequest->fullRequest.serialize()); + data["fullResponse"] = SEncodeBase64(httpsRequest->fullResponse.serialize()); + requests.push_back(SComposeJSONObject(data)); + } + + response["httpsRequests"] = SComposeJSONArray(requests); +} From 1a09647bf0e6870678342844ecdd3d9c28326878 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 11:18:33 -0800 Subject: [PATCH 07/20] Serialize when escalating to leader --- BedrockServer.cpp | 13 ------------- sqlitecluster/SQLiteClusterMessenger.cpp | 3 +++ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 0eaca206a..8df91d45e 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -944,19 +944,6 @@ void BedrockServer::runCommand(unique_ptr&& _command, bool isBlo // write it. We'll flag that here, to keep the node from falling out of LEADING/STANDINGDOWN // until we're finished with this command. if (command->httpsRequests.size()) { - // This *should* be impossible, but previous bugs have existed where it's feasible that we call - // `peekCommand` while leading, and by the time we're done, we're FOLLOWING, so we check just - // in case we ever introduce another similar bug. - if (state != SQLiteNodeState::LEADING && state != SQLiteNodeState::STANDINGDOWN) { - SALERT("Not leading or standing down (" << SQLiteNode::stateName(state) - << ") but have outstanding HTTPS command: " << command->request.methodLine - << ", returning 500."); - command->response.methodLine = "500 STANDDOWN TIMEOUT"; - _reply(command); - core.rollback(); - break; - } - if (command->repeek || !command->areHttpsRequestsComplete()) { // Roll back the existing transaction, but only if we are inside an transaction if (calledPeek) { diff --git a/sqlitecluster/SQLiteClusterMessenger.cpp b/sqlitecluster/SQLiteClusterMessenger.cpp index 6c8d0a597..7c9a88425 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -118,6 +118,9 @@ bool SQLiteClusterMessenger::runOnPeer(BedrockCommand& command, const string& pe return false; } + // Make sure we pass any HTTPS requests the command has made. + command.serializeHTTPSRequests(); + // _sendCommandOnSocket doesn't always call setErrorResponse - if the // command is intended for leader, we don't always want to set // command.complete = true because that prevents it from being retried. If From 7f40da28ec618d37d90155d4c351c7b8624ca365 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 12:59:52 -0800 Subject: [PATCH 08/20] This might actually work. --- BedrockCommand.cpp | 6 +++--- BedrockCommand.h | 4 ++-- BedrockCore.cpp | 4 ---- BedrockServer.cpp | 8 ++++++-- libstuff/libstuff.cpp | 1 - sqlitecluster/SQLiteClusterMessenger.cpp | 4 +--- test/clustertest/testplugin/TestPlugin.cpp | 4 ---- test/clustertest/tests/HTTPSTest.cpp | 8 ++++---- 8 files changed, 16 insertions(+), 23 deletions(-) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index ca166bc9c..52503c8b2 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -335,9 +335,9 @@ void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSReque } } -void BedrockCommand::serializeHTTPSRequests() { +string BedrockCommand::serializeHTTPSRequests() { if (!httpsRequests.size()) { - return; + return ""; } list requests; @@ -353,5 +353,5 @@ void BedrockCommand::serializeHTTPSRequests() { requests.push_back(SComposeJSONObject(data)); } - response["httpsRequests"] = SComposeJSONArray(requests); + return SComposeJSONArray(requests); } diff --git a/BedrockCommand.h b/BedrockCommand.h index 4ff8bdb5a..7edd913b4 100644 --- a/BedrockCommand.h +++ b/BedrockCommand.h @@ -80,8 +80,8 @@ class BedrockCommand : public SQLiteCommand { // Return the name of the plugin for this command. const string& getName() const; - // Take all of the HTTPS requests attached to this object, and serialize them into an `httpsRequests` header field in the response. - void serializeHTTPSRequests(); + // Take all of the HTTPS requests attached to this object, and serialize them to a string. + string serializeHTTPSRequests(); // Take a serialized list of HTTPS requests as from `serializeHTTPSRequests` and deserialize them into the `httpsRequests` object. void deserializeHTTPSRequests(const string& serializedHTTPSRequests); diff --git a/BedrockCore.cpp b/BedrockCore.cpp index c37715d2b..62dc11fa9 100644 --- a/BedrockCore.cpp +++ b/BedrockCore.cpp @@ -192,10 +192,6 @@ BedrockCore::RESULT BedrockCore::peekCommand(unique_ptr& command } catch (const SException& e) { command->repeek = false; _handleCommandException(command, e); - } catch (const SHTTPSManager::NotLeading& e) { - command->repeek = false; - returnValue = RESULT::SHOULD_PROCESS; - SINFO("Command '" << request.methodLine << "' wants to make HTTPS request, queuing for processing."); } catch (...) { command->repeek = false; SALERT("Unhandled exception typename: " << SGetCurrentExceptionName() << ", command: " << request.methodLine); diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 8df91d45e..30eb70824 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2157,15 +2157,19 @@ unique_ptr BedrockServer::buildCommandFromRequest(SData&& reques request["_source"] = ip; } - // Pull any serialized https requests off the requests object to apply tothe command. + // Pull any serialized https requests off the requests object to apply to the command. string serializedHTTPSRequests = request["httpsRequests"]; request.erase("httpsRequests"); // Create a command. unique_ptr command = getCommandFromPlugins(move(request)); - // Apply HTTPS requests. + // Apply HTTPS requests. If we had any, pretend we peeked this command in our current (likely LEADING) state. command->deserializeHTTPSRequests(serializedHTTPSRequests); + if (command->httpsRequests.size()) { + command->lastPeekedOrProcessedInState = _syncNode->getState(); + SINFO("Deserialized " << command->httpsRequests.size() << " HTTPS requests from client."); + } SDEBUG("Deserialized command " << command->request.methodLine); command->socket = fireAndForget ? nullptr : &socket; diff --git a/libstuff/libstuff.cpp b/libstuff/libstuff.cpp index 4fe1506bf..6fc763cd0 100644 --- a/libstuff/libstuff.cpp +++ b/libstuff/libstuff.cpp @@ -1460,7 +1460,6 @@ const char* _SParseJSONObject(const char* ptr, const char* end, STable& out, boo _JSONASSERTPTR(); // Make sure no parse error. if (populateOut) { // Got one more - SDEBUG("Parsed: '" << name << "':'" << value << "'"); out[name] = value; } _JSONLOG(); diff --git a/sqlitecluster/SQLiteClusterMessenger.cpp b/sqlitecluster/SQLiteClusterMessenger.cpp index 7c9a88425..735f118ad 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -118,9 +118,6 @@ bool SQLiteClusterMessenger::runOnPeer(BedrockCommand& command, const string& pe return false; } - // Make sure we pass any HTTPS requests the command has made. - command.serializeHTTPSRequests(); - // _sendCommandOnSocket doesn't always call setErrorResponse - if the // command is intended for leader, we don't always want to set // command.complete = true because that prevents it from being retried. If @@ -140,6 +137,7 @@ bool SQLiteClusterMessenger::_sendCommandOnSocket(SHTTPSManager::Socket& socket, // This is what we need to send. SData request = command.request; + request["httpsRequests"] = command.serializeHTTPSRequests(); request.nameValueMap["ID"] = command.id; SFastBuffer buf(request.serialize()); diff --git a/test/clustertest/testplugin/TestPlugin.cpp b/test/clustertest/testplugin/TestPlugin.cpp index 3a8144596..ed0b12060 100644 --- a/test/clustertest/testplugin/TestPlugin.cpp +++ b/test/clustertest/testplugin/TestPlugin.cpp @@ -223,10 +223,6 @@ bool TestPluginCommand::peek(SQLite& db) { response["stored_not_special"] = plugin().arbitraryData["not_special"]; return true; } else if (SStartsWith(request.methodLine, "sendrequest")) { - if (plugin().server.getState() != SQLiteNodeState::LEADING && plugin().server.getState() != SQLiteNodeState::STANDINGDOWN) { - // Only start HTTPS requests on leader, otherwise, we'll escalate. - return false; - } int requestCount = 1; if (request.isSet("httpsRequestCount")) { requestCount = max(request.calc("httpsRequestCount"), 1); diff --git a/test/clustertest/tests/HTTPSTest.cpp b/test/clustertest/tests/HTTPSTest.cpp index b35fcece9..514c6cbba 100644 --- a/test/clustertest/tests/HTTPSTest.cpp +++ b/test/clustertest/tests/HTTPSTest.cpp @@ -35,7 +35,7 @@ struct HTTPSTest : tpunit::TestFixture { } void testMultipleRequests() { - BedrockTester& brtester = tester->getTester(0); + BedrockTester& brtester = tester->getTester(1); SData request("sendrequest"); request["httpsRequestCount"] = to_string(3); auto result = brtester.executeWaitVerifyContent(request); @@ -45,7 +45,7 @@ struct HTTPSTest : tpunit::TestFixture { void test() { // Send one request to verify that it works. - BedrockTester& brtester = tester->getTester(0); + BedrockTester& brtester = tester->getTester(1); SData request("sendrequest a"); auto result = brtester.executeWaitVerifyContent(request); @@ -56,7 +56,7 @@ struct HTTPSTest : tpunit::TestFixture { // to cause a conflict. mutex m; - // Every 10th request on leader is an HTTP request. + // Every 10th request is an HTTP request. int nthHasRequest = 10; // Let's spin up three threads, each spamming commands at one of our nodes. @@ -66,7 +66,7 @@ struct HTTPSTest : tpunit::TestFixture { threads.emplace_back([this, i, nthHasRequest, &responses, &m](){ vector requests; for (int j = 0; j < 200; j++) { - if (i == 0 && (j % nthHasRequest == 0)) { + if (j % nthHasRequest == 0) { // They should throw all sorts of errors if they repeat HTTPS requests. SData request("sendrequest b"); request["writeConsistency"] = "ASYNC"; From c358132582a96ee4b72aa96e470e1e8a4458621c Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 13:53:25 -0800 Subject: [PATCH 09/20] typo --- libstuff/SHTTPSManager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libstuff/SHTTPSManager.h b/libstuff/SHTTPSManager.h index 87e725953..4ce3d9f32 100644 --- a/libstuff/SHTTPSManager.h +++ b/libstuff/SHTTPSManager.h @@ -60,7 +60,7 @@ class SHTTPSManager : public SStandaloneHTTPSManager { SHTTPSManager(BedrockPlugin& plugin_); SHTTPSManager(BedrockPlugin& plugin_, const string& pem, const string& srvCrt, const string& caCrt); - // TODO: Remove this once AUth no longer checks for it. + // TODO: Remove this once Auth no longer checks for it. class NotLeading : public exception { const char* what() { return "Can't create SHTTPSManager::Transaction when not leading"; From f11c74c647b7da5aaead4d63b5b75dbab7d15f54 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 14:02:20 -0800 Subject: [PATCH 10/20] Better logline --- BedrockServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 30eb70824..78fb85143 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2168,7 +2168,7 @@ unique_ptr BedrockServer::buildCommandFromRequest(SData&& reques command->deserializeHTTPSRequests(serializedHTTPSRequests); if (command->httpsRequests.size()) { command->lastPeekedOrProcessedInState = _syncNode->getState(); - SINFO("Deserialized " << command->httpsRequests.size() << " HTTPS requests from client."); + SINFO("Deserialized " << command->httpsRequests.size() << " HTTPS requests for command " << command->request.methodLine << "."); } SDEBUG("Deserialized command " << command->request.methodLine); From f8ecbfc1db290a3199510fd069d7319ae75b9722 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 15:25:26 -0800 Subject: [PATCH 11/20] Testing changest to hopefully make auth changes small. --- test/lib/BedrockTester.cpp | 14 ++++++++++++++ test/lib/BedrockTester.h | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/test/lib/BedrockTester.cpp b/test/lib/BedrockTester.cpp index 507762e32..6d78040d1 100644 --- a/test/lib/BedrockTester.cpp +++ b/test/lib/BedrockTester.cpp @@ -396,6 +396,9 @@ vector BedrockTester::executeWaitMultipleData(vector requests, int break; } else { myRequest = requests[myIndex]; + if (_enforceCommandOrder) { + myRequest["commitCount"] = to_string(_commitCount); + } } // Reset this for the next request that might need it. @@ -490,6 +493,13 @@ vector BedrockTester::executeWaitMultipleData(vector requests, int } else if (!timedOut) { // Ok, done, let's lock again and insert this in the results. SData responseData; + if (headers.find("commitCount") != headers.end()) { + uint64_t newCommitCount = SToUInt64(headers["commitCount"]); + if (newCommitCount > _commitCount) { + _commitCount = newCommitCount; + cout << "Updated _commitCount to " << _commitCount << endl; + } + } responseData.nameValueMap = headers; responseData.methodLine = methodLine; responseData.content = content; @@ -611,3 +621,7 @@ int BedrockTester::getPID() const { return _serverPID; } + +void BedrockTester::setEnforceCommandOrder(bool enforce) { + _enforceCommandOrder = enforce; +} diff --git a/test/lib/BedrockTester.h b/test/lib/BedrockTester.h index dcf20d03a..12d4f6fff 100644 --- a/test/lib/BedrockTester.h +++ b/test/lib/BedrockTester.h @@ -47,6 +47,10 @@ class BedrockTester { // Shuts down all bedrock servers associated with any existing testers. static void stopAll(); + // If enabled, commands will send the most recent `commitCount` received back from this BedrockTester with each new request, + // enforcing that prior commands finish before later commands. + void setEnforceCommandOrder(bool enforce); + // Generate a temporary filename with an optional prefix. Used particularly to create new DB files for each server, // but can generally be used for any temporary file required. static string getTempFileName(string prefix = ""); @@ -120,5 +124,8 @@ class BedrockTester { uint16_t _nodePort; uint16_t _controlPort; uint16_t _commandPortPrivate; + + bool _enforceCommandOrder = false; + uint64_t _commitCount = 0; }; From 42510fa095c3fb135b304482f682647cc9d4ad75 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Wed, 31 Jan 2024 15:40:16 -0800 Subject: [PATCH 12/20] Remove noisy line --- test/lib/BedrockTester.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/lib/BedrockTester.cpp b/test/lib/BedrockTester.cpp index 6d78040d1..30b302acf 100644 --- a/test/lib/BedrockTester.cpp +++ b/test/lib/BedrockTester.cpp @@ -497,7 +497,6 @@ vector BedrockTester::executeWaitMultipleData(vector requests, int uint64_t newCommitCount = SToUInt64(headers["commitCount"]); if (newCommitCount > _commitCount) { _commitCount = newCommitCount; - cout << "Updated _commitCount to " << _commitCount << endl; } } responseData.nameValueMap = headers; From a2f8d4d6c14f49b281dc61164503c64d98705d14 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Thu, 1 Feb 2024 13:23:56 -0800 Subject: [PATCH 13/20] Add base class serialization functions --- BedrockCommand.cpp | 17 ++++++++++++++++- BedrockCommand.h | 3 +++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 52503c8b2..254f3311f 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -306,13 +306,16 @@ void BedrockCommand::setTimeout(uint64_t timeoutDurationMS) { bool BedrockCommand::shouldCommitEmptyTransactions() const { return _commitEmptyTransactions; } - +#include void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSRequests) { if (serializedHTTPSRequests.empty()) { return; } list requests = SParseJSONArray(serializedHTTPSRequests); + cout << "Deserializing all: " << endl; + cout << serializedHTTPSRequests << endl; + cout << "Done with all." << endl; for (const string& requestStr : requests) { STable requestMap = SParseJSONObject(requestStr); @@ -326,6 +329,8 @@ void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSReque httpsRequest->fullRequest.deserialize(SDecodeBase64(requestMap["fullRequest"])); httpsRequest->fullResponse.deserialize(SDecodeBase64(requestMap["fullResponse"])); + cout << "Deserializing: " << httpsRequest->fullResponse.serialize() << endl; + cout << "Deserialized." << endl; httpsRequests.push_back(httpsRequest); // These should never be incomplete when passed with a serialized command. @@ -351,7 +356,17 @@ string BedrockCommand::serializeHTTPSRequests() { data["fullRequest"] = SEncodeBase64(httpsRequest->fullRequest.serialize()); data["fullResponse"] = SEncodeBase64(httpsRequest->fullResponse.serialize()); requests.push_back(SComposeJSONObject(data)); + + cout << "Serializing: " << httpsRequest->fullResponse.serialize() << endl; + cout << "Serialized." << endl; } return SComposeJSONArray(requests); } + +string BedrockCommand::serializeData() const { + return ""; +} + +void BedrockCommand::deserializeData(const string& data) { +} diff --git a/BedrockCommand.h b/BedrockCommand.h index 7edd913b4..317fbd291 100644 --- a/BedrockCommand.h +++ b/BedrockCommand.h @@ -191,6 +191,9 @@ class BedrockCommand : public SQLiteCommand { // Return the number of commands in existence. static size_t getCommandCount() { return _commandCount.load(); } + virtual string serializeData() const; + virtual void deserializeData(const string& data); + // True if this command should be escalated immediately. This can be true for any command that does all of its work // in `process` instead of peek, as it will always be escalated to leader const bool escalateImmediately; From a4672b3e8e0b147e8144893c05132408b678fac8 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Thu, 1 Feb 2024 13:27:40 -0800 Subject: [PATCH 14/20] Remove extra logging --- BedrockCommand.cpp | 10 +--------- libstuff/SHTTPSManager.cpp | 4 +++- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 254f3311f..7a47839c3 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -306,16 +306,13 @@ void BedrockCommand::setTimeout(uint64_t timeoutDurationMS) { bool BedrockCommand::shouldCommitEmptyTransactions() const { return _commitEmptyTransactions; } -#include + void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSRequests) { if (serializedHTTPSRequests.empty()) { return; } list requests = SParseJSONArray(serializedHTTPSRequests); - cout << "Deserializing all: " << endl; - cout << serializedHTTPSRequests << endl; - cout << "Done with all." << endl; for (const string& requestStr : requests) { STable requestMap = SParseJSONObject(requestStr); @@ -329,8 +326,6 @@ void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSReque httpsRequest->fullRequest.deserialize(SDecodeBase64(requestMap["fullRequest"])); httpsRequest->fullResponse.deserialize(SDecodeBase64(requestMap["fullResponse"])); - cout << "Deserializing: " << httpsRequest->fullResponse.serialize() << endl; - cout << "Deserialized." << endl; httpsRequests.push_back(httpsRequest); // These should never be incomplete when passed with a serialized command. @@ -356,9 +351,6 @@ string BedrockCommand::serializeHTTPSRequests() { data["fullRequest"] = SEncodeBase64(httpsRequest->fullRequest.serialize()); data["fullResponse"] = SEncodeBase64(httpsRequest->fullResponse.serialize()); requests.push_back(SComposeJSONObject(data)); - - cout << "Serializing: " << httpsRequest->fullResponse.serialize() << endl; - cout << "Serialized." << endl; } return SComposeJSONArray(requests); diff --git a/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index 2c1cc4f33..bf63820e9 100644 --- a/libstuff/SHTTPSManager.cpp +++ b/libstuff/SHTTPSManager.cpp @@ -61,7 +61,7 @@ void SStandaloneHTTPSManager::prePoll(fd_map& fdm, SStandaloneHTTPSManager::Tran } STCPManager::prePoll(fdm, *transaction.s); } - +#include void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Transaction& transaction, uint64_t& nextActivity, uint64_t timeoutMS) { if (!transaction.s || transaction.finished) { // If there's no socket, or we're done, skip. Because we call poll on commands, we may poll transactions that @@ -82,6 +82,8 @@ void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Tra uint64_t now = STimeNow(); int size = transaction.fullResponse.deserialize(transaction.s->recvBuffer); if (size) { + + cout << "Got: " << transaction.fullResponse.serialize() << endl; // Consume how much we read. transaction.s->recvBuffer.consumeFront(size); transaction.finished = now; From f0dfa4c67a6e029894e81054af6116661fc50319 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Thu, 1 Feb 2024 13:48:32 -0800 Subject: [PATCH 15/20] Correctly set serialized data --- BedrockServer.cpp | 9 +++++++-- sqlitecluster/SQLiteClusterMessenger.cpp | 11 ++++++++++- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 78fb85143..8105dd9a7 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2159,17 +2159,22 @@ unique_ptr BedrockServer::buildCommandFromRequest(SData&& reques // Pull any serialized https requests off the requests object to apply to the command. string serializedHTTPSRequests = request["httpsRequests"]; + string serializedData = request["serializedData"]; request.erase("httpsRequests"); + request.erase("serializedData"); // Create a command. unique_ptr command = getCommandFromPlugins(move(request)); // Apply HTTPS requests. If we had any, pretend we peeked this command in our current (likely LEADING) state. - command->deserializeHTTPSRequests(serializedHTTPSRequests); - if (command->httpsRequests.size()) { + if (serializedHTTPSRequests.size()) { + command->deserializeHTTPSRequests(serializedHTTPSRequests); command->lastPeekedOrProcessedInState = _syncNode->getState(); SINFO("Deserialized " << command->httpsRequests.size() << " HTTPS requests for command " << command->request.methodLine << "."); } + if (serializedData.size()) { + command->deserializeData(serializedData); + } SDEBUG("Deserialized command " << command->request.methodLine); command->socket = fireAndForget ? nullptr : &socket; diff --git a/sqlitecluster/SQLiteClusterMessenger.cpp b/sqlitecluster/SQLiteClusterMessenger.cpp index 735f118ad..02276abfc 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -137,7 +137,16 @@ bool SQLiteClusterMessenger::_sendCommandOnSocket(SHTTPSManager::Socket& socket, // This is what we need to send. SData request = command.request; - request["httpsRequests"] = command.serializeHTTPSRequests(); + string httpsRequests = command.serializeHTTPSRequests(); + string serializedData = command.serializeData(); + + if (httpsRequests.size()) { + request["httpsRequests"] = command.serializeHTTPSRequests(); + } + if (serializedData.size()) { + request["serializedData"] = command.serializeData(); + } + request.nameValueMap["ID"] = command.id; SFastBuffer buf(request.serialize()); From b9150b8316b583c39b4af427309fba499ddd78aa Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Thu, 1 Feb 2024 17:17:30 -0800 Subject: [PATCH 16/20] Remove test logging --- libstuff/SHTTPSManager.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index bf63820e9..2c1cc4f33 100644 --- a/libstuff/SHTTPSManager.cpp +++ b/libstuff/SHTTPSManager.cpp @@ -61,7 +61,7 @@ void SStandaloneHTTPSManager::prePoll(fd_map& fdm, SStandaloneHTTPSManager::Tran } STCPManager::prePoll(fdm, *transaction.s); } -#include + void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Transaction& transaction, uint64_t& nextActivity, uint64_t timeoutMS) { if (!transaction.s || transaction.finished) { // If there's no socket, or we're done, skip. Because we call poll on commands, we may poll transactions that @@ -82,8 +82,6 @@ void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Tra uint64_t now = STimeNow(); int size = transaction.fullResponse.deserialize(transaction.s->recvBuffer); if (size) { - - cout << "Got: " << transaction.fullResponse.serialize() << endl; // Consume how much we read. transaction.s->recvBuffer.consumeFront(size); transaction.finished = now; From c6689c9a7eef9684c5355608b7e21caaa322ff7b Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Mon, 5 Feb 2024 10:08:38 -0800 Subject: [PATCH 17/20] Fix up logic for when to serialize command data --- sqlitecluster/SQLiteClusterMessenger.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/sqlitecluster/SQLiteClusterMessenger.cpp b/sqlitecluster/SQLiteClusterMessenger.cpp index 02276abfc..64fb7eb4b 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -137,14 +137,15 @@ bool SQLiteClusterMessenger::_sendCommandOnSocket(SHTTPSManager::Socket& socket, // This is what we need to send. SData request = command.request; - string httpsRequests = command.serializeHTTPSRequests(); - string serializedData = command.serializeData(); - if (httpsRequests.size()) { + // If the command has https requests, we serialize them to escalate. In this case we also check if th command has data + // that needs serialization, and if so, we serialize that as well. + if (command.httpsRequests.size()) { request["httpsRequests"] = command.serializeHTTPSRequests(); - } - if (serializedData.size()) { - request["serializedData"] = command.serializeData(); + string serializedData = command.serializeData(); + if (serializedData.size()) { + request["serializedData"] = move(serializedData); + } } request.nameValueMap["ID"] = command.id; From 8b50e6c1f0487f4335b6e2e25ad8cca47d1c89a1 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Tue, 6 Feb 2024 14:01:30 -0800 Subject: [PATCH 18/20] Allow for a single commit count per cluster. --- test/clustertest/BedrockClusterTester.h | 4 +++- test/lib/BedrockTester.cpp | 6 ++++-- test/lib/BedrockTester.h | 6 ++++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/test/clustertest/BedrockClusterTester.h b/test/clustertest/BedrockClusterTester.h index b4625192e..4fead588d 100644 --- a/test/clustertest/BedrockClusterTester.h +++ b/test/clustertest/BedrockClusterTester.h @@ -35,6 +35,8 @@ class ClusterTester { // Stops a given node. void stopNode(size_t index); + atomic groupCommitCount{0}; + private: // The number of nodes in the cluster. @@ -140,7 +142,7 @@ ClusterTester::ClusterTester(ClusterSize size, } // And add the new entry in the map. - _cluster.emplace_back(args, queries, serverPort, nodePort, controlPort, false, processPath); + _cluster.emplace_back(args, queries, serverPort, nodePort, controlPort, false, processPath, &groupCommitCount); } // Now start them all. diff --git a/test/lib/BedrockTester.cpp b/test/lib/BedrockTester.cpp index 30b302acf..d05ce30a6 100644 --- a/test/lib/BedrockTester.cpp +++ b/test/lib/BedrockTester.cpp @@ -40,11 +40,13 @@ BedrockTester::BedrockTester(const map& args, uint16_t nodePort, uint16_t controlPort, bool startImmediately, - const string& bedrockBinary) : + const string& bedrockBinary, + atomic* alternateCounter) : _serverPort(serverPort ?: ports.getPort()), _nodePort(nodePort ?: ports.getPort()), _controlPort(controlPort ?: ports.getPort()), - _commandPortPrivate(ports.getPort()) + _commandPortPrivate(ports.getPort()), + _commitCount(alternateCounter ? *alternateCounter : _commitCountBase) { { lock_guard lock(_testersMutex); diff --git a/test/lib/BedrockTester.h b/test/lib/BedrockTester.h index 12d4f6fff..6e694b795 100644 --- a/test/lib/BedrockTester.h +++ b/test/lib/BedrockTester.h @@ -32,7 +32,8 @@ class BedrockTester { uint16_t nodePort = 0, uint16_t controlPort = 0, bool startImmediately = true, - const string& bedrockBinary = ""); + const string& bedrockBinary = "", + atomic* alternateCounter = nullptr); // Destructor. ~BedrockTester(); @@ -126,6 +127,7 @@ class BedrockTester { uint16_t _commandPortPrivate; bool _enforceCommandOrder = false; - uint64_t _commitCount = 0; + atomic _commitCountBase = 0; + atomic& _commitCount; }; From 4b4d9966653138989a5e5e4b86e7a5573357f148 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Thu, 8 Feb 2024 16:25:28 -0800 Subject: [PATCH 19/20] Disable actually HTTPS on followers --- BedrockCore.cpp | 4 ++++ libstuff/SHTTPSManager.cpp | 10 ++++++++++ libstuff/SHTTPSManager.h | 9 +++++++++ 3 files changed, 23 insertions(+) diff --git a/BedrockCore.cpp b/BedrockCore.cpp index 62dc11fa9..c37715d2b 100644 --- a/BedrockCore.cpp +++ b/BedrockCore.cpp @@ -192,6 +192,10 @@ BedrockCore::RESULT BedrockCore::peekCommand(unique_ptr& command } catch (const SException& e) { command->repeek = false; _handleCommandException(command, e); + } catch (const SHTTPSManager::NotLeading& e) { + command->repeek = false; + returnValue = RESULT::SHOULD_PROCESS; + SINFO("Command '" << request.methodLine << "' wants to make HTTPS request, queuing for processing."); } catch (...) { command->repeek = false; SALERT("Unhandled exception typename: " << SGetCurrentExceptionName() << ", command: " << request.methodLine); diff --git a/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index 2c1cc4f33..e877bbcf4 100644 --- a/libstuff/SHTTPSManager.cpp +++ b/libstuff/SHTTPSManager.cpp @@ -15,6 +15,13 @@ SHTTPSManager::SHTTPSManager(BedrockPlugin& plugin_, const string& pem, const st { } +void SHTTPSManager::validate() { + // These can only be created on a leader node. + if (plugin.server.getState() != SQLiteNodeState::LEADING && plugin.server.getState() != SQLiteNodeState::STANDINGDOWN) { + throw NotLeading(); + } +} + SStandaloneHTTPSManager::SStandaloneHTTPSManager() { } @@ -134,6 +141,8 @@ SStandaloneHTTPSManager::Transaction::Transaction(SStandaloneHTTPSManager& manag sentTime(0), requestID(requestID.empty() ? SThreadLogPrefix : requestID) { + // TODO: Remove this block to to enable HTTPS on followers. Also the `validate` method can be removed entirely. + manager.validate(); } SStandaloneHTTPSManager::Transaction::~Transaction() { @@ -161,6 +170,7 @@ SStandaloneHTTPSManager::Transaction* SStandaloneHTTPSManager::_httpsSend(const host += ":443"; } + // Create a new transaction. This can throw if `validate` fails. We explicitly do this *before* creating a socket. Transaction* transaction = new Transaction(*this); // If this is going to be an https transaction, create a certificate and give it to the socket. diff --git a/libstuff/SHTTPSManager.h b/libstuff/SHTTPSManager.h index 4ce3d9f32..022cf5f12 100644 --- a/libstuff/SHTTPSManager.h +++ b/libstuff/SHTTPSManager.h @@ -42,6 +42,13 @@ class SStandaloneHTTPSManager : public STCPManager { static int getHTTPResponseCode(const string& methodLine); + virtual void validate() { + // The constructor for a transaction needs to call this on it's manager. It can then throw in cases where this + // manager should not be allowed to create transactions. This lets us have different validation behavior for + // different managers without needing to subclass Transaction. + // The default implementation does nothing. + } + protected: // Child API // Used to create the signing certificate. @@ -67,6 +74,8 @@ class SHTTPSManager : public SStandaloneHTTPSManager { } }; + virtual void validate() override; + protected: // Reference to the plugin that owns this object. BedrockPlugin& plugin; From 66e0c3d16b20af54da8342440aaac56128849e3d Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Mon, 12 Feb 2024 11:12:31 -0800 Subject: [PATCH 20/20] Update sqlitecluster/SQLiteClusterMessenger.cpp Co-authored-by: Florent De'Neve --- sqlitecluster/SQLiteClusterMessenger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sqlitecluster/SQLiteClusterMessenger.cpp b/sqlitecluster/SQLiteClusterMessenger.cpp index 64fb7eb4b..617aba678 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -138,7 +138,7 @@ bool SQLiteClusterMessenger::_sendCommandOnSocket(SHTTPSManager::Socket& socket, // This is what we need to send. SData request = command.request; - // If the command has https requests, we serialize them to escalate. In this case we also check if th command has data + // If the command has https requests, we serialize them to escalate. In this case we also check if the command has data // that needs serialization, and if so, we serialize that as well. if (command.httpsRequests.size()) { request["httpsRequests"] = command.serializeHTTPSRequests();