From 728790a2b4d2f4e49d9bd66f707ece439c383238 Mon Sep 17 00:00:00 2001 From: Scott Pope Date: Sat, 3 Oct 2026 15:48:07 -0500 Subject: [PATCH] Parse multicall commands once per call Keep the change to d, f, p and t multicalls. Parse each column lazily on first use and evaluate it through rpc::call_object, preserving per-target argument expansion. Compared unpatched and patched builds on macOS: output was byte-identical across the test harness, including an erase-mid-call check. --- src/command_download.cc | 40 +++++++++++++++++++++---------- src/command_events.cc | 12 +++++++--- src/rpc/parse_commands.cc | 33 ++++++++++++++++++++++++++ src/rpc/parse_commands.h | 31 ++++++++++++++++++++++++ test/rpc/test_command.cc | 50 +++++++++++++++++++++++++++++++++++++++ test/rpc/test_command.h | 4 ++++ 6 files changed, 154 insertions(+), 16 deletions(-) diff --git a/src/command_download.cc b/src/command_download.cc index 1317245e..d76651ea 100644 --- a/src/command_download.cc +++ b/src/command_download.cc @@ -334,6 +334,11 @@ f_multicall(core::Download* download, const torrent::Object::list_type& args) { bool use_regex = true; + rpc::preparsed_commands commands([&args](auto& cmds) { + for (auto cItr = ++args.begin(); cItr != args.end(); ++cItr) + cmds.push_back(rpc::parse_command_object(cItr->as_string())); + }); + if (args.front().is_list()) for (const auto& o : args.front().as_list()) regex_list.push_back(o.as_string_c()); @@ -349,10 +354,10 @@ f_multicall(core::Download* download, const torrent::Object::list_type& args) { torrent::Object::list_type& row = result.insert(result.end(), torrent::Object::create_list())->as_list(); - for (torrent::Object::list_const_iterator cItr = ++args.begin(); cItr != args.end(); cItr++) { - const std::string& cmd = cItr->as_string(); - row.push_back(rpc::parse_command(rpc::make_target(file.get()), cmd.c_str(), cmd.c_str() + cmd.size()).first); - } + // Defer parsing until a file actually matches the multicall selection. + commands.prepare_if_needed(); + for (auto& itr : commands) + row.push_back(rpc::call_object(itr, rpc::make_target(file.get()))); } return resultRaw; @@ -372,6 +377,11 @@ t_multicall(core::Download* download, const torrent::Object::list_type& args) { auto result_raw = torrent::Object::create_list(); auto& result = result_raw.as_list(); + rpc::preparsed_commands commands([&args](auto& cmds) { + for (auto cItr = ++args.begin(); cItr != args.end(); ++cItr) + cmds.push_back(rpc::parse_command_object(cItr->as_string())); + }); + for (uint32_t idx = 0, last = download->tracker_list_size(); idx < last; idx++) { auto& row = result.insert(result.end(), torrent::Object::create_list())->as_list(); auto tracker = download->tracker_controller().at(idx); @@ -379,11 +389,10 @@ t_multicall(core::Download* download, const torrent::Object::list_type& args) { if (!tracker.is_valid()) continue; - for (auto cItr = ++args.begin(); cItr != args.end(); cItr++) { - auto& cmd = cItr->as_string(); - - row.push_back(rpc::parse_command(rpc::make_target(&tracker), cmd.c_str(), cmd.c_str() + cmd.size()).first); - } + // Do not parse columns when there are no valid tracker targets. + commands.prepare_if_needed(); + for (auto& itr : commands) + row.push_back(rpc::call_object(itr, rpc::make_target(&tracker))); } return result_raw; @@ -405,13 +414,18 @@ p_multicall(core::Download* download, const torrent::Object::list_type& args) { auto* connection_list = download->connection_list(); const auto change_counter = connection_list->change_counter(); + rpc::preparsed_commands commands([&args](auto& cmds) { + for (auto cItr = ++args.begin(); cItr != args.end(); ++cItr) + cmds.push_back(rpc::parse_command_object(cItr->as_string())); + }); + for (const auto& connection : *connection_list) { torrent::Object::list_type& row = result.insert(result.end(), torrent::Object::create_list())->as_list(); - for (auto cItr = ++args.begin(); cItr != args.end(); cItr++) { - const std::string& cmd = cItr->as_string(); - - row.push_back(rpc::parse_command(rpc::make_target(connection), cmd.c_str(), cmd.c_str() + cmd.size()).first); + // Prepare only after a peer exists, preserving empty-list laziness. + commands.prepare_if_needed(); + for (auto& itr : commands) { + row.push_back(rpc::call_object(itr, rpc::make_target(connection))); // Erasing a peer frees it and swaps the last element into its place, so // neither this peer nor the iteration survives a change to the list. diff --git a/src/command_events.cc b/src/command_events.cc index 2538944a..74203f77 100644 --- a/src/command_events.cc +++ b/src/command_events.cc @@ -247,20 +247,26 @@ d_multicall(const torrent::Object::list_type& args) { torrent::Object resultRaw = torrent::Object::create_list(); torrent::Object::list_type& result = resultRaw.as_list(); + rpc::preparsed_commands commands([&args](auto& cmds) { + for (auto cItr = ++args.begin(); cItr != args.end(); ++cItr) + cmds.push_back(rpc::parse_command_object(cItr->as_string())); + }); + for (const auto& download : dlist) { if (download.use_count() == 1) continue; torrent::Object::list_type& row = result.insert(result.end(), torrent::Object::create_list())->as_list(); - for (torrent::Object::list_const_iterator cItr = ++args.begin(); cItr != args.end(); cItr++) { + // Skip parsing if there are no usable download targets in the view. + commands.prepare_if_needed(); + for (auto& itr : commands) { // A command may erase this download, which destroys the torrent object it // wraps; the list dropping its reference is what tells us. if (download.use_count() == 1) break; - auto& cmd = cItr->as_string(); - row.push_back(rpc::parse_command(rpc::make_target(download), cmd.c_str(), cmd.c_str() + cmd.size()).first); + row.push_back(rpc::call_object(itr, rpc::make_target(download))); } } diff --git a/src/rpc/parse_commands.cc b/src/rpc/parse_commands.cc index 06c74f7c..64c5c525 100644 --- a/src/rpc/parse_commands.cc +++ b/src/rpc/parse_commands.cc @@ -123,6 +123,39 @@ parse_command(target_type target, const char* first, const char* last) { return std::make_pair(commands.call_command(key, args, target), first); } +torrent::Object +parse_command_object(const char* first, const char* last) { + first = std::find_if(first, last, [&](char c) { return !command_map_is_space(c); }); + + if (first == last || *first == '#') + return torrent::Object(); + + char key[128]; + + first = parse_command_name(first, last, key, key + 128); + first = std::find_if(first, last, [&](char c) { return !command_map_is_space(c); }); + + if (first == last || *first != '=') + throw torrent::input_error("Could not find '=' in command '" + std::string(key) + "'."); + + torrent::Object result = torrent::Object::create_dict_key(); + + result.as_dict_key() = key; + + first = parse_whole_list(first + 1, last, &result.as_dict_obj(), &parse_is_delim_command); + + // Find the last character that is part of this command, skipping + // the whitespace at the end. + first = std::find_if(first, last, [&](char c) { return !command_map_is_space(c); }); + + // This helper accepts exactly one command and cannot return where a next + // command begins, so reject every non-whitespace suffix, including ';'. + if (first != last && *first != '\0') + throw torrent::input_error("Junk at end of input."); + + return result; +} + torrent::Object parse_command_multiple(target_type target, const char* first, const char* last) { parse_command_type result; diff --git a/src/rpc/parse_commands.h b/src/rpc/parse_commands.h index e51155f3..cf20c87a 100644 --- a/src/rpc/parse_commands.h +++ b/src/rpc/parse_commands.h @@ -37,6 +37,9 @@ #include #include +#include +#include +#include #include "xmlrpc.h" #include "rpc_manager.h" @@ -67,6 +70,34 @@ parse_command_single(target_type target, const std::string& cmd) { return parse_command(target, cmd.c_str(), cmd.c_str() + cmd.size()).first; } +// Parse one RPC command without executing it. Repeated evaluations can use +// call_object on the result, which handles per-target argument expansion. +torrent::Object parse_command_object(const char* first, const char* last); + +inline torrent::Object parse_command_object(const std::string& cmd) { + return parse_command_object(cmd.c_str(), cmd.c_str() + cmd.size()); +} + +// Prepare a multicall's commands once, on the first target that uses them. This +// keeps empty target lists from parsing commands that would never be evaluated. +struct preparsed_commands : public std::vector { + explicit preparsed_commands(std::function prepare) + : m_prepare(std::move(prepare)) {} + + void prepare_if_needed() { + if (m_prepare) { + // Clear before invoking: the callback may inspect this vector, and a + // throwing callback must not be run again against partially added items. + auto prepare = std::move(m_prepare); + m_prepare = {}; + prepare(*this); + } + } + +private: + std::function m_prepare; +}; + inline torrent::Object parse_command_multiple_std(const std::string& cmd, target_type target = rpc::make_target()) { return parse_command_multiple(target, cmd.c_str(), cmd.c_str() + cmd.size()); diff --git a/test/rpc/test_command.cc b/test/rpc/test_command.cc index 969b68f7..2ee12f24 100644 --- a/test/rpc/test_command.cc +++ b/test/rpc/test_command.cc @@ -2,7 +2,10 @@ #include "test/rpc/test_command.h" +#include + #include "rpc/command.h" +#include "rpc/parse_commands.h" CPPUNIT_TEST_SUITE_REGISTRATION(TestCommand); @@ -83,3 +86,50 @@ TestCommand::test_stack_double() { rpc::command_base::pop_stack(&stack_first, last_stack_first); CPPUNIT_ASSERT(command_stack_all_empty()); } + +void +TestCommand::test_preparsed_commands() { + unsigned int prepare_count = 0; + rpc::preparsed_commands commands([&prepare_count](auto& prepared) { + ++prepare_count; + // Reentrant access must not invoke the same callback recursively. + prepared.prepare_if_needed(); + prepared.push_back(rpc::parse_command_object("string.length=abc")); + }); + + CPPUNIT_ASSERT_EQUAL(0u, prepare_count); + CPPUNIT_ASSERT(commands.empty()); + + commands.prepare_if_needed(); + CPPUNIT_ASSERT_EQUAL(1u, prepare_count); + + size_t count = 0; + for (auto& itr : commands) { + CPPUNIT_ASSERT(itr.is_dict_key()); + ++count; + } + + CPPUNIT_ASSERT_EQUAL(size_t(1), count); + CPPUNIT_ASSERT_EQUAL(1u, prepare_count); + + for (auto& itr : commands) + CPPUNIT_ASSERT(itr.is_dict_key()); + + CPPUNIT_ASSERT_EQUAL(1u, prepare_count); +} + +void +TestCommand::test_parse_command_object() { + auto command = rpc::parse_command_object("\tstring.length=abc "); + CPPUNIT_ASSERT(command.is_dict_key()); + CPPUNIT_ASSERT_EQUAL(std::string("string.length"), command.as_dict_key()); + + // This helper has no way to return the next-command pointer, so it must not + // silently accept a multipart command separated by ';'. + CPPUNIT_ASSERT_THROW(rpc::parse_command_object("string.length=abc;string.length=def"), torrent::input_error); + + // Unlike parse_command (which parses command files), this helper handles one + // multicall command and must reject newline boundaries, including CRLF. + CPPUNIT_ASSERT_THROW(rpc::parse_command_object("string.length=abc\nstring.length=def"), torrent::input_error); + CPPUNIT_ASSERT_THROW(rpc::parse_command_object("string.length=abc\r\nstring.length=def"), torrent::input_error); +} diff --git a/test/rpc/test_command.h b/test/rpc/test_command.h index 68ce4dcd..16d19e70 100644 --- a/test/rpc/test_command.h +++ b/test/rpc/test_command.h @@ -5,10 +5,14 @@ class TestCommand : public test_fixture { CPPUNIT_TEST(test_stack); CPPUNIT_TEST(test_stack_double); + CPPUNIT_TEST(test_preparsed_commands); + CPPUNIT_TEST(test_parse_command_object); CPPUNIT_TEST_SUITE_END(); public: void test_stack(); void test_stack_double(); + void test_preparsed_commands(); + void test_parse_command_object(); };