This is an automated email from the ASF dual-hosted git repository.

jacktengg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new 1503e390275 [fix](be) Quote shell arguments in profiling and Python 
tools (#70207)
1503e390275 is described below

commit 1503e390275c7c2bcf814907776491b8cbf3ccc0
Author: TengJianPing <[email protected]>
AuthorDate: Thu Nov 5 17:31:07 2026 +0800

    [fix](be) Quote shell arguments in profiling and Python tools (#70207)
    
    ### What problem does this PR solve?
    
    Issue Number: None
    
    Related PR: None
    
    Problem Summary:
    
    BE passes configured paths to a shell when it runs profiling tools. The
    shell can treat parts of those paths as commands. For example,
    `profile$(printf injected).heap` becomes `profileinjected.heap` before
    the profiling tool starts.
    
    This change quotes each variable argument in the jeprof, pprof, and perf
    commands. It also quotes the tool paths and the flame graph output path.
    The same rule applies to Python version checks, environment creation,
    and package lists. Double quotes alone do not stop command substitution
    in these Python paths.
    
    The bundled jeprof also checks for an existing local file before it
    expands remote profile groups. For example, a local directory named
    `safe@1` keeps its original name. The tool's own shell encoder now
    escapes every apostrophe. These changes prevent a local profile path
    from entering the tool's remote command path through the group syntax.
    
    The heap dump code copies `jeprofile_dir` under the configuration update
    lock. Directory creation and file creation then use the same copy. The
    shell command helper rejects NUL bytes before it calls `popen`. The
    pprof content path removes the temporary file that it created.
    
    Related paths checked:
    
    - `/jeheap/dump`: a mutable `jeprofile_dir` reaches the jeprof command.
    - `/pprof/heap?readable=1` and `/pprof/profile?type=text`: configured
    paths reach pprof.
    - `/pprof/profile?type=flamegraph`: configured paths reach perf, both
    pipeline tools, and file redirection.
    - Python environment checks and `get_python_packages`: operator paths
    reach shell commands.
    - The HTTP `type addr2line` command has no variable argument.
    - The CDC launcher passes an argument array to `execv`.
    
    The threat model classifies BE 8040 as an internal port. Network
    isolation remains required. Anonymous access to this port is out of
    model under sections 4.3 and 4.4. This change removes command
    interpretation from path values. It does not change the HTTP
    authentication default. The Python paths require operator configuration
    and Python UDF support.
    
    ### Release note
    
    BE now quotes paths in profiling and Python environment commands. The BE
    shell command no longer interprets shell characters in those arguments.
    The bundled jeprof preserves existing local profile paths that contain
    `@N` and quotes all apostrophes in its own commands.
    
    ### Check List (For Author)
    
    - Test:
      - ASAN BE build: `./build.sh --be -j32` passed.
    - Unit tests: 47 tests passed in `AgentUtilsTest`,
    `ProfilerCommandTest`, and `PythonEnvTest`. This includes the bundled
    jeprof unit tests and a real-tool local-profile case.
      - clang-format 16, clang-tidy, and BE header hygiene checks passed.
    - Manual test: the local BE accepted and returned the path unchanged.
    The original configuration was restored.
    - Manual test: ASAN HTTP profiling handlers returned their expected
    disabled responses.
    - Manual test: the installed jeprof read an empty profile with quotes
    and shell characters in its path. It returned `No nodes to print`.
    - Manual test: before/after comparison with a local `safe@1` directory.
    The old tool selected its remote path. The fixed tool read the local
    profile. A curl stub blocked network access in both runs.
    - Behavior changed: Yes. Shell characters in paths remain part of the
    path. Commands with NUL bytes fail before they start.
    - Does this need documentation: No. The API and configuration names are
    unchanged.
    
    ### Minimal reproduction
    
    The argument tests use small local scripts as profiling tools. They pass
    paths with spaces, repeated quotes, and `$(printf injected)` through the
    production code. The scripts must receive the original paths. A separate
    integration case calls the bundled jeprof with a two-line profile that
    has no samples. It uses a `safe@1` directory and blocks download
    commands. The bundled tool also tests a shell round trip with `a'b'c`.
    These tests need no tables or rows.
    
    ```sh
    ./run-be-ut.sh --run 
--filter='AgentUtilsTest.*:ProfilerCommandTest.*:PythonEnvTest.*' -j32
    ```
    
    ASAN builds disable the HTTP sampling handlers. The tests call the
    production conversion functions directly to check the shell boundary.
---
 be/src/agent/utils.cpp                  |  20 ++++
 be/src/agent/utils.h                    |   5 +-
 be/src/runtime/memory/heap_profiler.cpp |  30 +++---
 be/src/runtime/memory/heap_profiler.h   |   1 +
 be/src/udf/python/python_env.cpp        |  12 ++-
 be/src/util/pprof_utils.cpp             |  38 ++++---
 be/test/agent/utils_test.cpp            |  21 ++++
 be/test/udf/python/python_env_test.cpp  |  14 ++-
 be/test/util/profiler_command_test.cpp  | 170 ++++++++++++++++++++++++++++++++
 tools/jeprof                            |  23 ++++-
 10 files changed, 300 insertions(+), 34 deletions(-)

diff --git a/be/src/agent/utils.cpp b/be/src/agent/utils.cpp
index df8d21718cc..f4b4cf31f13 100644
--- a/be/src/agent/utils.cpp
+++ b/be/src/agent/utils.cpp
@@ -215,7 +215,27 @@ Status MasterServerClient::confirm_unused_remote_files(
     return Status::OK();
 }
 
+std::string AgentUtils::shell_quote(const std::string& argument) {
+    std::string quoted = "'";
+    for (char ch : argument) {
+        // A single quote ends a quoted section. Insert it between two quoted 
sections.
+        // For example, a'b becomes 'a'\''b'.
+        if (ch == '\'') {
+            quoted += "'\\''";
+        } else {
+            quoted += ch;
+        }
+    }
+    quoted += '\'';
+    return quoted;
+}
+
 bool AgentUtils::exec_cmd(const std::string& command, std::string* errmsg, 
bool redirect_stderr) {
+    // popen reads a C string. A NUL byte would discard the rest of the 
command.
+    if (command.find('\0') != std::string::npos) {
+        *errmsg = "The shell command contains a NUL byte.\n";
+        return false;
+    }
     // The exit status of the command.
     uint32_t rc = 0;
 
diff --git a/be/src/agent/utils.h b/be/src/agent/utils.h
index 42c59718861..57c258e7a87 100644
--- a/be/src/agent/utils.h
+++ b/be/src/agent/utils.h
@@ -76,7 +76,10 @@ public:
     AgentUtils() = default;
     virtual ~AgentUtils() = default;
 
-    // Execute shell cmd
+    // Quote one shell argument. For example, "a;pwd" must remain one file 
name.
+    static std::string shell_quote(const std::string& argument);
+
+    // Execute a shell command. Use shell_quote for each variable argument.
     virtual bool exec_cmd(const std::string& command, std::string* errmsg,
                           bool redirect_stderr = true);
 
diff --git a/be/src/runtime/memory/heap_profiler.cpp 
b/be/src/runtime/memory/heap_profiler.cpp
index f8f6fee743e..c667c031c28 100644
--- a/be/src/runtime/memory/heap_profiler.cpp
+++ b/be/src/runtime/memory/heap_profiler.cpp
@@ -65,14 +65,16 @@ bool HeapProfiler::get_prof_dump(const std::string& 
profile_file_name) {
 #endif
 }
 
-static std::string jeprof_profile_to_dot(const std::string& profile_file_name) 
{
+std::string HeapProfiler::jeprof_profile_to_dot(const std::string& 
profile_file_name) {
     AgentUtils util;
-    const static std::string jeprof_path = fmt::format("{}/bin/jeprof", 
std::getenv("DORIS_HOME"));
-    const static std::string binary_path =
-            fmt::format("{}/lib/doris_be", std::getenv("DORIS_HOME"));
+    const std::string jeprof_path = fmt::format("{}/bin/jeprof", 
std::getenv("DORIS_HOME"));
+    const std::string binary_path = fmt::format("{}/lib/doris_be", 
std::getenv("DORIS_HOME"));
     // 
https://doris.apache.org/community/developer-guide/debug-tool/#3-jeprof-parses-heap-profile
-    std::string jeprof_cmd =
-            fmt::format("{} --dot {} {}", jeprof_path, binary_path, 
profile_file_name);
+    // The configuration can contain shell syntax. Keep paths such as 
"log$(pwd)" literal.
+    // The -- separator also prevents a path from becoming a jeprof option.
+    std::string jeprof_cmd = fmt::format("{} --dot -- {} {}", 
AgentUtils::shell_quote(jeprof_path),
+                                         AgentUtils::shell_quote(binary_path),
+                                         
AgentUtils::shell_quote(profile_file_name));
     std::string msg;
     bool rc = util.exec_cmd(jeprof_cmd, &msg);
     if (!rc) {
@@ -116,16 +118,22 @@ bool HeapProfiler::check_enable_heap_profiler() {
 }
 
 std::string HeapProfiler::dump_heap_profile() {
-    if (!config::jeprofile_dir.empty()) {
-        auto st = 
io::global_local_filesystem()->create_directory(config::jeprofile_dir);
+    std::string profile_dir;
+    {
+        // A configuration update can replace this string during a dump.
+        // Copy it under the update lock. Use the same directory for both file 
operations.
+        std::lock_guard<std::mutex> 
lock(*config::get_mutable_string_config_lock());
+        profile_dir = config::jeprofile_dir;
+    }
+    if (!profile_dir.empty()) {
+        auto st = io::global_local_filesystem()->create_directory(profile_dir);
         if (!st.ok()) {
             LOG(WARNING) << "create jeprofile dir failed.";
             return "";
         }
     }
-    std::string profile_file_name =
-            fmt::format("{}/jeheap_dump.{}.{}.{}.heap", config::jeprofile_dir, 
std::time(nullptr),
-                        getpid(), rand());
+    std::string profile_file_name = 
fmt::format("{}/jeheap_dump.{}.{}.{}.heap", profile_dir,
+                                                std::time(nullptr), getpid(), 
rand());
     if (get_prof_dump(profile_file_name)) {
         return profile_file_name;
     } else {
diff --git a/be/src/runtime/memory/heap_profiler.h 
b/be/src/runtime/memory/heap_profiler.h
index 84e9d084b73..1043cd47c7e 100644
--- a/be/src/runtime/memory/heap_profiler.h
+++ b/be/src/runtime/memory/heap_profiler.h
@@ -36,6 +36,7 @@ public:
     std::string dump_heap_profile_to_dot();
 
 private:
+    static std::string jeprof_profile_to_dot(const std::string& 
profile_file_name);
     void set_prof_active(bool prof);
     bool get_prof_dump(const std::string& profile_file_name);
 
diff --git a/be/src/udf/python/python_env.cpp b/be/src/udf/python/python_env.cpp
index ef7eb77eb7a..7e79a0ae29a 100644
--- a/be/src/udf/python/python_env.cpp
+++ b/be/src/udf/python/python_env.cpp
@@ -26,6 +26,7 @@
 #include <regex>
 #include <vector>
 
+#include "agent/utils.h"
 #include "common/status.h"
 #include "udf/python/python_server.h"
 #include "util/string_util.h"
@@ -54,7 +55,8 @@ static Status extract_python_version(const std::string& 
python_path, std::string
         return Status::NotFound("Python executable not found: {}", 
python_path);
     }
 
-    std::string cmd = fmt::format("\"{}\" --version", python_path);
+    // Double quotes still expand shell expressions such as $(pwd). Keep the 
path literal.
+    std::string cmd = fmt::format("{} --version", 
AgentUtils::shell_quote(python_path));
     FILE* pipe = popen(cmd.c_str(), "r");
     if (!pipe) {
         return Status::InternalError("Failed to run: {}", cmd);
@@ -177,7 +179,8 @@ Status PythonEnvironment::scan_from_venv_root_path(
             // Use --system-site-packages to inherit packages from system 
Python
             // This ensures pandas and pyarrow are available if installed in 
system
             std::string create_venv_cmd = fmt::format("{} -m venv 
--system-site-packages {}",
-                                                      interpreter_path, 
env_base_path);
+                                                      
AgentUtils::shell_quote(interpreter_path),
+                                                      
AgentUtils::shell_quote(env_base_path));
 
             if (system(create_venv_cmd.c_str()) != 0 || 
!fs::exists(python_path)) {
                 return Status::RuntimeError("Failed to create python virtual 
environment, cmd: {}",
@@ -340,8 +343,9 @@ Status list_installed_packages(const PythonVersion& version,
     }
 
     // Run pip list --format=json to get installed packages
-    std::string cmd =
-            fmt::format("\"{}\" -m pip list --format=json 2>/dev/null", 
version.executable_path);
+    // Use the same path quoting as version discovery and environment creation.
+    std::string cmd = fmt::format("{} -m pip list --format=json 2>/dev/null",
+                                  
AgentUtils::shell_quote(version.executable_path));
     FILE* pipe = popen(cmd.c_str(), "r");
     if (!pipe) {
         return Status::InternalError("Failed to run pip list for python 
version: {}",
diff --git a/be/src/util/pprof_utils.cpp b/be/src/util/pprof_utils.cpp
index 8f8cd7578e4..eadf07192d5 100644
--- a/be/src/util/pprof_utils.cpp
+++ b/be/src/util/pprof_utils.cpp
@@ -35,15 +35,17 @@ extern std::string pprof_profile_dir;
 
 Status PprofUtils::get_pprof_cmd(std::string* cmd) {
     AgentUtils util;
-    // check if pprof cmd exist
-    const static std::string tools_path = 
std::string(std::getenv("DORIS_HOME")) + "/tools/bin/";
+    // The startup script sets DORIS_HOME before the BE starts.
+    const char* doris_home = std::getenv("DORIS_HOME");
+    DORIS_CHECK(doris_home != nullptr);
+    const std::string tools_path = std::string(doris_home) + "/tools/bin/";
     std::string pprof_cmd = tools_path + "pprof";
     std::string msg;
-    bool rc = util.exec_cmd(pprof_cmd + " --version", &msg);
+    bool rc = util.exec_cmd(AgentUtils::shell_quote(pprof_cmd) + " --version", 
&msg);
     if (!rc) {
         // not found in BE tools dir, found in system
         pprof_cmd = "pprof";
-        rc = util.exec_cmd(pprof_cmd + " --version", &msg);
+        rc = util.exec_cmd(AgentUtils::shell_quote(pprof_cmd) + " --version", 
&msg);
         if (!rc) {
             return Status::NotSupported(
                     "pprof: command not found in systemp PATH or 
be/tools/bin/. Install gperftools "
@@ -59,7 +61,7 @@ Status PprofUtils::get_perf_cmd(std::string* cmd) {
     // check if perf cmd exist
     std::string perf_cmd = "perf";
     std::string msg;
-    bool rc = util.exec_cmd(perf_cmd + " --version", &msg);
+    bool rc = util.exec_cmd(AgentUtils::shell_quote(perf_cmd) + " --version", 
&msg);
     if (!rc) {
         return Status::NotSupported("perf: command not found in systemp PATH");
     }
@@ -111,13 +113,16 @@ Status PprofUtils::get_readable_profile(const 
std::string& file_or_content, bool
 
     // parse raw with "pprof --text cmdline raw_file"
     std::string cmd_output;
-    std::string final_cmd = pprof_cmd + absl::Substitute(" --text $0 $1", 
self_cmdline, final_file);
+    // Quote each path. For example, "log;pwd/profile" is one path, not two 
commands.
+    std::string final_cmd = absl::Substitute(
+            "$0 --text -- $1 $2", AgentUtils::shell_quote(pprof_cmd),
+            AgentUtils::shell_quote(self_cmdline), 
AgentUtils::shell_quote(final_file));
     AgentUtils util;
     LOG(INFO) << "begin to run command: " << final_cmd;
     bool rc = util.exec_cmd(final_cmd, &cmd_output, false);
 
     // delete raw file
-    
static_cast<void>(io::global_local_filesystem()->delete_file(file_or_content));
+    static_cast<void>(io::global_local_filesystem()->delete_file(final_file));
 
     if (!rc) {
         return Status::InternalError("Failed to execute command: {}", 
cmd_output);
@@ -153,8 +158,9 @@ Status PprofUtils::generate_flamegraph(int32_t 
sample_seconds,
 
     // sample
     std::stringstream cmd;
-    cmd << perf_cmd << " record -m 2 -g -p " << getpid() << " -o " << 
tmp_file.str() << " -- sleep "
-        << sample_seconds;
+    // Keep configured paths as arguments, including paths with spaces or 
shell syntax.
+    cmd << AgentUtils::shell_quote(perf_cmd) << " record -m 2 -g -p " << 
getpid() << " -o "
+        << AgentUtils::shell_quote(tmp_file.str()) << " -- sleep " << 
sample_seconds;
 
     AgentUtils util;
     std::string cmd_output;
@@ -173,8 +179,12 @@ Status PprofUtils::generate_flamegraph(int32_t 
sample_seconds,
         graph_file << config::pprof_profile_dir << "/flamegraph." << getpid() 
<< "." << rand()
                    << ".svg";
         std::stringstream gen_cmd;
-        gen_cmd << perf_cmd << " script -i " << tmp_file.str() << " | " << 
stackcollapse_perf_pl
-                << " | " << flamegraph_pl << " > " << graph_file.str();
+        // Only these fixed pipe and redirection operators are shell syntax.
+        gen_cmd << AgentUtils::shell_quote(perf_cmd) << " script -i "
+                << AgentUtils::shell_quote(tmp_file.str()) << " | "
+                << AgentUtils::shell_quote(stackcollapse_perf_pl) << " | "
+                << AgentUtils::shell_quote(flamegraph_pl) << " > "
+                << AgentUtils::shell_quote(graph_file.str());
         LOG(INFO) << "begin to run command: " << gen_cmd.str();
         rc = util.exec_cmd(gen_cmd.str(), &res_content);
         if (!rc) {
@@ -185,8 +195,10 @@ Status PprofUtils::generate_flamegraph(int32_t 
sample_seconds,
         *svg_file_or_content = graph_file.str();
     } else {
         std::stringstream gen_cmd;
-        gen_cmd << perf_cmd << " script -i " << tmp_file.str() << " | " << 
stackcollapse_perf_pl
-                << " | " << flamegraph_pl;
+        gen_cmd << AgentUtils::shell_quote(perf_cmd) << " script -i "
+                << AgentUtils::shell_quote(tmp_file.str()) << " | "
+                << AgentUtils::shell_quote(stackcollapse_perf_pl) << " | "
+                << AgentUtils::shell_quote(flamegraph_pl);
         LOG(INFO) << "begin to run command: " << gen_cmd.str();
         rc = util.exec_cmd(gen_cmd.str(), &res_content, false);
         if (!rc) {
diff --git a/be/test/agent/utils_test.cpp b/be/test/agent/utils_test.cpp
index dd32a219a0c..658997f31dd 100644
--- a/be/test/agent/utils_test.cpp
+++ b/be/test/agent/utils_test.cpp
@@ -45,4 +45,25 @@ TEST(AgentUtilsTest, Test) {
     }
 }
 
+// The shell must return each input unchanged. No profiler or table data is 
required.
+TEST(AgentUtilsTest, ShellQuotePreservesOneArgument) {
+    for (const std::string& value :
+         {"", "a b", "a'b'c", "\"$HOME\"`printf injected`$(printf 
injected);|&<>*?\\\n#"}) {
+        std::string output;
+        AgentUtils util;
+        ASSERT_TRUE(util.exec_cmd("printf '<%s>' " + 
AgentUtils::shell_quote(value), &output));
+        EXPECT_EQ("<" + value + ">", output);
+    }
+}
+
+TEST(AgentUtilsTest, RejectNulBeforeRunningCommand) {
+    std::string command = "printf unexpected";
+    command += '\0';
+    command += "printf ignored";
+    std::string output;
+    AgentUtils util;
+    EXPECT_FALSE(util.exec_cmd(command, &output));
+    EXPECT_EQ("The shell command contains a NUL byte.\n", output);
+}
+
 } // namespace doris
diff --git a/be/test/udf/python/python_env_test.cpp 
b/be/test/udf/python/python_env_test.cpp
index 4e48ee71d7a..f8956d6aa11 100644
--- a/be/test/udf/python/python_env_test.cpp
+++ b/be/test/udf/python/python_env_test.cpp
@@ -186,8 +186,9 @@ TEST_F(PythonEnvTest, 
ScanFromVenvRootPathNonExistentInterpreter) {
 }
 
 TEST_F(PythonEnvTest, ScanFromVenvRootPathCreatesAndValidatesEnvironment) {
-    const std::string root_path = test_dir_ + "/venv_root";
-    const std::string interpreter_path = test_dir_ + "/python3.11";
+    // One environment is sufficient to test version discovery and creation 
with literal paths.
+    const std::string root_path = test_dir_ + "/venv 'root' $(printf 
injected)";
+    const std::string interpreter_path = test_dir_ + "/python '3.11' $(printf 
injected)";
     fs::create_directories(root_path);
 
     // Model the two interpreter operations used by the production VENV path: 
version discovery
@@ -579,6 +580,15 @@ TEST_F(PythonEnvTest, 
PythonVersionManagerInitCondaSuccess) {
 // list_installed_packages tests
 // ============================================================================
 
+TEST_F(PythonEnvTest, ListInstalledPackagesKeepsPathLiteral) {
+    const auto version = create_fake_python_version_for_pip_list(
+            test_dir_ + "/python 'path' $(printf injected)", "printf '[]'\n", 
"3.11.9");
+    std::vector<std::pair<std::string, std::string>> packages;
+    const auto status = list_installed_packages(version, &packages);
+    ASSERT_TRUE(status.ok()) << status;
+    EXPECT_TRUE(packages.empty());
+}
+
 TEST_F(PythonEnvTest, ListInstalledPackagesInvalidPythonVersion) {
     PythonVersion invalid_version;
     std::vector<std::pair<std::string, std::string>> packages;
diff --git a/be/test/util/profiler_command_test.cpp 
b/be/test/util/profiler_command_test.cpp
new file mode 100644
index 00000000000..e1cc31a26ed
--- /dev/null
+++ b/be/test/util/profiler_command_test.cpp
@@ -0,0 +1,170 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include <gtest/gtest.h>
+
+#include <cstdlib>
+#include <filesystem>
+#include <fstream>
+#include <sstream>
+#include <string>
+
+#include "agent/utils.h"
+#include "common/config.h"
+#include "runtime/memory/heap_profiler.h"
+#include "util/pprof_utils.h"
+
+namespace doris {
+
+// Scripts check the shell boundary. The jeprof integration test uses the 
bundled tool.
+class ProfilerCommandTest : public testing::Test {
+protected:
+    void SetUp() override {
+        _old_home = getenv("DORIS_HOME");
+        _old_path = getenv("PATH");
+        _old_profile_dir = config::pprof_profile_dir;
+        std::string pattern =
+                (std::filesystem::current_path() / 
"profiler_command.XXXXXX").string();
+        ASSERT_NE(mkdtemp(pattern.data()), nullptr);
+        _root = pattern;
+        _home = _root / "tools 'one''two' $(printf injected)";
+        _profile_dir = _root / "profiles 'one''two' $(printf injected);#";
+        std::filesystem::create_directories(_profile_dir);
+        config::pprof_profile_dir = _profile_dir.string();
+        ASSERT_EQ(setenv("DORIS_HOME", _home.c_str(), 1), 0);
+        ASSERT_EQ(setenv("PATH", (_home.string() + "/bin:" + 
_old_path).c_str(), 1), 0);
+        write_tool(_home / "bin/jeprof", "printf '%s\\n' \"$@\"\n");
+        write_tool(_home / "tools/bin/pprof",
+                   "if [ \"$1\" = --version ]; then exit 0; fi\nprintf '%s\\n' 
\"$@\"\n");
+        write_tool(_home / "bin/perf", R"(case "$1" in
+--version) exit 0 ;;
+record)
+    while [ "$#" -gt 0 ]; do
+        if [ "$1" = -o ]; then
+            shift
+            printf 'sample\n' > "$1"
+            exit
+        fi
+        shift
+    done
+    exit 1 ;;
+script) cat -- "$3" ;;
+esac
+)");
+        write_tool(_home / "FlameGraph/stackcollapse-perf.pl", "cat\n");
+        write_tool(_home / "FlameGraph/flamegraph.pl", "cat\n");
+    }
+
+    void TearDown() override {
+        EXPECT_EQ(setenv("DORIS_HOME", _old_home.c_str(), 1), 0);
+        EXPECT_EQ(setenv("PATH", _old_path.c_str(), 1), 0);
+        config::pprof_profile_dir = _old_profile_dir;
+        std::filesystem::remove_all(_root);
+    }
+
+    static void write_tool(const std::filesystem::path& path, const 
std::string& body) {
+        std::filesystem::create_directories(path.parent_path());
+        std::ofstream file(path);
+        file << "#!/bin/sh\n" << body;
+        file.close();
+        ASSERT_TRUE(file.good());
+        std::filesystem::permissions(path, std::filesystem::perms::owner_all);
+    }
+
+    std::filesystem::path _root;
+    std::filesystem::path _home;
+    std::filesystem::path _profile_dir;
+    std::string _old_home;
+    std::string _old_path;
+    std::string _old_profile_dir;
+};
+
+TEST_F(ProfilerCommandTest, JeprofKeepsPathsLiteral) {
+    const std::string profile = (_profile_dir / "jeheap_dump.heap").string();
+    EXPECT_EQ("--dot\n--\n" + (_home / "lib/doris_be").string() + "\n" + 
profile + "\n",
+              HeapProfiler::jeprof_profile_to_dot(profile));
+}
+
+TEST_F(ProfilerCommandTest, BundledJeprofKeepsLocalProfile) {
+    const auto jeprof = std::filesystem::current_path() / "tools/jeprof";
+    // Keep failure output in this fixture. The curl stub prevents remote 
requests.
+    write_tool(_home / "bin/jeprof",
+               "export JEPROF_TMPDIR=" + 
AgentUtils::shell_quote(_root.string()) + "\nexec " +
+                       AgentUtils::shell_quote(jeprof.string()) + " \"$@\"\n");
+    write_tool(_home / "bin/curl", "echo 'Unexpected remote profile' >&2\nexit 
1\n");
+    std::filesystem::create_directories(_home / "lib");
+    std::filesystem::create_symlink("/bin/true", _home / "lib/doris_be");
+    const auto profile_dir = _profile_dir / "safe@1";
+    std::filesystem::create_directories(profile_dir);
+    const std::string profile = (profile_dir / "jeheap_dump.heap").string();
+    // Two lines describe a valid profile with no samples. No process sampling 
is needed.
+    std::ofstream(profile) << "heap_v2/1\n  t*: 0: 0 [0: 0]\n";
+    EXPECT_EQ("Using local file " + (_home / "lib/doris_be").string() + 
".\nUsing local file " +
+                      profile + ".\nNo nodes to print\n",
+              HeapProfiler::jeprof_profile_to_dot(profile));
+}
+
+TEST_F(ProfilerCommandTest, BundledJeprofUnitTests) {
+    const auto jeprof = std::filesystem::current_path() / "tools/jeprof";
+    std::string output;
+    AgentUtils util;
+    ASSERT_TRUE(util.exec_cmd(AgentUtils::shell_quote(jeprof.string()) + " 
--test", &output))
+            << output;
+    EXPECT_NE(output.find("ShellEscape round trip passed\n"), 
std::string::npos);
+}
+
+TEST_F(ProfilerCommandTest, PprofKeepsFilePathLiteral) {
+    const std::string profile = (_profile_dir / "cpu_profile").string();
+    std::ofstream(profile) << "sample";
+    std::string binary;
+    ASSERT_TRUE(PprofUtils::get_self_cmdline(&binary).ok());
+    std::stringstream output;
+    const auto status = PprofUtils::get_readable_profile(profile, true, 
&output);
+    ASSERT_TRUE(status.ok()) << status;
+    EXPECT_EQ("Profile(Sample 30 seconds)\n--text\n--\n" + binary + "\n" + 
profile + "\n\n",
+              output.str());
+    EXPECT_FALSE(std::filesystem::exists(profile));
+}
+
+TEST_F(ProfilerCommandTest, PprofKeepsGeneratedPathLiteral) {
+    std::stringstream output;
+    const auto status = PprofUtils::get_readable_profile("sample", false, 
&output);
+    ASSERT_TRUE(status.ok()) << status;
+    EXPECT_NE(output.str().find(_profile_dir.string() + "/pprof_profile."), 
std::string::npos);
+    EXPECT_TRUE(std::filesystem::is_empty(_profile_dir));
+}
+
+TEST_F(ProfilerCommandTest, FlamegraphKeepsPathsLiteral) {
+    // Both response modes use the same profile path. File mode also tests 
shell redirection.
+    for (bool return_file : {false, true}) {
+        std::string result;
+        const auto status = PprofUtils::generate_flamegraph(0, (_home / 
"FlameGraph").string(),
+                                                            return_file, 
&result);
+        ASSERT_TRUE(status.ok()) << status;
+        if (return_file) {
+            EXPECT_EQ(std::filesystem::path(result).parent_path(), 
_profile_dir);
+            std::ifstream file(result);
+            std::stringstream content;
+            content << file.rdbuf();
+            EXPECT_EQ("sample\n", content.str());
+        } else {
+            EXPECT_EQ("sample\n", result);
+        }
+    }
+}
+
+} // namespace doris
diff --git a/tools/jeprof b/tools/jeprof
index 620bff916bb..756951c9eae 100755
--- a/tools/jeprof
+++ b/tools/jeprof
@@ -550,7 +550,10 @@ sub Init() {
 
   # Parse profile file/location arguments
   foreach my $farg (@ARGV) {
-    if ($farg =~ m/(.*)\@([0-9]+)(|\/.*)$/ ) {
+    # A local directory can contain @1. Do not treat its files as remote 
groups.
+    if (-f $farg) {
+      unshift(@main::pfile_args, $farg);
+    } elsif ($farg =~ m/(.*)\@([0-9]+)(|\/.*)$/ ) {
       my $machine = $1;
       my $num_machines = $2;
       my $path = $3;
@@ -5305,7 +5308,8 @@ sub ShellEscape {
   foreach my $word (@_) {
     my $escaped_word = $word;
     if ($word =~ m![^a-zA-Z0-9/.,_=-]!) {  # check for anything not in 
whitelist
-      $escaped_word =~ s/'/'\\''/;
+      # Escape each quote. For example, a'b'c contains two quotes.
+      $escaped_word =~ s/'/'\\''/g;
       $escaped_word = "'$escaped_word'";
     }
     push(@escaped_words, $escaped_word);
@@ -5672,8 +5676,20 @@ sub AddressIncUnitTest {
 }
 
 
+# Check that the shell returns a path with two quotes unchanged.
+sub ShellEscapeUnitTest {
+  my $word = q{a'b'c};
+  my $command = "printf '%s' " . ShellEscape($word);
+  my $result = `$command`;
+  if ($? != 0 || $result ne $word) {
+    print STDERR "ShellEscape round trip failed\n";
+    return 1;
+  }
+  print STDERR "ShellEscape round trip passed\n";
+  return 0;
+}
+
 # Driver for unit tests.
-# Currently just the address add/subtract/increment routines for 64-bit.
 sub RunUnitTests {
   my $error_count = 0;
 
@@ -5714,6 +5730,7 @@ sub RunUnitTests {
   $error_count += AddressAddUnitTest($unit_test_data_8, $unit_test_data_16);
   $error_count += AddressSubUnitTest($unit_test_data_8, $unit_test_data_16);
   $error_count += AddressIncUnitTest($unit_test_data_8, $unit_test_data_16);
+  $error_count += ShellEscapeUnitTest();
   if ($error_count > 0) {
     print STDERR $error_count, " errors: FAILED\n";
   } else {


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to