diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 931874684..7a47839c3 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -1,9 +1,12 @@ #include +#include #include "BedrockCommand.h" #include "BedrockPlugin.h" atomic BedrockCommand::_commandCount(0); +SStandaloneHTTPSManager BedrockCommand::_noopHTTPSManager; + const string BedrockCommand::defaultPluginName("NO_PLUGIN"); BedrockCommand::BedrockCommand(SQLiteCommand&& baseCommand, BedrockPlugin* plugin, bool escalateImmediately_) : @@ -303,3 +306,59 @@ void BedrockCommand::setTimeout(uint64_t timeoutDurationMS) { bool BedrockCommand::shouldCommitEmptyTransactions() const { return _commitEmptyTransactions; } + +void BedrockCommand::deserializeHTTPSRequests(const string& serializedHTTPSRequests) { + if (serializedHTTPSRequests.empty()) { + return; + } + + list requests = SParseJSONArray(serializedHTTPSRequests); + for (const string& requestStr : requests) { + 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.deserialize(SDecodeBase64(requestMap["fullRequest"])); + httpsRequest->fullResponse.deserialize(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."); + } + } +} + +string 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)); + } + + return SComposeJSONArray(requests); +} + +string BedrockCommand::serializeData() const { + return ""; +} + +void BedrockCommand::deserializeData(const string& data) { +} diff --git a/BedrockCommand.h b/BedrockCommand.h index bcc5ad3f5..317fbd291 100644 --- a/BedrockCommand.h +++ b/BedrockCommand.h @@ -80,6 +80,12 @@ 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 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); + // 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. @@ -185,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; @@ -231,5 +240,7 @@ class BedrockCommand : public SQLiteCommand { static atomic _commandCount; + static SStandaloneHTTPSManager _noopHTTPSManager; + static const string defaultPluginName; }; 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/BedrockServer.cpp b/BedrockServer.cpp index 18c6239fa..8105dd9a7 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) { @@ -2170,8 +2157,25 @@ unique_ptr BedrockServer::buildCommandFromRequest(SData&& reques request["_source"] = ip; } + // 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. + 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/libstuff/SHTTPSManager.cpp b/libstuff/SHTTPSManager.cpp index 9f3fbe349..e877bbcf4 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); } void SHTTPSManager::validate() { @@ -102,7 +100,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 { @@ -133,7 +131,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), @@ -141,8 +139,9 @@ SStandaloneHTTPSManager::Transaction::Transaction(SStandaloneHTTPSManager& manag response(0), manager(manager_), sentTime(0), - requestID(SThreadLogPrefix) + 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(); } diff --git a/libstuff/SHTTPSManager.h b/libstuff/SHTTPSManager.h index 9d2a1d1a3..022cf5f12 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 @@ -60,9 +60,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 { @@ -70,6 +67,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"; 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 6c8d0a597..617aba678 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -137,6 +137,17 @@ 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 the command has data + // that needs serialization, and if so, we serialize that as well. + if (command.httpsRequests.size()) { + request["httpsRequests"] = command.serializeHTTPSRequests(); + string serializedData = command.serializeData(); + if (serializedData.size()) { + request["serializedData"] = move(serializedData); + } + } + 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";