Reverted parse value to strtoll with restrictions on input.

`confirm_finished` did, and the `input_error` reached `main`.

configure.ac

@@ -1,6 +1,6 @@
 m4_pattern_allow([PKG_CHECK_EXISTS])
 
-AC_INIT([rtorrent],[0.16.25],[sundell.software@gmail.com])
+AC_INIT([rtorrent],[0.16.24],[sundell.software@gmail.com])
 
 AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_MACRO_DIRS([scripts])
@@ -14,7 +14,7 @@ AX_CXX_COMPILE_STDCXX(20, noext, mandatory)
 
 PKG_PROG_PKG_CONFIG
 
-AC_DEFINE([API_VERSION], [29], [api version])
+AC_DEFINE([API_VERSION], [28], [api version])
 
 RAK_CHECK_CFLAGS
 RAK_CHECK_CXXFLAGS
@@ -49,7 +49,7 @@ fi
 
 PKG_CHECK_MODULES([CPPUNIT], [cppunit],, [no_cppunit="yes"])
 PKG_CHECK_MODULES([ZLIB], [zlib])
-PKG_CHECK_MODULES([DEPENDENCIES], [libtorrent >= 0.16.25])
+PKG_CHECK_MODULES([DEPENDENCIES], [libtorrent >= 0.16.24])
 
 AC_LANG_PUSH(C++)
 TORRENT_WITH_XMLRPC_C

src/command_download.cc

@@ -334,11 +334,6 @@ 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());
@@ -354,10 +349,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();
 
-    // Defer parsing until a file actually matches the multicall selection.
+    for (torrent::Object::list_const_iterator cItr = ++args.begin(); cItr != args.end(); cItr++) {
-    commands.prepare_if_needed();
+      const std::string& cmd = cItr->as_string();
-    for (auto& itr : commands)
+      row.push_back(rpc::parse_command(rpc::make_target(file.get()), cmd.c_str(), cmd.c_str() + cmd.size()).first);
-      row.push_back(rpc::call_object(itr, rpc::make_target(file.get())));
+    }
   }
 
   return resultRaw;
@@ -377,11 +372,6 @@ 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);
@@ -389,10 +379,11 @@ t_multicall(core::Download* download, const torrent::Object::list_type& args) {
     if (!tracker.is_valid())
       continue;
 
-    // Do not parse columns when there are no valid tracker targets.
+    for (auto cItr = ++args.begin(); cItr != args.end(); cItr++) {
-    commands.prepare_if_needed();
+      auto& cmd = cItr->as_string();
-    for (auto& itr : commands)
+
-      row.push_back(rpc::call_object(itr, rpc::make_target(&tracker)));
+      row.push_back(rpc::parse_command(rpc::make_target(&tracker), cmd.c_str(), cmd.c_str() + cmd.size()).first);
+    }
   }
 
   return result_raw;
@@ -414,18 +405,13 @@ 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();
 
-    // Prepare only after a peer exists, preserving empty-list laziness.
+    for (auto cItr = ++args.begin(); cItr != args.end(); cItr++) {
-    commands.prepare_if_needed();
+      const std::string& cmd = cItr->as_string();
-    for (auto& itr : commands) {
+
-      row.push_back(rpc::call_object(itr, rpc::make_target(connection)));
+      row.push_back(rpc::parse_command(rpc::make_target(connection), cmd.c_str(), cmd.c_str() + cmd.size()).first);
 
       // 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.

src/command_events.cc

@@ -247,26 +247,20 @@ 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();
 
-    // Skip parsing if there are no usable download targets in the view.
+    for (torrent::Object::list_const_iterator cItr = ++args.begin(); cItr != args.end(); cItr++) {
-    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;
 
-      row.push_back(rpc::call_object(itr, rpc::make_target(download)));
+      auto& cmd = cItr->as_string();
+      row.push_back(rpc::parse_command(rpc::make_target(download), cmd.c_str(), cmd.c_str() + cmd.size()).first);
     }
   }
 

src/command_system.cc

@@ -86,10 +86,8 @@ initialize_command_system() {
   CMD_VAR_VALUE   ("system.file.split_size",       -1);
   CMD_VAR_STRING  ("system.file.split_suffix",     ".part");
 
-  CMD_ANY         ("system.file_name.replace_slash",         [](auto, auto)        { return torrent::runtime::client_config()->file_name_replace_slash(); });
+  CMD_ANY         ("system.file_name.replace_slash",     [](auto, auto)      { return torrent::runtime::client_config()->file_name_replace_slash(); });
-  CMD_ANY_STRING_V("system.file_name.replace_slash.set",     [](auto, auto& str)   { return torrent::runtime::client_config()->set_file_name_replace_slash(str); });
+  CMD_ANY_STRING_V("system.file_name.replace_slash.set", [](auto, auto& str) { return torrent::runtime::client_config()->set_file_name_replace_slash(str); });
-  CMD_ANY         ("system.file_name.allow_legacy_utf8",     [](auto, auto)        { return torrent::runtime::client_config()->file_name_allow_legacy_utf8(); });
-  CMD_ANY_VALUE_V ("system.file_name.allow_legacy_utf8.set", [](auto, auto& value) { return torrent::runtime::client_config()->set_file_name_allow_legacy_utf8(value); });
 
   CMD_ANY         ("system.file_status_cache.size",      [](auto, auto)      { return control->core()->file_status_cache()->size(); });
   CMD_ANY_V       ("system.file_status_cache.prune",     [](auto, auto)      { return control->core()->file_status_cache()->prune(); });
@@ -169,13 +167,10 @@ initialize_command_system() {
   rpc::rpc.mark_safe("system.time");
   rpc::rpc.mark_safe("system.time_seconds");
   rpc::rpc.mark_safe("system.time_usec");
-  rpc::rpc.mark_safe("system.torrent_name.use_sanitized");
   rpc::rpc.mark_safe("system.file.allocate");
   rpc::rpc.mark_safe("system.file.max_size");
   rpc::rpc.mark_safe("system.file.split_size");
   rpc::rpc.mark_safe("system.file.split_suffix");
-  rpc::rpc.mark_safe("system.file_name.replace_slash");
-  rpc::rpc.mark_safe("system.file_name.allow_legacy_utf8");
 
   rpc::rpc.mark_safe("system.sockets.size");
   rpc::rpc.mark_safe("system.sockets.max_size");

src/core/download_list.h

@@ -1,7 +1,6 @@
 #ifndef RTORRENT_CORE_DOWNLOAD_LIST_H
 #define RTORRENT_CORE_DOWNLOAD_LIST_H
 
-#include <cstdint>
 #include <iosfwd>
 #include <list>
 #include <memory>

src/rpc/command.h

@@ -108,7 +108,7 @@ public:
 
     static stack_type* from_data(char* data) { return reinterpret_cast<stack_type*>(data); }
 
-    alignas(optimal_alignment) char buffer[sizeof(torrent::Object) * max_arguments];
+    char buffer[sizeof(torrent::Object) * max_arguments];
   };
 
   command_base() : m_copy_helper(nullptr), m_dest_helper(nullptr) {}

src/rpc/exec_file.cc

@@ -1,7 +1,5 @@
 #include "config.h"
 
-#include <memory>
-
 #include "rpc/exec_file.h"
 
 // #include <cassert>
@@ -83,9 +81,8 @@ ExecFile::execute_object(const torrent::Object& rawArgs, int flags) {
   char*  argsBuffer[max_args];
   char** argsCurrent = argsBuffer;
 
-  // On the heap: a buffer of buffer_size does not belong on the stack of whichever thread runs the command.
+  // Size of value strings are less than 24.
-  auto   valueStorage = std::make_unique<char[]>(buffer_size + 1);
+  char   valueBuffer[buffer_size+1];
-  char*  valueBuffer  = valueStorage.get();
   char*  valueCurrent = valueBuffer;
 
   if (rawArgs.is_list()) {

src/rpc/exec_file.h

@@ -10,7 +10,7 @@ namespace rpc {
 class ExecFile {
 public:
   static constexpr unsigned int max_args    = 128;
-  static constexpr unsigned int buffer_size = 128 * 1024;
+  static constexpr unsigned int buffer_size = 4096;
 
   static constexpr int flag_expand_tilde = 0x1;
   static constexpr int flag_throw        = 0x2;

src/rpc/parse.cc

@@ -118,32 +118,27 @@ parse_value_nothrow(const char* src, int64_t* value, int base, int unit) {
   if (base != 0 && base != 8 && base != 10 && base != 16)
     throw torrent::input_error("Command::string_to_value_unit(...) received invalid base.");
 
-  const char* first = src;
-
   while (parse_is_space(*src))
     src++;
 
   if (src[0] == '+')
-    return first;
+    return src;
 
   if (src[0] == '-') {
     if (base == 8 || base == 16)
-      return first;
+      return src;
 
     if (src[1] == '0')
-      return first;
+      return src;
   }
 
-  if (base == 10 && src[0] == '0' && (src[1] >= '0' && src[1] <= '9'))
-    return first;
-
   char* last{};
 
   errno  = 0;
   *value = strtoll(src, &last, base);
 
   if (errno == ERANGE)
-    return first;
+    return src;
 
   if (last == src) {
     *value = 0;
@@ -153,7 +148,7 @@ parse_value_nothrow(const char* src, int64_t* value, int base, int unit) {
     if (strcasecmp(src, "true") == 0)  { *value = 1; return src + strlen("true"); }
     if (strcasecmp(src, "false") == 0) { *value = 0; return src + strlen("false"); }
 
-    return first;
+    return src;
   }
 
   switch (*last) {
@@ -161,15 +156,15 @@ parse_value_nothrow(const char* src, int64_t* value, int base, int unit) {
   case 'B': ++last; break;
   case 'k':
   case 'K':
-    if (!value_fits_shifted(*value, 10)) return first; // overflow guard
+    if (!value_fits_shifted(*value, 10)) return src; // overflow guard
     *value = *value << 10; ++last; break;
   case 'm':
   case 'M':
-    if (!value_fits_shifted(*value, 20)) return first; // overflow guard
+    if (!value_fits_shifted(*value, 20)) return src; // overflow guard
     *value = *value << 20; ++last; break;
   case 'g':
   case 'G':
-    if (!value_fits_shifted(*value, 30)) return first; // overflow guard
+    if (!value_fits_shifted(*value, 30)) return src; // overflow guard
     *value = *value << 30; ++last; break;
 //   case ' ':
 //   case '\0': *value = *value * unit; break;
@@ -177,7 +172,7 @@ parse_value_nothrow(const char* src, int64_t* value, int base, int unit) {
   default:
     if (*value > std::numeric_limits<int64_t>::max() / unit ||
         *value < std::numeric_limits<int64_t>::min() / unit)
-      return first; // overflow guard
+      return src; // overflow guard
 
     *value = *value * unit;
     break;

src/rpc/parse_commands.cc

@@ -123,39 +123,6 @@ 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;

src/rpc/parse_commands.h

@@ -37,9 +37,6 @@
 
 #include <string>
 #include <cstring>
-#include <functional>
-#include <utility>
-#include <vector>
 
 #include "xmlrpc.h"
 #include "rpc_manager.h"
@@ -70,34 +67,6 @@ 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<torrent::Object> {
-  explicit preparsed_commands(std::function<void(preparsed_commands&)> 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<void(preparsed_commands&)> 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());

test/Makefile.am

@@ -71,18 +71,10 @@ rtorrent_Test_Src_SOURCES = $(rtorrent_Test_Common) \
 	src/test_command_path.h \
 	src/test_command_string.cc \
 	src/test_command_string.h \
-	src/test_command_throttle.cc \
-	src/test_command_throttle.h \
 	src/test_command_tracker.cc \
 	src/test_command_tracker.h \
 	src/test_download_list.cc \
 	src/test_download_list.h \
-	src/test_input_path_input.cc \
-	src/test_input_path_input.h \
-	src/test_session_commit.cc \
-	src/test_session_commit.h \
-	src/test_session_storer.cc \
-	src/test_session_storer.h \
 	src/test_setup.cc \
 	src/test_setup.h \
 	src/test_ui_download_list.cc \

test/rpc/test_command.cc

@@ -2,10 +2,7 @@
 
 #include "test/rpc/test_command.h"
 
-#include <torrent/exceptions.h>
-
 #include "rpc/command.h"
-#include "rpc/parse_commands.h"
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestCommand);
 
@@ -86,50 +83,3 @@ 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);
-}

test/rpc/test_command.h

@@ -5,14 +5,10 @@ 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();
 };