From 1b6b3c5e0542692fa821247248b6ca2f35e90bc0 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Tue, 26 Nov 2024 11:49:06 -0800 Subject: [PATCH 1/5] Send serialized data regardless of https requests --- sqlitecluster/SQLiteClusterMessenger.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sqlitecluster/SQLiteClusterMessenger.cpp b/sqlitecluster/SQLiteClusterMessenger.cpp index 67777ecbb..80f4eb46b 100644 --- a/sqlitecluster/SQLiteClusterMessenger.cpp +++ b/sqlitecluster/SQLiteClusterMessenger.cpp @@ -130,10 +130,10 @@ bool SQLiteClusterMessenger::_sendCommandOnSocket(SHTTPSManager::Socket& socket, // 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); - } + } + string serializedData = command.serializeData(); + if (serializedData.size()) { + request["serializedData"] = move(serializedData); } request.nameValueMap["ID"] = command.id; From e3179092ee5c785987a6d3c811a131bbc0a0b884 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Tue, 26 Nov 2024 12:13:40 -0800 Subject: [PATCH 2/5] Add tests --- test/clustertest/testplugin/TestPlugin.cpp | 30 +++++++++++++++++++++- test/clustertest/testplugin/TestPlugin.h | 4 +++ test/clustertest/tests/EscalateTest.cpp | 22 ++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/test/clustertest/testplugin/TestPlugin.cpp b/test/clustertest/testplugin/TestPlugin.cpp index c7a76f785..4c5d83084 100644 --- a/test/clustertest/testplugin/TestPlugin.cpp +++ b/test/clustertest/testplugin/TestPlugin.cpp @@ -1,5 +1,6 @@ #include "TestPlugin.h" +#include #include #include #include @@ -102,7 +103,8 @@ unique_ptr BedrockPlugin_TestPlugin::getCommand(SQLiteCommand&& "prepeekpostprocesscommand", "preparehandler", "testquery", - "testPostProcessTimeout" + "testPostProcessTimeout", + "EscalateSerializedData" }; for (auto& cmdName : supportedCommands) { if (SStartsWith(baseCommand.request.methodLine, cmdName)) { @@ -119,6 +121,19 @@ TestPluginCommand::TestPluginCommand(SQLiteCommand&& baseCommand, BedrockPlugin_ { } +string TestPluginCommand::serializeData() const { + if (SStartsWith(request.methodLine, "EscalateSerializedData")) { + return serializedDataString; + } + return ""; +} + +void TestPluginCommand::deserializeData(const string& data) { + if (SStartsWith(request.methodLine, "EscalateSerializedData")) { + serializedDataString = data; + } +} + TestPluginCommand::~TestPluginCommand() { if (request.methodLine == "testescalate") { @@ -200,6 +215,15 @@ bool TestPluginCommand::peek(SQLite& db) { } response.content = "this is a test response"; return true; + } else if (SStartsWith(request.methodLine,"EscalateSerializedData")) { + // Only set this if it's blank. The intention is that it will be blank on a follower, but already set by + // serialize/deserialize on the leader. + if (serializedDataString.empty()) { + serializedDataString = _plugin->server.args["-nodeName"] + ":" + _plugin->server.args["-testName"]; + } + // On followers, this immediately escalates to leader. + // On leader, it immediately skips to `process`. + return false; } else if (SStartsWith(request.methodLine, "broadcastwithtimeouts")) { // First, send a `broadcastwithtimeouts` which will generate a new command and broadcast that to peers. SData subCommand("storeboradcasttimeouts"); @@ -391,6 +415,10 @@ void TestPluginCommand::process(SQLite& db) { querySize += nq.size(); } response["QuerySize"] = to_string(querySize); + } else if (SStartsWith(request.methodLine,"EscalateSerializedData")) { + // We want to return the data that was serialized and escalated, and also our own nodename, to verify it does not match + // the node that serialized the data. + response.content = _plugin->server.args["-nodeName"] + ":" + serializedDataString; } else if (SStartsWith(request.methodLine, "sendrequest")) { // This flag makes us pass through the response we got from the server, rather than returning 200 if every // response we got from the server was < 400. I.e., if the server returns 202, or 304, or anything less than diff --git a/test/clustertest/testplugin/TestPlugin.h b/test/clustertest/testplugin/TestPlugin.h index 67f0810ac..26f24535d 100644 --- a/test/clustertest/testplugin/TestPlugin.h +++ b/test/clustertest/testplugin/TestPlugin.h @@ -53,10 +53,14 @@ class TestPluginCommand : public BedrockCommand { virtual void reset(BedrockCommand::STAGE stage) override; bool shouldEnableOnPrepareNotification(const SQLite& db, void (**handler)(SQLite& _db, int64_t tableID)) override; + virtual string serializeData() const override; + virtual void deserializeData(const string& data) override; + private: BedrockPlugin_TestPlugin& plugin() { return static_cast(*_plugin); } bool pendingResult; string chainedHTTPResponseContent; string urls; + string serializedDataString; }; diff --git a/test/clustertest/tests/EscalateTest.cpp b/test/clustertest/tests/EscalateTest.cpp index 1c8a478ad..761c9d282 100644 --- a/test/clustertest/tests/EscalateTest.cpp +++ b/test/clustertest/tests/EscalateTest.cpp @@ -1,3 +1,4 @@ +#include "libstuff/libstuff.h" #include #include @@ -5,6 +6,7 @@ struct EscalateTest : tpunit::TestFixture { EscalateTest() : tpunit::TestFixture("Escalate", BEFORE_CLASS(EscalateTest::setup), AFTER_CLASS(EscalateTest::teardown), TEST(EscalateTest::test), + TEST(EscalateTest::testSerializedData), TEST(EscalateTest::socketReuse)) { } BedrockClusterTester* tester = nullptr; @@ -46,6 +48,26 @@ struct EscalateTest : tpunit::TestFixture { SFileDelete(cmd["tempFile"]); } + void testSerializedData() + { + // We're going to escalate from follower 1. + BedrockTester& brtester = tester->getTester(1); + SData cmd("EscalateSerializedData"); + SData result = brtester.executeWaitMultipleData({cmd})[0]; + + // Parse the results, which shoould contain the node name we escalated from, + // the node name we escalated to, and the test name. + auto resultComponenets = SParseList(result.content, ':'); + + // Validate the results. + ASSERT_EQUAL("cluster_node_0", resultComponenets.front()); + resultComponenets.pop_front(); + ASSERT_EQUAL("cluster_node_1", resultComponenets.front()); + resultComponenets.pop_front(); + ASSERT_EQUAL("Escalate", resultComponenets.front()); + resultComponenets.pop_front(); + } + // Note: see this PR: https://github.com/Expensify/Bedrock/pull/1308 for the reasoning behind this test. void socketReuse() { From 9c6b55a94689ad69a17c4de12ce50e6b878a6cb4 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Tue, 26 Nov 2024 17:28:36 -0800 Subject: [PATCH 3/5] Update test/clustertest/tests/EscalateTest.cpp Co-authored-by: Carlos Martins --- test/clustertest/tests/EscalateTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/clustertest/tests/EscalateTest.cpp b/test/clustertest/tests/EscalateTest.cpp index 761c9d282..f995fab06 100644 --- a/test/clustertest/tests/EscalateTest.cpp +++ b/test/clustertest/tests/EscalateTest.cpp @@ -55,7 +55,7 @@ struct EscalateTest : tpunit::TestFixture { SData cmd("EscalateSerializedData"); SData result = brtester.executeWaitMultipleData({cmd})[0]; - // Parse the results, which shoould contain the node name we escalated from, + // Parse the results, which should contain the node name we escalated from, // the node name we escalated to, and the test name. auto resultComponenets = SParseList(result.content, ':'); From 2ba91a8c7ffe8f2e4191ddfa19a3fba3c99d50b9 Mon Sep 17 00:00:00 2001 From: Tyler Karaszewski Date: Tue, 26 Nov 2024 17:28:46 -0800 Subject: [PATCH 4/5] Update test/clustertest/testplugin/TestPlugin.cpp Co-authored-by: Carlos Martins --- test/clustertest/testplugin/TestPlugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/clustertest/testplugin/TestPlugin.cpp b/test/clustertest/testplugin/TestPlugin.cpp index 4c5d83084..ca9e91e22 100644 --- a/test/clustertest/testplugin/TestPlugin.cpp +++ b/test/clustertest/testplugin/TestPlugin.cpp @@ -215,7 +215,7 @@ bool TestPluginCommand::peek(SQLite& db) { } response.content = "this is a test response"; return true; - } else if (SStartsWith(request.methodLine,"EscalateSerializedData")) { + } else if (SStartsWith(request.methodLine, "EscalateSerializedData")) { // Only set this if it's blank. The intention is that it will be blank on a follower, but already set by // serialize/deserialize on the leader. if (serializedDataString.empty()) { From f029cc3a22cd6ab525600d30c244b2e005e2847d Mon Sep 17 00:00:00 2001 From: Maria DCosta Date: Thu, 28 Nov 2024 17:38:38 +0400 Subject: [PATCH 5/5] Add keys for OFAC watchlist alert --- libstuff/SLog.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp index e87c92398..107269ef0 100644 --- a/libstuff/SLog.cpp +++ b/libstuff/SLog.cpp @@ -76,6 +76,7 @@ static const set PARAMS_WHITELIST = { "nvpName", "policyAccountID", "policyID", + "reimbursementEntryID", "reportID", "requestID", "requestTimestamp", @@ -89,7 +90,8 @@ static const set PARAMS_WHITELIST = { "transactionID", "type", "userID", - "secondaryLogin" + "secondaryLogin", + "walletBankAccountID" }; string addLogParams(string&& message, const STable& params) {