From 31f73eb12dd0888efc61c0333539d1354c7268ce Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 11 Sep 2017 11:05:20 -0400 Subject: get_filename_component: Revise PROGRAM/PROGRAM_ARGS split semantics The KWSys `SystemTools::SplitProgramFromArgs` implementation goes into an infinite loop when the value is just " " (a space). Since the "program path with unquoted spaces plus command-line arguments" operation it is trying to provide is poorly defined (string parsing should not depend on filesystem content), just stop using it. Instead consider the main two use cases the old approach tried to handle: * The value is the name or absolute path of a program with no quoting or escaping, but also no command-line arguments. In this case we can use the value as given with no parsing, and assume no arguments. * The value is a command-line string containing the program name/path plus arguments. In this case we now assume that the command line is properly quoted or escaped. Fixes: #17262 --- .../get_filename_component-fix-program-split.rst | 9 ++++ Source/cmGetFilenameComponentCommand.cxx | 25 ++++++++++- Source/cmSystemTools.cxx | 50 ++++++++++++++++++++++ Source/cmSystemTools.h | 5 +++ .../get_filename_component/KnownComponents.cmake | 11 +++++ 5 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 Help/release/dev/get_filename_component-fix-program-split.rst diff --git a/Help/release/dev/get_filename_component-fix-program-split.rst b/Help/release/dev/get_filename_component-fix-program-split.rst new file mode 100644 index 0000000..55c8719 --- /dev/null +++ b/Help/release/dev/get_filename_component-fix-program-split.rst @@ -0,0 +1,9 @@ +get_filename_component-fix-program-split +---------------------------------------- + +* The :command:`get_filename_component` ``PROGRAM`` mode semantics + have been revised to not tolerate unquoted spaces in the path + to the program while also accepting arguments. While technically + incompatible with the old behavior, it is expected that behavior + under typical use cases with properly-quoted command-lines has + not changed. diff --git a/Source/cmGetFilenameComponentCommand.cxx b/Source/cmGetFilenameComponentCommand.cxx index c23684c..1b358ab 100644 --- a/Source/cmGetFilenameComponentCommand.cxx +++ b/Source/cmGetFilenameComponentCommand.cxx @@ -60,7 +60,30 @@ bool cmGetFilenameComponentCommand::InitialPass( } } } - cmSystemTools::SplitProgramFromArgs(filename, result, programArgs); + + // First assume the path to the program was specified with no + // arguments and with no quoting or escaping for spaces. + // Only bother doing this if there is non-whitespace. + if (!cmSystemTools::TrimWhitespace(filename).empty()) { + result = cmSystemTools::FindProgram(filename); + } + + // If that failed then assume a command-line string was given + // and split the program part from the rest of the arguments. + if (result.empty()) { + std::string program; + if (cmSystemTools::SplitProgramFromArgs(filename, program, + programArgs)) { + if (cmSystemTools::FileExists(program)) { + result = program; + } else { + result = cmSystemTools::FindProgram(program); + } + } + if (result.empty()) { + programArgs.clear(); + } + } } else if (args[2] == "EXT") { result = cmSystemTools::GetFilenameExtension(filename); } else if (args[2] == "NAME_WE") { diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 5c63d98..2635d6d 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -603,6 +603,56 @@ std::vector cmSystemTools::ParseArguments(const char* command) return args; } +bool cmSystemTools::SplitProgramFromArgs(std::string const& command, + std::string& program, + std::string& args) +{ + const char* c = command.c_str(); + + // Skip leading whitespace. + while (isspace(static_cast(*c))) { + ++c; + } + + // Parse one command-line element up to an unquoted space. + bool in_escape = false; + bool in_double = false; + bool in_single = false; + for (; *c; ++c) { + if (in_single) { + if (*c == '\'') { + in_single = false; + } else { + program += *c; + } + } else if (in_escape) { + in_escape = false; + program += *c; + } else if (*c == '\\') { + in_escape = true; + } else if (in_double) { + if (*c == '"') { + in_double = false; + } else { + program += *c; + } + } else if (*c == '"') { + in_double = true; + } else if (*c == '\'') { + in_single = true; + } else if (isspace(static_cast(*c))) { + break; + } else { + program += *c; + } + } + + // The remainder of the command line holds unparsed arguments. + args = c; + + return !in_single && !in_escape && !in_double; +} + size_t cmSystemTools::CalculateCommandLineLengthLimit() { size_t sz = diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h index 9bec361..e7082e6 100644 --- a/Source/cmSystemTools.h +++ b/Source/cmSystemTools.h @@ -255,6 +255,11 @@ public: static void ParseUnixCommandLine(const char* command, std::vector& args); + /** Split a command-line string into the parsed command and the unparsed + arguments. Returns false on unfinished quoting or escaping. */ + static bool SplitProgramFromArgs(std::string const& command, + std::string& program, std::string& args); + /** * Handle response file in an argument list and return a new argument list * **/ diff --git a/Tests/RunCMake/get_filename_component/KnownComponents.cmake b/Tests/RunCMake/get_filename_component/KnownComponents.cmake index 7dfb55d..ac77ac3 100644 --- a/Tests/RunCMake/get_filename_component/KnownComponents.cmake +++ b/Tests/RunCMake/get_filename_component/KnownComponents.cmake @@ -80,6 +80,17 @@ get_filename_component(test_program_name "/ arg1 arg2" PROGRAM check("PROGRAM with args output: name" "${test_program_name}" "/") check("PROGRAM with args output: args" "${test_program_args}" " arg1 arg2") +get_filename_component(test_program_name " " PROGRAM) +check("PROGRAM with just a space" "${test_program_name}" "") + +get_filename_component(test_program_name "${CMAKE_CURRENT_LIST_FILE}" PROGRAM) +check("PROGRAM specified explicitly without quoting" "${test_program_name}" "${CMAKE_CURRENT_LIST_FILE}") + +get_filename_component(test_program_name "\"${CMAKE_CURRENT_LIST_FILE}\" arg1 arg2" PROGRAM + PROGRAM_ARGS test_program_args) +check("PROGRAM specified explicitly with arguments: name" "${test_program_name}" "${CMAKE_CURRENT_LIST_FILE}") +check("PROGRAM specified explicitly with arguments: args" "${test_program_args}" " arg1 arg2") + list(APPEND non_cache_vars test_program_name) list(APPEND non_cache_vars test_program_args) -- cgit v0.12