Skip to content

Fix bug in diff parser output #454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 30, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/source/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Changelog
2.0.4 - Fixes
=============

* Fix: non-ASCII paths are now properly decoded and returned in
``.diff()`` output
* Fix: `RemoteProgress` will now strip the ', ' prefix or suffix from messages.
* API: Remote.[fetch|push|pull](...) methods now allow the ``progress`` argument to
be a callable. This saves you from creating a custom type with usually just one
Expand Down
17 changes: 15 additions & 2 deletions git/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,23 @@
PY3
)


__all__ = ('Diffable', 'DiffIndex', 'Diff', 'NULL_TREE')

# Special object to compare against the empty tree in diffs
NULL_TREE = object()

_octal_byte_re = re.compile(b'\\\\([0-9]{3})')


def _octal_repl(matchobj):
value = matchobj.group(1)
value = int(value, 8)
if PY3:
value = bytes(bytearray((value,)))
else:
value = chr(value)
return value


def decode_path(path, has_ab_prefix=True):
if path == b'/dev/null':
Expand All @@ -32,6 +43,8 @@ def decode_path(path, has_ab_prefix=True):
.replace(b'\\"', b'"')
.replace(b'\\\\', b'\\'))

path = _octal_byte_re.sub(_octal_repl, path)

if has_ab_prefix:
assert path.startswith(b'a/') or path.startswith(b'b/')
path = path[2:]
Expand Down Expand Up @@ -337,7 +350,7 @@ def renamed(self):
:note: This property is deprecated, please use ``renamed_file`` instead.
"""
return self.renamed_file

@property
def renamed_file(self):
""":returns: True if the blob of our diff has been renamed
Expand Down
7 changes: 7 additions & 0 deletions git/test/fixtures/diff_patch_unsafe_paths
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ index 0000000000000000000000000000000000000000..eaf5f7510320b6a327fb308379de2f94
+++ "b/path/¯\\_(ツ)_|¯"
@@ -0,0 +1 @@
+dummy content
diff --git "a/path/\360\237\222\251.txt" "b/path/\360\237\222\251.txt"
new file mode 100644
index 0000000000000000000000000000000000000000..eaf5f7510320b6a327fb308379de2f94d8859a54
--- /dev/null
+++ "b/path/\360\237\222\251.txt"
@@ -0,0 +1 @@
+dummy content
diff --git a/a/with spaces b/b/with some spaces
similarity index 100%
rename from a/with spaces
Expand Down
13 changes: 7 additions & 6 deletions git/test/test_diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,16 +161,17 @@ def test_diff_unsafe_paths(self):
self.assertEqual(res[6].b_path, u'path/with spaces')
self.assertEqual(res[7].b_path, u'path/with-question-mark?')
self.assertEqual(res[8].b_path, u'path/¯\\_(ツ)_|¯')
self.assertEqual(res[9].b_path, u'path/💩.txt')

# The "Moves"
# NOTE: The path prefixes a/ and b/ here are legit! We're actually
# verifying that it's not "a/a/" that shows up, see the fixture data.
self.assertEqual(res[9].a_path, u'a/with spaces') # NOTE: path a/ here legit!
self.assertEqual(res[9].b_path, u'b/with some spaces') # NOTE: path b/ here legit!
self.assertEqual(res[10].a_path, u'a/ending in a space ')
self.assertEqual(res[10].b_path, u'b/ending with space ')
self.assertEqual(res[11].a_path, u'a/"with-quotes"')
self.assertEqual(res[11].b_path, u'b/"with even more quotes"')
self.assertEqual(res[10].a_path, u'a/with spaces') # NOTE: path a/ here legit!
self.assertEqual(res[10].b_path, u'b/with some spaces') # NOTE: path b/ here legit!
self.assertEqual(res[11].a_path, u'a/ending in a space ')
self.assertEqual(res[11].b_path, u'b/ending with space ')
self.assertEqual(res[12].a_path, u'a/"with-quotes"')
self.assertEqual(res[12].b_path, u'b/"with even more quotes"')

def test_diff_patch_format(self):
# test all of the 'old' format diffs for completness - it should at least
Expand Down