From: Joel Rosdahl <joel@rosdahl.net>
Date: Thu, 19 Nov 2020 20:31:41 +0000 (+0100)
Subject: Fix bug in depfile when output filename includes special characters
X-Git-Tag: v4.1~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f06b2beeeea442e5533ae9936be8d2e038bc3bdd;p=thirdparty%2Fccache.git

Fix bug in depfile when output filename includes special characters

The rewriting of the dependency file content introduced in 2ab31632 just
replaces the object file part of the dependency file with the raw
filename. This is incorrect if the filename includes special characters
such as space or dollar.

Fix this by escaping the filename properly. Note that newlines are not
treated specially since Make cannot handle them anyway.

See also the similar bug fix for the depend mode (but for parsing
instead of escaping in that case) in 1d2b5bf4.
---

diff --git a/src/Depfile.cpp b/src/Depfile.cpp
index 90da9c9cc..ed491f4dc 100644
--- a/src/Depfile.cpp
+++ b/src/Depfile.cpp
@@ -31,6 +31,29 @@ is_blank(const std::string& s)
 
 namespace Depfile {
 
+std::string
+escape_filename(nonstd::string_view filename)
+{
+  std::string result;
+  result.reserve(filename.size());
+  for (const char c : filename) {
+    switch (c) {
+    case '\\':
+    case '#':
+    case ':':
+    case ' ':
+    case '\t':
+      result.push_back('\\');
+      break;
+    case '$':
+      result.push_back('$');
+      break;
+    }
+    result.push_back(c);
+  }
+  return result;
+}
+
 nonstd::optional<std::string>
 rewrite_paths(const Context& ctx, const std::string& file_content)
 {
diff --git a/src/Depfile.hpp b/src/Depfile.hpp
index a6a67e500..770f78964 100644
--- a/src/Depfile.hpp
+++ b/src/Depfile.hpp
@@ -29,6 +29,7 @@ class Hash;
 
 namespace Depfile {
 
+std::string escape_filename(nonstd::string_view filename);
 nonstd::optional<std::string> rewrite_paths(const Context& ctx,
                                             const std::string& file_content);
 void make_paths_relative_in_output_dep(const Context& ctx);
diff --git a/src/ResultRetriever.cpp b/src/ResultRetriever.cpp
index 04deaaf58..77e044dd0 100644
--- a/src/ResultRetriever.cpp
+++ b/src/ResultRetriever.cpp
@@ -19,6 +19,7 @@
 #include "ResultRetriever.hpp"
 
 #include "Context.hpp"
+#include "Depfile.hpp"
 #include "Logging.hpp"
 
 using Result::FileType;
@@ -163,9 +164,10 @@ ResultRetriever::write_dependency_file()
     if (m_rewrite_dependency_target) {
       size_t colon_pos = m_dest_data.find(':');
       if (colon_pos != std::string::npos) {
-        Util::write_fd(*m_dest_fd,
-                       m_ctx.args_info.output_obj.data(),
-                       m_ctx.args_info.output_obj.length());
+        const auto escaped_output_obj =
+          Depfile::escape_filename(m_ctx.args_info.output_obj);
+        Util::write_fd(
+          *m_dest_fd, escaped_output_obj.data(), escaped_output_obj.length());
         start_pos = colon_pos;
       }
     }
diff --git a/test/suites/depend.bash b/test/suites/depend.bash
index ca48bee45..0d9fcc5be 100644
--- a/test/suites/depend.bash
+++ b/test/suites/depend.bash
@@ -294,6 +294,23 @@ EOF
     expect_stat 'files in cache' 5
 
     # -------------------------------------------------------------------------
+    TEST "Source file with special characters"
 
-    # TODO: Add more test cases (see direct.bash for inspiration)
+    touch 'file with$special#characters.c'
+    $REAL_COMPILER -MMD -c 'file with$special#characters.c'
+    mv 'file with$special#characters.d' reference.d
+
+    CCACHE_DEPEND=1 $CCACHE_COMPILE -MMD -c 'file with$special#characters.c'
+    expect_stat 'cache hit (direct)' 0
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 1
+    expect_equal_content 'file with$special#characters.d' reference.d
+
+    rm 'file with$special#characters.d'
+
+    CCACHE_DEPEND=1 $CCACHE_COMPILE -MMD -c 'file with$special#characters.c'
+    expect_stat 'cache hit (direct)' 1
+    expect_stat 'cache hit (preprocessed)' 0
+    expect_stat 'cache miss' 1
+    expect_equal_content 'file with$special#characters.d' reference.d
 }
diff --git a/unittest/test_Depfile.cpp b/unittest/test_Depfile.cpp
index e0bbf142f..b098800b4 100644
--- a/unittest/test_Depfile.cpp
+++ b/unittest/test_Depfile.cpp
@@ -27,6 +27,17 @@ using TestUtil::TestContext;
 
 TEST_SUITE_BEGIN("Depfile");
 
+TEST_CASE("Depfile::escape_filename")
+{
+  CHECK(Depfile::escape_filename("") == "");
+  CHECK(Depfile::escape_filename("foo") == "foo");
+  CHECK(Depfile::escape_filename("foo\\bar") == "foo\\\\bar");
+  CHECK(Depfile::escape_filename("foo#bar") == "foo\\#bar");
+  CHECK(Depfile::escape_filename("foo bar") == "foo\\ bar");
+  CHECK(Depfile::escape_filename("foo\tbar") == "foo\\\tbar");
+  CHECK(Depfile::escape_filename("foo$bar") == "foo$$bar");
+}
+
 TEST_CASE("Depfile::rewrite_paths")
 {
   Context ctx;