launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/turnip:delete-repo-with-non-utf8-files into turnip:master.

Commit message:
Fix deletion of repositories with non-UTF-8 files

Python 2's shutil.rmtree crashes if you give it a Unicode path to a
directory that contains non-UTF-8 files.  Make sure we always give it
bytes.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/turnip/+git/turnip/+merge/359051
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/turnip:delete-repo-with-non-utf8-files into turnip:master.

diff --git a/turnip/api/store.py b/turnip/api/store.py
index 8968a4a..38d6294 100644
--- a/turnip/api/store.py
+++ b/turnip/api/store.py
@@ -246,6 +246,10 @@ def set_default_branch(repo_path, target):
 
 def delete_repo(repo_path):
     """Permanently delete a git repository from repo store."""
+    # XXX cjwatson 2018-11-20: Python 2's shutil.rmtree crashes if given a
+    # Unicode path to a directory that contains non-UTF-8 files.
+    if not isinstance(repo_path, bytes):
+        repo_path = repo_path.encode('UTF-8')
     shutil.rmtree(repo_path)
 
 
diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
index adbf3be..4ca1aac 100644
--- a/turnip/api/tests/test_api.py
+++ b/turnip/api/tests/test_api.py
@@ -179,6 +179,17 @@ class ApiTestCase(TestCase):
         self.assertEqual(200, resp.status_code)
         self.assertFalse(os.path.exists(self.repo_store))
 
+    def test_repo_delete_non_utf8_file_names(self):
+        """Deleting a repo works even if it contains non-UTF-8 file names."""
+        self.app.post_json('/repo', {'repo_path': self.repo_path})
+        factory = RepoFactory(self.repo_store)
+        factory.add_commit('foo', 'foobar.txt')
+        subprocess.check_call(
+            ['git', '-C', self.repo_store, 'branch', b'\x80'])
+        resp = self.app.delete('/repo/{}'.format(self.repo_path))
+        self.assertEqual(200, resp.status_code)
+        self.assertFalse(os.path.exists(self.repo_store))
+
     def test_repo_get_refs(self):
         """Ensure expected ref objects are returned and shas match."""
         ref = self.commit.get('ref')