FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Make diff patch parsing more reliable · ali-security/gitpython-4635@e77128e · GitHub

  1. ali-security
  2. gitpython-4635

Repository navigation

Commit e77128e

Browse files
nvie
committed
Make diff patch parsing more reliable
The a_path and b_path cannot reliably be read from the first diff line as it's ambiguous. From the git-diff manpage: > The a/ and b/ filenames are the same unless rename/copy is involved. > Especially, **even for a creation or a deletion**, /dev/null is not > used in place of the a/ or b/ filenames. This patch changes the a_path and b_path detection to read it from the more reliable locations further down the diff headers. Two use cases are fixed by this: - As the man page snippet above states, for new/deleted files the a or b path will now be properly None. - File names with spaces in it are now properly parsed. Working on this patch, I realized the --- and +++ lines really belong to the diff header, not the diff contents. This means that when parsing the patch format, the --- and +++ will now be swallowed, and not end up anymore as part of the diff contents. The diff contents now always start with an @@ line. This may be a breaking change for some users that rely on this behaviour. However, those users could now access that information more reliably via the normal Diff properties a_path and b_path now.
1 parent 0d7a40f commit e77128e

4 files changed

Lines changed: 38 additions & 14 deletions

File tree

‎git/diff.py‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -198,16 +198,18 @@ class Diff(object):
198198
# precompiled regex
199199
re_header = re.compile(r"""
200200
^diff[ ]--git
201-
[ ](?:a/)?(?P<a_path>.+?)[ ](?:b/)?(?P<b_path>.+?)\n
202-
(?:^similarity[ ]index[ ](?P<similarity_index>\d+)%\n
203-
^rename[ ]from[ ](?P<rename_from>\S+)\n
204-
^rename[ ]to[ ](?P<rename_to>\S+)(?:\n|$))?
201+
[ ](?:a/)?(?P<a_path_fallback>.+?)[ ](?:b/)?(?P<b_path_fallback>.+?)\n
202+
(?:^similarity[ ]index[ ]\d+%\n
203+
^rename[ ]from[ ](?P<rename_from>.*)\n
204+
^rename[ ]to[ ](?P<rename_to>.*)(?:\n|$))?
205205
(?:^old[ ]mode[ ](?P<old_mode>\d+)\n
206206
^new[ ]mode[ ](?P<new_mode>\d+)(?:\n|$))?
207207
(?:^new[ ]file[ ]mode[ ](?P<new_file_mode>.+)(?:\n|$))?
208208
(?:^deleted[ ]file[ ]mode[ ](?P<deleted_file_mode>.+)(?:\n|$))?
209209
(?:^index[ ](?P<a_blob_id>[0-9A-Fa-f]+)
210210
\.\.(?P<b_blob_id>[0-9A-Fa-f]+)[ ]?(?P<b_mode>.+)?(?:\n|$))?
211+
(?:^---[ ](?:a/)?(?P<a_path>.*)(?:\n|$))?
212+
(?:^\+\+\+[ ](?:b/)?(?P<b_path>.*)(?:\n|$))?
211213
""".encode('ascii'), re.VERBOSE | re.MULTILINE)
212214
# can be used for comparisons
213215
NULL_HEX_SHA = "0" * 40
@@ -231,15 +233,14 @@ def __init__(self, repo, a_path, b_path, a_blob_id, b_blob_id, a_mode,
231233
if self.b_mode:
232234
self.b_mode = mode_str_to_int(self.b_mode)
233235

234-
if a_blob_id is None:
236+
if a_blob_id is None or a_blob_id == self.NULL_HEX_SHA:
235237
self.a_blob = None
236238
else:
237-
assert self.a_mode is not None
238239
self.a_blob = Blob(repo, hex_to_bin(a_blob_id), mode=self.a_mode, path=a_path)
239-
if b_blob_id is None:
240+
241+
if b_blob_id is None or b_blob_id == self.NULL_HEX_SHA:
240242
self.b_blob = None
241243
else:
242-
assert self.b_mode is not None
243244
self.b_blob = Blob(repo, hex_to_bin(b_blob_id), mode=self.b_mode, path=b_path)
244245

245246
self.new_file = new_file
@@ -329,11 +330,22 @@ def _index_from_patch_format(cls, repo, stream):
329330
index = DiffIndex()
330331
previous_header = None
331332
for header in cls.re_header.finditer(text):
332-
a_path, b_path, similarity_index, rename_from, rename_to, \
333+
a_path_fallback, b_path_fallback, \
334+
rename_from, rename_to, \
333335
old_mode, new_mode, new_file_mode, deleted_file_mode, \
334-
a_blob_id, b_blob_id, b_mode = header.groups()
336+
a_blob_id, b_blob_id, b_mode, \
337+
a_path, b_path = header.groups()
335338
new_file, deleted_file = bool(new_file_mode), bool(deleted_file_mode)
336339

340+
a_path = a_path or rename_from or a_path_fallback
341+
b_path = b_path or rename_to or b_path_fallback
342+
343+
if a_path == b'/dev/null':
344+
a_path = None
345+
346+
if b_path == b'/dev/null':
347+
b_path = None
348+
337349
# Our only means to find the actual text is to see what has not been matched by our regex,
338350
# and then retro-actively assin it to our index
339351
if previous_header is not None:

‎git/test/fixtures/diff_file_with_spaces‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
diff --git a/file with spaces b/file with spaces
2+
new file mode 100644
3+
index 0000000000000000000000000000000000000000..75c620d7b0d3b0100415421a97f553c979d75174
4+
--- /dev/null
5+
+++ b/file with spaces
6+
@@ -0,0 +1 @@
7+
+ohai

‎git/test/fixtures/diff_initial‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
--- /dev/null
2-
+++ b/CHANGES
31
@@ -0,0 +1,7 @@
42
+=======
53
+CHANGES

‎git/test/test_diff.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def test_list_from_string_new_mode(self):
7676
self._assert_diff_format(diffs)
7777

7878
assert_equal(1, len(diffs))
79-
assert_equal(10, len(diffs[0].diff.splitlines()))
79+
assert_equal(8, len(diffs[0].diff.splitlines()))
8080

8181
def test_diff_with_rename(self):
8282
output = StringProcessAdapter(fixture('diff_rename'))
@@ -140,7 +140,8 @@ def test_diff_initial_commit(self):
140140

141141
# ...and with creating a patch
142142
diff_index = initial_commit.diff(NULL_TREE, create_patch=True)
143-
assert diff_index[0].b_path == 'CHANGES'
143+
assert diff_index[0].a_path is None, repr(diff_index[0].a_path)
144+
assert diff_index[0].b_path == 'CHANGES', repr(diff_index[0].b_path)
144145
assert diff_index[0].new_file
145146
assert diff_index[0].diff == fixture('diff_initial')
146147

@@ -156,6 +157,12 @@ def test_diff_patch_format(self):
156157
Diff._index_from_patch_format(self.rorepo, diff_proc.stdout)
157158
# END for each fixture
158159

160+
def test_diff_with_spaces(self):
161+
data = StringProcessAdapter(fixture('diff_file_with_spaces'))
162+
diff_index = Diff._index_from_patch_format(self.rorepo, data.stdout)
163+
assert diff_index[0].a_path is None, repr(diff_index[0].a_path)
164+
assert diff_index[0].b_path == u'file with spaces', repr(diff_index[0].b_path)
165+
159166
def test_diff_interface(self):
160167
# test a few variations of the main diff routine
161168
assertion_map = dict()

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL