Blob Blame History Raw
From b96bdab03f06408e162b1733b20e8ba9fbf8e012 Mon Sep 17 00:00:00 2001
From: "Bryn M. Reeves" <bmr@redhat.com>
Date: Mon, 2 Jul 2018 12:01:04 +0100
Subject: [PATCH] [archive] fix add_string()/do_*_sub() regression

A change in the handling of add_string() operations in the archive
class causes the Plugin string substitution methods to fail (since
the archive was enforcing a check that the path did not already
exist - for substitutions this is always the case).

Maintain the check for content that is being copied into the
archive anew, but make the add_string() method override this and
disable the existence checks.

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>
---
 sos/archive.py         | 14 ++++++++++----
 tests/archive_tests.py | 12 ++----------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/sos/archive.py b/sos/archive.py
index d53baf41..e153c09a 100644
--- a/sos/archive.py
+++ b/sos/archive.py
@@ -158,7 +158,7 @@ class FileCacheArchive(Archive):
             name = name.lstrip(os.sep)
         return (os.path.join(self._archive_root, name))
 
-    def _check_path(self, src, path_type, dest=None):
+    def _check_path(self, src, path_type, dest=None, force=False):
         """Check a new destination path in the archive.
 
             Since it is possible for multiple plugins to collect the same
@@ -185,6 +185,7 @@ class FileCacheArchive(Archive):
             :param src: the source path to be copied to the archive
             :param path_type: the type of object to be copied
             :param dest: an optional destination path
+            :param force: force file creation even if the path exists
             :returns: An absolute destination path if the path should be
                       copied now or `None` otherwise
         """
@@ -208,6 +209,9 @@ class FileCacheArchive(Archive):
                 stat.ISSOCK(mode)
             ])
 
+        if force:
+            return dest
+
         # Check destination path presence and type
         if os.path.exists(dest):
             # Use lstat: we care about the current object, not the referent.
@@ -274,9 +278,11 @@ class FileCacheArchive(Archive):
         with self._path_lock:
             src = dest
 
-            dest = self._check_path(dest, P_FILE)
-            if not dest:
-                return
+            # add_string() is a special case: it must always take precedence
+            # over any exixting content in the archive, since it is used by
+            # the Plugin postprocessing hooks to perform regex substitution
+            # on file content.
+            dest = self._check_path(dest, P_FILE, force=True)
 
             f = codecs.open(dest, 'w', encoding='utf-8')
             if isinstance(content, bytes):