From 364f772cc51a849ce7ac6cb91b9c185319ca9b08 Mon Sep 17 00:00:00 2001
From: Toshio Kuratomi <toshio@fedoraproject.org>
Date: Thu, 22 Jan 2015 12:31:32 -0800
Subject: [PATCH] Fix quoting of shell parameters used in remote_checksum and
 add integration test to detect the error

Fixes #682
---
 lib/ansible/runner/shell_plugins/sh.py        |  5 ++-
 .../roles/test_unarchive/tasks/main.yml       | 33 +++++++++++++++++--
 v2/ansible/plugins/shell/sh.py                |  5 ++-
 3 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/lib/ansible/runner/shell_plugins/sh.py b/lib/ansible/runner/shell_plugins/sh.py
index 95d48e9e7d..c43a8e725c 100644
--- a/lib/ansible/runner/shell_plugins/sh.py
+++ b/lib/ansible/runner/shell_plugins/sh.py
@@ -79,7 +79,6 @@ class ShellModule(object):
         return 'echo %s' % user_home_path
 
     def checksum(self, path, python_interp):
-        path = pipes.quote(path)
         # The following test needs to be SH-compliant.  BASH-isms will
         # not work if /bin/sh points to a non-BASH shell.
         #
@@ -97,14 +96,14 @@ class ShellModule(object):
         # 0.  This logic is added to the end of the cmd at the bottom of this
         # function.
 
-        test = "rc=flag; [ -r \"%(p)s\" ] || rc=2; [ -f \"%(p)s\" ] || rc=1; [ -d \"%(p)s\" ] && rc=3; %(i)s -V 2>/dev/null || rc=4; [ x\"$rc\" != \"xflag\" ] && echo \"${rc}  %(p)s\" && exit 0" % dict(p=path, i=python_interp)
+        test = "rc=flag; [ -r \'%(p)s\' ] || rc=2; [ -f \'%(p)s\' ] || rc=1; [ -d \'%(p)s\' ] && rc=3; %(i)s -V 2>/dev/null || rc=4; [ x\"$rc\" != \"xflag\" ] && echo \"${rc}\"\'  %(p)s\' && exit 0" % dict(p=path, i=python_interp)
         csums = [
             "(%s -c 'import hashlib; print(hashlib.sha1(open(\"%s\", \"rb\").read()).hexdigest())' 2>/dev/null)" % (python_interp, path),      # Python > 2.4 (including python3)
             "(%s -c 'import sha; print(sha.sha(open(\"%s\", \"rb\").read()).hexdigest())' 2>/dev/null)" % (python_interp, path),        # Python == 2.4
         ]
 
         cmd = " || ".join(csums)
-        cmd = "%s; %s || (echo \"0  %s\")" % (test, cmd, path)
+        cmd = "%s; %s || (echo \'0  %s\')" % (test, cmd, path)
         return cmd
 
     def build_module_command(self, env_string, shebang, cmd, rm_tmp=None):
diff --git a/test/integration/roles/test_unarchive/tasks/main.yml b/test/integration/roles/test_unarchive/tasks/main.yml
index 06ca81151b..3e315a7e94 100644
--- a/test/integration/roles/test_unarchive/tasks/main.yml
+++ b/test/integration/roles/test_unarchive/tasks/main.yml
@@ -22,7 +22,7 @@
   when: ansible_pkg_mgr  ==  'yum'
 
 - name: Ensure zip is present to create test archive (apt)
-  yum: name=zip state=latest
+  apt: name=zip state=latest
   when: ansible_pkg_mgr  ==  'apt'
 
 - name: prep our file
@@ -196,5 +196,34 @@
       - "unarchive07.changed == false"
 
 - name: remove our tar.gz unarchive destination
-  file: path={{output_dir}}/test-unarchive-tar-gz state=absent
+  file: path={{ output_dir }}/test-unarchive-tar-gz state=absent
 
+- name: create a directory with quotable chars
+  file: path="{{ output_dir }}/test-quotes~root" state=directory
+
+- name: unarchive into directory with quotable chars
+  unarchive:
+    src: "{{ output_dir }}/test-unarchive.tar.gz"
+    dest: "{{ output_dir | expanduser }}/test-quotes~root"
+    copy: no
+  register: unarchive08
+
+- name: Test that unarchive succeeded
+  assert:
+    that:
+      - "unarchive08.changed == true"
+
+- name: unarchive into directory with quotable chars a second time
+  unarchive:
+    src: "{{ output_dir }}/test-unarchive.tar.gz"
+    dest: "{{ output_dir | expanduser }}/test-quotes~root"
+    copy: no
+  register: unarchive09
+
+- name: Test that unarchive did nothing
+  assert:
+    that:
+      - "unarchive09.changed == false"
+
+- name: remove quotable chars test
+  file: path="{{ output_dir }}/test-quotes~root" state=absent
diff --git a/v2/ansible/plugins/shell/sh.py b/v2/ansible/plugins/shell/sh.py
index 95d48e9e7d..c43a8e725c 100644
--- a/v2/ansible/plugins/shell/sh.py
+++ b/v2/ansible/plugins/shell/sh.py
@@ -79,7 +79,6 @@ class ShellModule(object):
         return 'echo %s' % user_home_path
 
     def checksum(self, path, python_interp):
-        path = pipes.quote(path)
         # The following test needs to be SH-compliant.  BASH-isms will
         # not work if /bin/sh points to a non-BASH shell.
         #
@@ -97,14 +96,14 @@ class ShellModule(object):
         # 0.  This logic is added to the end of the cmd at the bottom of this
         # function.
 
-        test = "rc=flag; [ -r \"%(p)s\" ] || rc=2; [ -f \"%(p)s\" ] || rc=1; [ -d \"%(p)s\" ] && rc=3; %(i)s -V 2>/dev/null || rc=4; [ x\"$rc\" != \"xflag\" ] && echo \"${rc}  %(p)s\" && exit 0" % dict(p=path, i=python_interp)
+        test = "rc=flag; [ -r \'%(p)s\' ] || rc=2; [ -f \'%(p)s\' ] || rc=1; [ -d \'%(p)s\' ] && rc=3; %(i)s -V 2>/dev/null || rc=4; [ x\"$rc\" != \"xflag\" ] && echo \"${rc}\"\'  %(p)s\' && exit 0" % dict(p=path, i=python_interp)
         csums = [
             "(%s -c 'import hashlib; print(hashlib.sha1(open(\"%s\", \"rb\").read()).hexdigest())' 2>/dev/null)" % (python_interp, path),      # Python > 2.4 (including python3)
             "(%s -c 'import sha; print(sha.sha(open(\"%s\", \"rb\").read()).hexdigest())' 2>/dev/null)" % (python_interp, path),        # Python == 2.4
         ]
 
         cmd = " || ".join(csums)
-        cmd = "%s; %s || (echo \"0  %s\")" % (test, cmd, path)
+        cmd = "%s; %s || (echo \'0  %s\')" % (test, cmd, path)
         return cmd
 
     def build_module_command(self, env_string, shebang, cmd, rm_tmp=None):