Merge lp:~jelmer/brz/smart-add-illegal-char into lp:brz

Proposed by Jelmer Vernooij on 2018-11-11
Status: Needs review
Proposed branch: lp:~jelmer/brz/smart-add-illegal-char
Merge into: lp:brz
Diff against target: 108 lines (+55/-2)
5 files modified
breezy/bzr/inventorytree.py (+9/-0)
breezy/bzr/workingtree.py (+9/-1)
breezy/git/workingtree.py (+7/-1)
breezy/tests/per_workingtree/test_smart_add.py (+25/-0)
doc/en/release-notes/brz-3.0.txt (+5/-0)
To merge this branch: bzr merge lp:~jelmer/brz/smart-add-illegal-char
Reviewer Review Type Date Requested Status
Martin Packman 2018-11-11 Needs Fixing on 2018-11-16
Review via email: mp+358608@code.launchpad.net

Description of the change

Raise better exception when encountering filenames that are invalid in the current encoding.

To post a comment you must log in.
7145. By Jelmer Vernooij on 2018-11-13

Fix python3 compatibility."

Martin Packman (gz) wrote :

This should use the osutils.DirReader abstraction that uses different logic for filesystems that are really bytes (*nix) and filesystems that are really unicode (windows).

review: Needs Fixing

Unmerged revisions

7145. By Jelmer Vernooij on 2018-11-13

Fix python3 compatibility."

7144. By Jelmer Vernooij on 2018-11-11

Raise better exception when encountering invalid characters in the current fs encoding.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/bzr/inventorytree.py'
2--- breezy/bzr/inventorytree.py 2018-09-14 14:13:30 +0000
3+++ breezy/bzr/inventorytree.py 2018-11-13 02:37:00 +0000
4@@ -671,7 +671,16 @@
5 if this_ie.kind != 'directory':
6 this_ie = self._convert_to_directory(this_ie, inv_path)
7
8+ abspath = abspath.encode(osutils._fs_enc)
9+
10 for subf in sorted(os.listdir(abspath)):
11+ try:
12+ subf = subf.decode(osutils._fs_enc)
13+ except UnicodeDecodeError:
14+ relpath = (directory.encode(osutils._fs_enc) +
15+ b'/' + subf)
16+ raise errors.BadFilenameEncoding(relpath,
17+ osutils._fs_enc)
18 inv_f, _ = osutils.normalized_filename(subf)
19 # here we could use TreeDirectory rather than
20 # string concatenation.
21
22=== modified file 'breezy/bzr/workingtree.py'
23--- breezy/bzr/workingtree.py 2018-09-22 01:03:30 +0000
24+++ breezy/bzr/workingtree.py 2018-11-13 02:37:00 +0000
25@@ -1104,7 +1104,15 @@
26
27 # But do this child first if recursing down
28 if recursive:
29- new_children = sorted(os.listdir(fap))
30+ new_children = []
31+ for subf in os.listdir(fap.encode(osutils._fs_enc)):
32+ try:
33+ new_children.append(subf.decode(osutils._fs_enc))
34+ except UnicodeDecodeError:
35+ relpath = fp.lstrip('/').encode(osutils._fs_enc) + b'/' + subf
36+ raise errors.BadFilenameEncoding(relpath,
37+ osutils._fs_enc)
38+ new_children.sort()
39 new_children = collections.deque(new_children)
40 stack.append((f_ie.file_id, fp, fap, new_children))
41 # Break out of inner loop,
42
43=== modified file 'breezy/git/workingtree.py'
44--- breezy/git/workingtree.py 2018-09-30 00:33:53 +0000
45+++ breezy/git/workingtree.py 2018-11-13 02:37:00 +0000
46@@ -482,7 +482,13 @@
47 trace.warning('skipping nested tree %r', abs_user_dir)
48 continue
49
50- for name in os.listdir(abs_user_dir):
51+ for name in os.listdir(abs_user_dir.encode(osutils._fs_enc)):
52+ try:
53+ name = name.decode(osutils._fs_enc)
54+ except UnicodeDecodeError:
55+ relpath = user_dir.encode(osutils._fs_enc) + b'/' + name
56+ raise errors.BadFilenameEncoding(relpath,
57+ osutils._fs_enc)
58 subp = os.path.join(user_dir, name)
59 if self.is_control_filename(subp) or self.mapping.is_special_file(subp):
60 continue
61
62=== modified file 'breezy/tests/per_workingtree/test_smart_add.py'
63--- breezy/tests/per_workingtree/test_smart_add.py 2018-07-23 22:49:40 +0000
64+++ breezy/tests/per_workingtree/test_smart_add.py 2018-11-13 02:37:00 +0000
65@@ -377,3 +377,28 @@
66 # just ignore files that don't fit the normalization
67 # rules, rather than exploding
68 self.assertRaises(errors.InvalidNormalization, self.wt.smart_add, [])
69+
70+
71+class TestIllegalPaths(per_workingtree.TestCaseWithWorkingTree):
72+
73+ def test_bad_fs_path(self):
74+ if osutils.normalizes_filenames():
75+ # You *can't* create an illegal filename on OSX.
76+ raise tests.TestNotApplicable('OSX normalizes filenames')
77+ self.requireFeature(features.UTF8Filesystem)
78+ # We require a UTF8 filesystem, because otherwise we would need to get
79+ # tricky to figure out how to create an illegal filename.
80+ # \xb5 is an illegal path because it should be \xc2\xb5 for UTF-8
81+ tree = self.make_branch_and_tree('tree')
82+ self.build_tree(['tree/subdir/', 'tree/subdir/somefile'])
83+ tree.add(['subdir', 'subdir/somefile'])
84+
85+ with open(b'tree/subdir/m\xb5', 'wb') as f:
86+ f.write(b'trivial\n')
87+
88+ with tree.lock_tree_write():
89+ e = self.assertListRaises(errors.BadFilenameEncoding,
90+ tree.smart_add, ['tree'])
91+ # We should display the relative path
92+ self.assertEqual(b'subdir/m\xb5', e.filename)
93+ self.assertEqual(osutils._fs_enc, e.fs_encoding)
94
95=== modified file 'doc/en/release-notes/brz-3.0.txt'
96--- doc/en/release-notes/brz-3.0.txt 2018-09-18 02:08:46 +0000
97+++ doc/en/release-notes/brz-3.0.txt 2018-11-13 02:37:00 +0000
98@@ -171,6 +171,11 @@
99
100 * Support '0' markers in fastimport plugin. (Jelmer Vernooń≥, #1744615)
101
102+* Raise a clearer exception when encountering filenames not in
103+ the current encoding when running 'brz add'.
104+ (Jelmer Vernooń≥, #715547)
105+
106+
107 Documentation
108 *************
109

Subscribers

People subscribed via source and target branches