Add monitor placeholder validation and remove unused function

- Remove unused get_random_command() overload (parameterless version)
- Add strict validation for monitor placeholders (__monitor_cN__):
  - Reject malformed placeholders like __monitor_c1, __monitor_c1_,
    __monitor_c1__garbage, __monitor_c1abc__, __monitor_c__
  - Validation only applies when --monitor-input is provided
- Add tests for malformed placeholder rejection
- Add test verifying placeholder-like strings are literal without --monitor-input

Author: fcostaoliveira

config_types.cpp

@@ -530,16 +530,6 @@ const std::string &monitor_command_list::get_command(size_t index) const
     return commands[index];
 }
 
-const std::string &monitor_command_list::get_random_command() const
-{
-    if (commands.empty()) {
-        static std::string empty;
-        return empty;
-    }
-    size_t random_index = rand() % commands.size();
-    return commands[random_index];
-}
-
 const std::string &monitor_command_list::get_random_command(size_t *out_index) const
 {
     if (commands.empty()) {

config_types.h

@@ -209,7 +209,6 @@ struct monitor_command_list
 
     bool load_from_file(const char *filename);
     const std::string &get_command(size_t index) const;
-    const std::string &get_random_command() const;
     const std::string &get_random_command(size_t *out_index) const;
     const std::string &get_next_sequential_command(size_t *out_index);
     size_t size() const { return commands.size(); }

memtier_benchmark.cpp

@@ -1987,14 +1987,35 @@ int main(int argc, char *argv[])
             }
 
             // Check if command is a specific monitor placeholder
-            if (strncmp(cmd.command.c_str(), MONITOR_PLACEHOLDER_PREFIX, strlen(MONITOR_PLACEHOLDER_PREFIX)) == 0) {
-                // Extract the index from __monitor_cN__
-                const char *num_start = cmd.command.c_str() + strlen(MONITOR_PLACEHOLDER_PREFIX);
-                char *endptr;
-                long index = strtol(num_start, &endptr, 10);
-
-                if (endptr == num_start || index < 1 || (size_t) index > cfg.monitor_commands->size()) {
-                    fprintf(stderr, "error: invalid monitor placeholder '%s' (valid range: q1-q%zu or q@)\n",
+            // Expected format: __monitor_cN__ where N is a 1-based index
+            const size_t prefix_len = sizeof(MONITOR_PLACEHOLDER_PREFIX) - 1; // compile-time constant
+            const size_t suffix_len = 2;                                      // "__"
+            size_t cmd_len = cmd.command.length();
+
+            // Check if command starts with monitor prefix
+            if (cmd_len >= prefix_len && strncmp(cmd.command.c_str(), MONITOR_PLACEHOLDER_PREFIX, prefix_len) == 0) {
+                // Validate full format: must end with __, have digits between prefix and suffix
+                bool valid = false;
+                long index = 0;
+
+                if (cmd_len > prefix_len + suffix_len && cmd.command[cmd_len - 2] == '_' &&
+                    cmd.command[cmd_len - 1] == '_') {
+                    // Extract the index from __monitor_cN__
+                    const char *num_start = cmd.command.c_str() + prefix_len;
+                    char *endptr;
+                    index = strtol(num_start, &endptr, 10);
+
+                    // Valid if: consumed at least one digit, ends exactly at "__", index in range
+                    if (endptr != num_start && endptr == cmd.command.c_str() + cmd_len - suffix_len && index >= 1 &&
+                        (size_t) index <= cfg.monitor_commands->size()) {
+                        valid = true;
+                    }
+                }
+
+                if (!valid) {
+                    fprintf(stderr,
+                            "error: invalid monitor placeholder '%s' (valid range: __monitor_c1__ to __monitor_c%zu__ "
+                            "or __monitor_c@__)\n",
                             cmd.command.c_str(), cfg.monitor_commands->size());
                     exit(1);
                 }

tests/test_monitor_input.py

@@ -325,3 +325,133 @@ def test_monitor_input_mixed_commands(env):
             found = True
             break
     env.assertTrue(found)
+
+
+def test_monitor_input_malformed_placeholder_rejected(env):
+    """
+    Test that malformed monitor placeholders are rejected with an error.
+
+    This test verifies that placeholders like:
+    - __monitor_c1 (missing trailing __)
+    - __monitor_c1_ (only one trailing underscore)
+    - __monitor_c1__garbage (trailing characters)
+    - __monitor_c1abc__ (non-numeric characters before __)
+    are properly rejected when --monitor-input is provided.
+    """
+    # Create monitor input file
+    test_dir = tempfile.mkdtemp()
+    monitor_file = os.path.join(test_dir, "monitor.txt")
+    with open(monitor_file, "w") as f:
+        f.write(
+            '[ proxy49 ] 1764031576.604009 [0 172.16.10.147:51682] "SET" "key1" "value1"\n'
+        )
+
+    malformed_placeholders = [
+        "__monitor_c1",        # missing trailing __
+        "__monitor_c1_",       # only one trailing underscore
+        "__monitor_c1__garbage",  # trailing characters after valid placeholder
+        "__monitor_c1abc__",   # non-numeric characters before __
+        "__monitor_c__",       # missing index number
+    ]
+
+    config = get_default_memtier_config(threads=1, clients=1, requests=10)
+    master_nodes_list = env.getMasterNodesList()
+
+    for placeholder in malformed_placeholders:
+        benchmark_specs = {
+            "name": env.testName,
+            "args": [
+                "--monitor-input={}".format(monitor_file),
+                "--command={}".format(placeholder),
+            ],
+        }
+        addTLSArgs(benchmark_specs, env)
+
+        add_required_env_arguments(benchmark_specs, config, env, master_nodes_list)
+
+        run_config = RunConfig(test_dir, env.testName, config, {})
+        ensure_clean_benchmark_folder(run_config.results_dir)
+
+        benchmark = Benchmark.from_json(run_config, benchmark_specs)
+
+        # Run memtier_benchmark - should fail
+        memtier_ok = benchmark.run()
+
+        # Verify failure
+        if memtier_ok:
+            env.debugPrint(
+                "Expected failure for malformed placeholder '{}' but it succeeded".format(
+                    placeholder
+                ),
+                True,
+            )
+        env.assertFalse(memtier_ok)
+
+        # Check stderr for error message
+        stderr_file = "{0}/mb.stderr".format(run_config.results_dir)
+        if os.path.isfile(stderr_file):
+            with open(stderr_file) as stderr:
+                stderr_content = stderr.read()
+                # The placeholder should either be rejected as invalid monitor placeholder
+                # or treated as unknown command by Redis
+                has_error = (
+                    "invalid monitor placeholder" in stderr_content
+                    or "error" in stderr_content.lower()
+                )
+                if not has_error:
+                    env.debugPrint(
+                        "Expected error message for '{}', got: {}".format(
+                            placeholder, stderr_content
+                        ),
+                        True,
+                    )
+                env.assertTrue(has_error)
+
+
+def test_monitor_placeholder_literal_without_monitor_input(env):
+    """
+    Test that __monitor_c1__ is treated as a literal command when --monitor-input is not provided.
+
+    When monitor input is not configured, placeholder-like strings should be sent
+    to Redis as-is (which will result in an unknown command error from Redis).
+    This verifies that the placeholder validation only applies when monitor is in use.
+    """
+    test_dir = tempfile.mkdtemp()
+
+    # Configure memtier WITHOUT --monitor-input but WITH a monitor-like placeholder
+    benchmark_specs = {
+        "name": env.testName,
+        "args": [
+            "--command=__monitor_c1__",  # This should be sent literally to Redis
+        ],
+    }
+    addTLSArgs(benchmark_specs, env)
+
+    config = get_default_memtier_config(threads=1, clients=1, requests=10)
+    master_nodes_list = env.getMasterNodesList()
+
+    add_required_env_arguments(benchmark_specs, config, env, master_nodes_list)
+
+    config = RunConfig(test_dir, env.testName, config, {})
+    ensure_clean_benchmark_folder(config.results_dir)
+
+    benchmark = Benchmark.from_json(config, benchmark_specs)
+
+    # Run memtier_benchmark
+    # This should NOT fail with "invalid monitor placeholder" error
+    # It will likely fail because Redis doesn't recognize __monitor_c1__ as a command,
+    # but that's a Redis error, not a memtier validation error
+    benchmark.run()  # Result doesn't matter - we only care about the error type
+
+    # Check stderr - should NOT contain "invalid monitor placeholder"
+    stderr_file = "{0}/mb.stderr".format(config.results_dir)
+    if os.path.isfile(stderr_file):
+        with open(stderr_file) as stderr:
+            stderr_content = stderr.read()
+            has_monitor_error = "invalid monitor placeholder" in stderr_content
+            if has_monitor_error:
+                env.debugPrint(
+                    "Placeholder validation should not apply without --monitor-input",
+                    True,
+                )
+            env.assertFalse(has_monitor_error)