From 5c2c29e71f944f5bdffab2d6a48b6eaec54f8f8d Mon Sep 17 00:00:00 2001
From: Chris Church <chris@ninemoreminutes.com>
Date: Wed, 26 Feb 2014 22:23:14 -0500
Subject: [PATCH 1/2] Pass svn arguments as a list of strings instead of using
 string substition to ensure all parameters are escaped properly.

---
 library/source_control/subversion | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/library/source_control/subversion b/library/source_control/subversion
index 0bb2560553..497052af00 100644
--- a/library/source_control/subversion
+++ b/library/source_control/subversion
@@ -94,47 +94,46 @@ class Subversion(object):
 
     def _exec(self, args):
         bits = [
-            'LANG=C',
             self.svn_path,
             '--non-interactive',
             '--trust-server-cert',
             '--no-auth-cache',
         ]
         if self.username:
-            bits.append("--username '%s'" % self.username)
+            bits.extend(["--username", self.username])
         if self.password:
-            bits.append("--password '%s'" % self.password)
-        bits.append(args)
-        rc, out, err = self.module.run_command(' '.join(bits), check_rc=True)
+            bits.extend(["--password", self.password])
+        bits.extend(args)
+        rc, out, err = self.module.run_command(bits, check_rc=True)
         return out.splitlines()
 
     def checkout(self):
         '''Creates new svn working directory if it does not already exist.'''
-        self._exec("checkout -r %s '%s' '%s'" % (self.revision, self.repo, self.dest))
+        self._exec(["checkout", "-r", self.revision, self.repo, self.dest])
 
     def switch(self):
         '''Change working directory's repo.'''
         # switch to ensure we are pointing at correct repo.
-        self._exec("switch '%s' '%s'" % (self.repo, self.dest))
+        self._exec(["switch", self.repo, self.dest])
 
     def update(self):
         '''Update existing svn working directory.'''
-        self._exec("update -r %s '%s'" % (self.revision, self.dest))
+        self._exec(["update", "-r", self.revision, self.dest])
 
     def revert(self):
         '''Revert svn working directory.'''
-        self._exec("revert -R '%s'" % self.dest)
+        self._exec(["revert", "-R", self.dest])
 
     def get_revision(self):
         '''Revision and URL of subversion working directory.'''
-        text = '\n'.join(self._exec("info '%s'" % self.dest))
+        text = '\n'.join(self._exec(["info", self.dest]))
         rev = re.search(r'^Revision:.*$', text, re.MULTILINE).group(0)
         url = re.search(r'^URL:.*$', text, re.MULTILINE).group(0)
         return rev, url
 
     def has_local_mods(self):
         '''True if revisioned files have been added or modified. Unrevisioned files are ignored.'''
-        lines = self._exec("status '%s'" % self.dest)
+        lines = self._exec(["status", self.dest])
         # Match only revisioned files, i.e. ignore status '?'.
         regex = re.compile(r'^[^?]')
         # Has local mods if more than 0 modifed revisioned files.
@@ -142,7 +141,7 @@ class Subversion(object):
 
     def needs_update(self):
         curr, url = self.get_revision()
-        out2 = '\n'.join(self._exec("info -r HEAD '%s'" % self.dest))
+        out2 = '\n'.join(self._exec(["info", "-r", "HEAD", self.dest]))
         head = re.search(r'^Revision:.*$', out2, re.MULTILINE).group(0)
         rev1 = int(curr.split(':')[1].strip())
         rev2 = int(head.split(':')[1].strip())
@@ -176,6 +175,7 @@ def main():
     password = module.params['password']
     svn_path = module.params['executable'] or module.get_bin_path('svn', True)
 
+    os.environ['LANG'] = 'C'
     svn = Subversion(module, dest, repo, revision, username, password, svn_path)
 
     if not os.path.exists(dest):

From adeea2c3f35a6d3710ad1345e47948c972b4e8e2 Mon Sep 17 00:00:00 2001
From: Chris Church <chris@ninemoreminutes.com>
Date: Wed, 26 Feb 2014 22:44:03 -0500
Subject: [PATCH 2/2] Added integration test for subversion parameters that
 need escaping.

---
 test/integration/roles/test_subversion/tasks/main.yml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/integration/roles/test_subversion/tasks/main.yml b/test/integration/roles/test_subversion/tasks/main.yml
index 12ef8bbf40..22503de35c 100644
--- a/test/integration/roles/test_subversion/tasks/main.yml
+++ b/test/integration/roles/test_subversion/tasks/main.yml
@@ -84,6 +84,12 @@
       - "trunk.stat.isdir"
       - "branches.stat.isdir"
 
+- name: checkout with quotes in username
+  subversion: repo={{ repo }} dest={{ checkout_dir }} username="quoteme'''"
+  register: subverted3
+
+- debug: var=subverted3
+
 # FIXME: this needs to be fixed in the code see GitHub 6079
 
 #- name: verify on a reclone things are marked unchanged