Merge ~cjwatson/turnip:delete-repo-with-non-utf8-files into turnip:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 8b2a05a4696de94c595e3405b84724a28c07b690
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/turnip:delete-repo-with-non-utf8-files
Merge into: turnip:master
Diff against target: 37 lines (+15/-0)
2 files modified
turnip/api/store.py (+4/-0)
turnip/api/tests/test_api.py (+11/-0)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Tom Wardill (community) Approve
Review via email: mp+359051@code.launchpad.net

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.

To post a comment you must log in.
Revision history for this message
Tom Wardill (twom) :
review: Approve
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote :
Revision history for this message
Colin Watson (cjwatson) wrote :

Self-approving to work around Jenkins's idea of our voting criteria.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/turnip/api/store.py b/turnip/api/store.py
2index 8968a4a..38d6294 100644
3--- a/turnip/api/store.py
4+++ b/turnip/api/store.py
5@@ -246,6 +246,10 @@ def set_default_branch(repo_path, target):
6
7 def delete_repo(repo_path):
8 """Permanently delete a git repository from repo store."""
9+ # XXX cjwatson 2018-11-20: Python 2's shutil.rmtree crashes if given a
10+ # Unicode path to a directory that contains non-UTF-8 files.
11+ if not isinstance(repo_path, bytes):
12+ repo_path = repo_path.encode('UTF-8')
13 shutil.rmtree(repo_path)
14
15
16diff --git a/turnip/api/tests/test_api.py b/turnip/api/tests/test_api.py
17index adbf3be..4ca1aac 100644
18--- a/turnip/api/tests/test_api.py
19+++ b/turnip/api/tests/test_api.py
20@@ -179,6 +179,17 @@ class ApiTestCase(TestCase):
21 self.assertEqual(200, resp.status_code)
22 self.assertFalse(os.path.exists(self.repo_store))
23
24+ def test_repo_delete_non_utf8_file_names(self):
25+ """Deleting a repo works even if it contains non-UTF-8 file names."""
26+ self.app.post_json('/repo', {'repo_path': self.repo_path})
27+ factory = RepoFactory(self.repo_store)
28+ factory.add_commit('foo', 'foobar.txt')
29+ subprocess.check_call(
30+ ['git', '-C', self.repo_store, 'branch', b'\x80'])
31+ resp = self.app.delete('/repo/{}'.format(self.repo_path))
32+ self.assertEqual(200, resp.status_code)
33+ self.assertFalse(os.path.exists(self.repo_store))
34+
35 def test_repo_get_refs(self):
36 """Ensure expected ref objects are returned and shas match."""
37 ref = self.commit.get('ref')

Subscribers

People subscribed via source and target branches