Rev 623: Ensure that per-file commit messages and global commit messages get sanitized. in http://bazaar.launchpad.net/%7Ejameinel/bzr-gtk/sanitize_commit_messages

[John Arbash Meinel]

At http://bazaar.launchpad.net/%7Ejameinel/bzr-gtk/sanitize_commit_messages

------------------------------------------------------------
revno: 623
revision-id: john at arbash-meinel.com-20081113054835-8s7xth55j7kcexl3
parent: jelmer at samba.org-20081107161737-q29iraxmifhkzn2p
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: sanitize_commit_messages
timestamp: Wed 2008-11-12 23:48:35 -0600
message:
  Ensure that per-file commit messages and global commit messages get sanitized.
-------------- next part --------------
=== modified file 'commit.py'
--- a/commit.py	2008-09-29 16:10:51 +0000
+++ b/commit.py	2008-11-13 05:48:35 +0000
@@ -97,6 +97,13 @@
     return pm
 
 
+_newline_variants_re = re.compile(r'\r\n?')
+def _sanitize_and_decode_message(utf8_message):
+    """Turn a utf-8 message into a sanitized Unicode message."""
+    fixed_newline = _newline_variants_re.sub('\n', utf8_message)
+    return fixed_newline.decode('utf-8')
+
+
 class CommitDialog(gtk.Dialog):
     """Implementation of Commit."""
 
@@ -631,10 +638,12 @@
             if self._commit_all_changes or record[2]:# [2] checkbox
                 file_id = record[0] # [0] file_id
                 path = record[1]    # [1] real path
-                file_message = record[5] # [5] commit message
+                # [5] commit message
+                file_message = _sanitize_and_decode_message(record[5])
                 files.append(path.decode('UTF-8'))
                 if self._enable_per_file_commits and file_message:
                     # All of this needs to be utf-8 information
+                    file_message = file_message.encode('UTF-8')
                     file_info.append({'path':path, 'file_id':file_id,
                                      'message':file_message})
         file_info.sort(key=lambda x:(x['path'], x['file_id']))
@@ -712,7 +721,8 @@
     def _get_global_commit_message(self):
         buf = self._global_message_text_view.get_buffer()
         start, end = buf.get_bounds()
-        return buf.get_text(start, end).decode('utf-8')
+        text = buf.get_text(start, end)
+        return _sanitize_and_decode_message(text)
 
     def _set_global_commit_message(self, message):
         """Just a helper for the test suite."""

=== modified file 'tests/test_commit.py'
--- a/tests/test_commit.py	2008-09-29 16:10:51 +0000
+++ b/tests/test_commit.py	2008-11-13 05:48:35 +0000
@@ -644,6 +644,28 @@
                             'message':'message\nfor b_dir\n'},
                           ]), dlg._get_specific_files())
 
+    def test_specific_files_sanitizes_messages(self):
+        tree = self.make_branch_and_tree('tree')
+        tree.branch.get_config().set_user_option('per_file_commits', 'true')
+        self.build_tree(['tree/a_file', 'tree/b_dir/'])
+        tree.add(['a_file', 'b_dir'], ['1a-id', '0b-id'])
+
+        dlg = commit.CommitDialog(tree)
+        dlg._commit_selected_radio.set_active(True)
+        self.assertEqual((['a_file', 'b_dir'], []), dlg._get_specific_files())
+
+        dlg._treeview_files.set_cursor((1,))
+        dlg._set_file_commit_message('Test\r\nmessage\rfor a_file\n')
+        dlg._treeview_files.set_cursor((2,))
+        dlg._set_file_commit_message('message\r\nfor\nb_dir\r')
+
+        self.assertEqual((['a_file', 'b_dir'],
+                          [{'path':'a_file', 'file_id':'1a-id',
+                            'message':'Test\nmessage\nfor a_file\n'},
+                           {'path':'b_dir', 'file_id':'0b-id',
+                            'message':'message\nfor\nb_dir\n'},
+                          ]), dlg._get_specific_files())
+
 
 class TestCommitDialog_Commit(tests.TestCaseWithTransport):
     """Tests on the actual 'commit' button being pushed."""
@@ -687,6 +709,21 @@
         self.assertEqual(last_rev, dlg.committed_revision_id)
         self.assertEqual(rev_id1, tree.branch.last_revision())
 
+    def test_commit_global_sanitizes_message(self):
+        tree = self.make_branch_and_tree('tree')
+        self.build_tree(['tree/a'])
+        tree.add(['a'], ['a-id'])
+        rev_id1 = tree.commit('one')
+
+        self.build_tree(['tree/b'])
+        tree.add(['b'], ['b-id'])
+        dlg = commit.CommitDialog(tree)
+        # With the check box set, it should only effect the local branch
+        dlg._set_global_commit_message('Commit\r\nmessage\rfoo\n')
+        dlg._do_commit()
+        rev = tree.branch.repository.get_revision(tree.last_revision())
+        self.assertEqual('Commit\nmessage\nfoo\n', rev.message)
+
     def test_bound_commit_both(self):
         tree = self.make_branch_and_tree('tree')
         self.build_tree(['tree/a'])
@@ -1030,3 +1067,22 @@
                           {'path':u'\u03a9', 'file_id':'omega-id',
                            'message':u'\u03a9 is the end of all things.\n'},
                          ], file_info_decoded)
+
+
+class TestSanitizeMessage(tests.TestCase):
+
+    def assertSanitize(self, expected, original):
+        self.assertEqual(expected,
+                         commit._sanitize_and_decode_message(original))
+
+    def test_untouched(self):
+        self.assertSanitize('foo\nbar\nbaz\n', 'foo\nbar\nbaz\n')
+
+    def test_converts_cr_to_lf(self):
+        self.assertSanitize('foo\nbar\nbaz\n', 'foo\rbar\rbaz\r')
+
+    def test_converts_crlf_to_lf(self):
+        self.assertSanitize('foo\nbar\nbaz\n', 'foo\r\nbar\r\nbaz\r\n')
+
+    def test_converts_mixed_to_lf(self):
+        self.assertSanitize('foo\nbar\nbaz\n', 'foo\r\nbar\rbaz\n')