From e78729ff310bf4846a34fd0f60944c717920c9a3 Mon Sep 17 00:00:00 2001
From: Mihai Dinca <mdinca@suse.de>
Date: Fri, 22 Jun 2018 16:44:43 +0200
Subject: [PATCH] Porting - fix diffing binary files in file.get_diff
 (bsc#1098394)

---
 salt/modules/file.py | 138 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 113 insertions(+), 25 deletions(-)

diff --git a/salt/modules/file.py b/salt/modules/file.py
index 144c57d634..46ffd6042b 100644
--- a/salt/modules/file.py
+++ b/salt/modules/file.py
@@ -4479,43 +4479,131 @@ def check_file_meta(
     return changes
 
 
-def get_diff(
-        minionfile,
-        masterfile,
-        saltenv='base'):
+def get_diff(file1,
+             file2,
+             saltenv='base',
+             show_filenames=True,
+             show_changes=True,
+             template=False,
+             source_hash_file1=None,
+             source_hash_file2=None):
     '''
-    Return unified diff of file compared to file on master
+    Return unified diff of two files
 
-    CLI Example:
+    file1
+        The first file to feed into the diff utility
+
+        .. versionchanged:: 2018.3.0
+            Can now be either a local or remote file. In earlier releases,
+            thuis had to be a file local to the minion.
+
+    file2
+        The second file to feed into the diff utility
+
+        .. versionchanged:: 2018.3.0
+            Can now be either a local or remote file. In earlier releases, this
+            had to be a file on the salt fileserver (i.e.
+            ``salt://somefile.txt``)
+
+    show_filenames : True
+        Set to ``False`` to hide the filenames in the top two lines of the
+        diff.
+
+    show_changes : True
+        If set to ``False``, and there are differences, then instead of a diff
+        a simple message stating that show_changes is set to ``False`` will be
+        returned.
+
+    template : False
+        Set to ``True`` if two templates are being compared. This is not useful
+        except for within states, with the ``obfuscate_templates`` option set
+        to ``True``.
+
+        .. versionadded:: 2018.3.0
+
+    source_hash_file1
+        If ``file1`` is an http(s)/ftp URL and the file exists in the minion's
+        file cache, this option can be passed to keep the minion from
+        re-downloading the archive if the cached copy matches the specified
+        hash.
+
+        .. versionadded:: 2018.3.0
+
+    source_hash_file2
+        If ``file2`` is an http(s)/ftp URL and the file exists in the minion's
+        file cache, this option can be passed to keep the minion from
+        re-downloading the archive if the cached copy matches the specified
+        hash.
+
+        .. versionadded:: 2018.3.0
+
+    CLI Examples:
 
     .. code-block:: bash
 
         salt '*' file.get_diff /home/fred/.vimrc salt://users/fred/.vimrc
+        salt '*' file.get_diff /tmp/foo.txt /tmp/bar.txt
     '''
-    minionfile = os.path.expanduser(minionfile)
+    files = (file1, file2)
+    source_hashes = (source_hash_file1, source_hash_file2)
+    paths = []
+    errors = []
+
+    for filename, source_hash in zip(files, source_hashes):
+        try:
+            # Local file paths will just return the same path back when passed
+            # to cp.cache_file.
+            cached_path = __salt__['cp.cache_file'](filename,
+                                                    saltenv,
+                                                    source_hash=source_hash)
+            if cached_path is False:
+                errors.append(
+                    'File {0} not found'.format(
+                        salt.utils.stringutils.to_unicode(filename)
+                    )
+                )
+                continue
+            paths.append(cached_path)
+        except MinionError as exc:
+            errors.append(salt.utils.stringutils.to_unicode(exc.__str__()))
+            continue
 
-    ret = ''
+    if errors:
+        raise CommandExecutionError(
+            'Failed to cache one or more files',
+            info=errors
+        )
 
-    if not os.path.exists(minionfile):
-        ret = 'File {0} does not exist on the minion'.format(minionfile)
-        return ret
+    args = []
+    for idx, filename in enumerate(files):
+        try:
+            with salt.utils.files.fopen(filename, 'rb') as fp_:
+                args.append(fp_.readlines())
+        except (IOError, OSError) as exc:
+            raise CommandExecutionError(
+                'Failed to read {0}: {1}'.format(
+                    salt.utils.stringutils.to_unicode(filename),
+                    exc.strerror
+                )
+            )
 
-    sfn = __salt__['cp.cache_file'](masterfile, saltenv)
-    if sfn:
-        with salt.utils.fopen(sfn, 'r') as src:
-            slines = src.readlines()
-        with salt.utils.fopen(minionfile, 'r') as name_:
-            nlines = name_.readlines()
-        if ''.join(nlines) != ''.join(slines):
-            bdiff = _binary_replace(minionfile, sfn)
+    if args[0] != args[1]:
+        if template and __salt__['config.option']('obfuscate_templates'):
+            ret = '<Obfuscated Template>'
+        elif not show_changes:
+            ret = '<show_changes=False>'
+        else:
+            bdiff = _binary_replace(*files)
             if bdiff:
-                ret += bdiff
+                ret = bdiff
             else:
-                ret += ''.join(difflib.unified_diff(nlines, slines,
-                                                    minionfile, masterfile))
-    else:
-        ret = 'Failed to copy file from master'
-
+                if show_filenames:
+                    args.extend(files)
+                ret = ''.join(
+                    difflib.unified_diff(
+                        *salt.utils.data.decode(args)
+                    )
+                )
     return ret
 
 
-- 
2.17.1


