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

[Jelmer Vernooij]

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

[Martin Packman (gz)]

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

[The Breezy Bot (the-breezy-bot)]

Voting criteria not met
https://ci.breezy-vcs.org/job/brz/job/brz-land/753/

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.

=== modified file 'breezy/bzr/inventorytree.py'
--- breezy/bzr/inventorytree.py	2018-09-14 14:13:30 +0000
+++ breezy/bzr/inventorytree.py	2018-11-13 02:37:00 +0000
@@ -671,7 +671,16 @@
                 if this_ie.kind != 'directory':
                     this_ie = self._convert_to_directory(this_ie, inv_path)
 
+                abspath = abspath.encode(osutils._fs_enc)
+
                 for subf in sorted(os.listdir(abspath)):
+                    try:
+                        subf = subf.decode(osutils._fs_enc)
+                    except UnicodeDecodeError:
+                        relpath = (directory.encode(osutils._fs_enc) +
+                                   b'/' + subf)
+                        raise errors.BadFilenameEncoding(relpath,
+                                                         osutils._fs_enc)
                     inv_f, _ = osutils.normalized_filename(subf)
                     # here we could use TreeDirectory rather than
                     # string concatenation.
 
=== modified file 'breezy/bzr/workingtree.py'
--- breezy/bzr/workingtree.py	2018-09-22 01:03:30 +0000
+++ breezy/bzr/workingtree.py	2018-11-13 02:37:00 +0000
@@ -1104,7 +1104,15 @@
 
                     # But do this child first if recursing down
                     if recursive:
-                        new_children = sorted(os.listdir(fap))
+                        new_children = []
+                        for subf in os.listdir(fap.encode(osutils._fs_enc)):
+                            try:
+                                new_children.append(subf.decode(osutils._fs_enc))
+                            except UnicodeDecodeError:
+                                relpath = fp.lstrip('/').encode(osutils._fs_enc) + b'/' + subf
+                                raise errors.BadFilenameEncoding(relpath,
+                                                                 osutils._fs_enc)
+                        new_children.sort()
                         new_children = collections.deque(new_children)
                         stack.append((f_ie.file_id, fp, fap, new_children))
                         # Break out of inner loop,
 
=== modified file 'breezy/git/workingtree.py'
--- breezy/git/workingtree.py	2018-09-30 00:33:53 +0000
+++ breezy/git/workingtree.py	2018-11-13 02:37:00 +0000
@@ -482,7 +482,13 @@
                     trace.warning('skipping nested tree %r', abs_user_dir)
                     continue
 
-                for name in os.listdir(abs_user_dir):
+                for name in os.listdir(abs_user_dir.encode(osutils._fs_enc)):
+                    try:
+                        name = name.decode(osutils._fs_enc)
+                    except UnicodeDecodeError:
+                        relpath = user_dir.encode(osutils._fs_enc) + b'/' + name
+                        raise errors.BadFilenameEncoding(relpath,
+                                                         osutils._fs_enc)
                     subp = os.path.join(user_dir, name)
                     if self.is_control_filename(subp) or self.mapping.is_special_file(subp):
                         continue
 
=== modified file 'breezy/tests/per_workingtree/test_smart_add.py'
--- breezy/tests/per_workingtree/test_smart_add.py	2018-07-23 22:49:40 +0000
+++ breezy/tests/per_workingtree/test_smart_add.py	2018-11-13 02:37:00 +0000
@@ -377,3 +377,28 @@
         #       just ignore files that don't fit the normalization
         #       rules, rather than exploding
         self.assertRaises(errors.InvalidNormalization, self.wt.smart_add, [])
+
+
+class TestIllegalPaths(per_workingtree.TestCaseWithWorkingTree):
+
+    def test_bad_fs_path(self):
+        if osutils.normalizes_filenames():
+            # You *can't* create an illegal filename on OSX.
+            raise tests.TestNotApplicable('OSX normalizes filenames')
+        self.requireFeature(features.UTF8Filesystem)
+        # We require a UTF8 filesystem, because otherwise we would need to get
+        # tricky to figure out how to create an illegal filename.
+        # \xb5 is an illegal path because it should be \xc2\xb5 for UTF-8
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree(['tree/subdir/', 'tree/subdir/somefile'])
+        tree.add(['subdir', 'subdir/somefile'])
+
+        with open(b'tree/subdir/m\xb5', 'wb') as f:
+            f.write(b'trivial\n')
+
+        with tree.lock_tree_write():
+            e = self.assertListRaises(errors.BadFilenameEncoding,
+                                      tree.smart_add, ['tree'])
+        # We should display the relative path
+        self.assertEqual(b'subdir/m\xb5', e.filename)
+        self.assertEqual(osutils._fs_enc, e.fs_encoding)
 
=== modified file 'doc/en/release-notes/brz-3.0.txt'
--- doc/en/release-notes/brz-3.0.txt	2018-09-18 02:08:46 +0000
+++ doc/en/release-notes/brz-3.0.txt	2018-11-13 02:37:00 +0000
@@ -171,6 +171,11 @@
 
 * Support '0' markers in fastimport plugin. (Jelmer Vernooij, #1744615)
 
+* Raise a clearer exception when encountering filenames not in
+  the current encoding when running 'brz add'.
+  (Jelmer Vernooij, #715547)
+
+
 Documentation
 *************