fix(RemoteProgress): improve message sanitization

Byron · Byron · commit 6ef273914de9 · 2016-05-25T09:57:54.000+02:00
Don't allow `, ` prefixes or suffixes in messages. Fixes gitpython-developers#438
diff --git a/doc/source/changes.rst b/doc/source/changes.rst
@@ -7,11 +7,11 @@ Changelog
 
 * Fix: bug in ``git-blame --incremental`` output parser that broken when
   commit messages contained ``\r`` characters
-* Fix: progress handler exceptions are not caught anymore, which would
-  usually just hide bugs previously.
-* Fix: The ``Git.execute`` method will now redirect ``stdout`` to
-  ``devnull`` if ``with_stdout`` is false, which is the intended behaviour
-  based on the parameter's documentation.
+* Fix: progress handler exceptions are not caught anymore, which would usually just hide bugs
+  previously.
+* Fix: The `Git.execute` method will now redirect `stdout` to `devnull` if `with_stdout` is false, 
+  which is the intended behaviour based on the parameter's documentation.
+* Fix: `RemoteProgress` will now strip the ', ' prefix or suffix from messages.
 
 2.0.2 - Fixes
 =============
diff --git a/git/test/test_remote.py b/git/test/test_remote.py
@@ -62,6 +62,14 @@ def update(self, op_code, cur_count, max_count=None, message=''):
         # check each stage only comes once
         op_id = op_code & self.OP_MASK
         assert op_id in (self.COUNTING, self.COMPRESSING, self.WRITING)
+        
+        if op_code & self.WRITING > 0:
+            if op_code & self.BEGIN > 0:
+                assert not message, 'should not have message when remote begins writing'
+            elif op_code & self.END > 0:
+                assert message
+                assert not message.startswith(', '), "Sanitize progress messages: '%s'" % message
+                assert not message.endswith(', '), "Sanitize progress messages: '%s'" % message
 
         self._stages_per_op.setdefault(op_id, 0)
         self._stages_per_op[op_id] = self._stages_per_op[op_id] | (op_code & self.STAGE_MASK)
diff --git a/git/util.py b/git/util.py
@@ -171,12 +171,16 @@ class RemoteProgress(object):
     STAGE_MASK = BEGIN | END
     OP_MASK = ~STAGE_MASK
 
+    DONE_TOKEN = 'done.'
+    TOKEN_SEPARATOR = ', '
+
     __slots__ = ("_cur_line", "_seen_ops")
-    re_op_absolute = re.compile("(remote: )?([\w\s]+):\s+()(\d+)()(.*)")
-    re_op_relative = re.compile("(remote: )?([\w\s]+):\s+(\d+)% \((\d+)/(\d+)\)(.*)")
+    re_op_absolute = re.compile(r"(remote: )?([\w\s]+):\s+()(\d+)()(.*)")
+    re_op_relative = re.compile(r"(remote: )?([\w\s]+):\s+(\d+)% \((\d+)/(\d+)\)(.*)")
 
     def __init__(self):
         self._seen_ops = list()
+        self._cur_line = None
 
     def _parse_progress_line(self, line):
         """Parse progress information from the given line as retrieved by git-push
@@ -257,11 +261,11 @@ def _parse_progress_line(self, line):
             # END message handling
 
             message = message.strip()
-            done_token = ', done.'
-            if message.endswith(done_token):
+            if message.endswith(self.DONE_TOKEN):
                 op_code |= self.END
-                message = message[:-len(done_token)]
+                message = message[:-len(self.DONE_TOKEN)]
             # END end message handling
+            message = message.strip(self.TOKEN_SEPARATOR)
 
             self.update(op_code,
                         cur_count and float(cur_count),
