kallithea Changeset - 55d2b08d9c44

Changeset - 55d2b08d9c44

Parent rev.

Child rev.

[Not reviewed]

stable

0 4 0

Branko Majic (branko) - 8 years ago 2018-02-09 18:12:19
branko@majic.rs

vcs: sanitize diff context values (Issue #306)

- Updated Git repository implementation to ensure context falls within
0 to 2**31-1 range (inclusive) when fetching a diff.
- Added tests for Git repositories for checking passed-in negative and
overflowing contexts (for the --unified option).
- Updated Mercurial repository implementation to ensure context is not
negative when fetching a diff.
- Added tests for Mercurial repositories for checking passed-in
negative context (for the --unified option).

4 files changed with 91 insertions and 2 deletions:

kallithea/lib/vcs/backends/git/repository.py

kallithea/lib/vcs/backends/hg/repository.py

kallithea/tests/vcs/test_git.py

kallithea/tests/vcs/test_hg.py

0 comments (0 inline, 0 general)

kallithea/lib/vcs/backends/git/repository.py

➞

Show inline comments

@@ @@ -542,98 +542,120 @@ class GitRepository(BaseRepository): @@
         if end_date:
             cmd += ['--until', end_date.strftime('%m/%d/%y %H:%M:%S')]
         if branch_name:
             cmd.append(branch_name)
         else:
             cmd.append(settings.GIT_REV_FILTER)
         revs = self.run_git_command(cmd)[0].splitlines()
         start_pos = 0
         end_pos = len(revs)
         if start:
             _start = self._get_revision(start)
             try:
                 start_pos = revs.index(_start)
             except ValueError:
                 pass
         if end is not None:
             _end = self._get_revision(end)
             try:
                 end_pos = revs.index(_end)
             except ValueError:
                 pass
         if None not in [start, end] and start_pos > end_pos:
             raise RepositoryError('start cannot be after end')
         if end_pos is not None:
             end_pos += 1
         revs = revs[start_pos:end_pos]
         if reverse:
             revs = reversed(revs)
         return CollectionGenerator(self, revs)
     def get_diff(self, rev1, rev2, path=None, ignore_whitespace=False,
                  context=3):
         """
         Returns (git like) *diff*, as plain text. Shows changes introduced by
         ``rev2`` since ``rev1``.
         :param rev1: Entry point from which diff is shown. Can be
           ``self.EMPTY_CHANGESET`` - in this case, patch showing all
           the changes since empty state of the repository until ``rev2``
         :param rev2: Until which revision changes should be shown.
         :param ignore_whitespace: If set to ``True``, would not show whitespace
           changes. Defaults to ``False``.
         :param context: How many lines before/after changed lines should be
           shown. Defaults to ``3``.
           shown. Defaults to ``3``. Due to limitations in Git, if
           value passed-in is greater than ``2**31-1``
           (``2147483647``), it will be set to ``2147483647``
           instead. If negative value is passed-in, it will be set to
           ``0`` instead.
         """
         # Git internally uses a signed long int for storing context
         # size (number of lines to show before and after the
         # differences). This can result in integer overflow, so we
         # ensure the requested context is smaller by one than the
         # number that would cause the overflow. It is highly unlikely
         # that a single file will contain that many lines, so this
         # kind of change should not cause any realistic consequences.
         overflowed_long_int = 2**31
         if context >= overflowed_long_int:
             context = overflowed_long_int-1
         # Negative context values make no sense, and will result in
         # errors. Ensure this does not happen.
         if context < 0:
             context = 0
         flags = ['-U%s' % context, '--full-index', '--binary', '-p', '-M', '--abbrev=40']
         if ignore_whitespace:
             flags.append('-w')
         if hasattr(rev1, 'raw_id'):
             rev1 = getattr(rev1, 'raw_id')
         if hasattr(rev2, 'raw_id'):
             rev2 = getattr(rev2, 'raw_id')
         if rev1 == self.EMPTY_CHANGESET:
             rev2 = self.get_changeset(rev2).raw_id
             cmd = ['show'] + flags + [rev2]
         else:
             rev1 = self.get_changeset(rev1).raw_id
             rev2 = self.get_changeset(rev2).raw_id
             cmd = ['diff'] + flags + [rev1, rev2]
         if path:
             cmd += ['--', path]
         stdout, stderr = self.run_git_command(cmd)
         # TODO: don't ignore stderr
         # If we used 'show' command, strip first few lines (until actual diff
         # starts)
         if rev1 == self.EMPTY_CHANGESET:
             parts = stdout.split('\ndiff ', 1)
             if len(parts) > 1:
                 stdout = 'diff ' + parts[1]
         return stdout
     @LazyProperty
     def in_memory_changeset(self):
         """
         Returns ``GitInMemoryChangeset`` object for this repository.
         """
         return GitInMemoryChangeset(self)
     def clone(self, url, update_after_clone=True, bare=False):
         """
         Tries to clone changes from external location.
         :param update_after_clone: If set to ``False``, git won't checkout
           working directory
         :param bare: If set to ``True``, repository would be cloned into
           *bare* git repository (no working directory at all).
         """
         url = self._get_url(url)

kallithea/lib/vcs/backends/hg/repository.py

➞

Show inline comments

@@ @@ -199,98 +199,105 @@ class MercurialRepository(BaseRepository @@
         """
         if name not in self.tags:
             raise TagDoesNotExistError("Tag %s does not exist" % name)
         if message is None:
             message = "Removed tag %s" % name
         if date is None:
             date = datetime.datetime.now().ctime()
         local = False
         try:
             self._repo.tag(name, nullid, message, local, user, date)
             self.tags = self._get_tags()
         except Abort as e:
             raise RepositoryError(e.message)
     @LazyProperty
     def bookmarks(self):
         """
         Gets bookmarks for this repository
         """
         return self._get_bookmarks()
     def _get_bookmarks(self):
         if self._empty:
             return {}
         sortkey = lambda ctx: ctx[0]  # sort by name
         _bookmarks = [(safe_unicode(n), hex(h),) for n, h in
                  self._repo._bookmarks.items()]
         return OrderedDict(sorted(_bookmarks, key=sortkey, reverse=True))
     def _get_all_revisions(self):
         return [self._repo[x].hex() for x in self._repo.filtered('visible').changelog.revs()]
     def get_diff(self, rev1, rev2, path='', ignore_whitespace=False,
                   context=3):
         """
         Returns (git like) *diff*, as plain text. Shows changes introduced by
         ``rev2`` since ``rev1``.
         :param rev1: Entry point from which diff is shown. Can be
           ``self.EMPTY_CHANGESET`` - in this case, patch showing all
           the changes since empty state of the repository until ``rev2``
         :param rev2: Until which revision changes should be shown.
         :param ignore_whitespace: If set to ``True``, would not show whitespace
           changes. Defaults to ``False``.
         :param context: How many lines before/after changed lines should be
           shown. Defaults to ``3``.
           shown. Defaults to ``3``. If negative value is passed-in, it will be
           set to ``0`` instead.
         """
         # Negative context values make no sense, and will result in
         # errors. Ensure this does not happen.
         if context < 0:
             context = 0
         if hasattr(rev1, 'raw_id'):
             rev1 = getattr(rev1, 'raw_id')
         if hasattr(rev2, 'raw_id'):
             rev2 = getattr(rev2, 'raw_id')
         # Check if given revisions are present at repository (may raise
         # ChangesetDoesNotExistError)
         if rev1 != self.EMPTY_CHANGESET:
             self.get_changeset(rev1)
         self.get_changeset(rev2)
         if path:
             file_filter = match(self.path, '', [path])
         else:
             file_filter = None
         return ''.join(patch.diff(self._repo, rev1, rev2, match=file_filter,
                           opts=diffopts(git=True,
                                         showfunc=True,
                                         ignorews=ignore_whitespace,
                                         context=context)))
     @classmethod
     def _check_url(cls, url, repoui=None):
         """
         Function will check given url and try to verify if it's a valid
         link. Sometimes it may happened that mercurial will issue basic
         auth request that can cause whole API to hang when used from python
         or other external calls.
         On failures it'll raise urllib2.HTTPError, exception is also thrown
         when the return code is non 200
         """
         # check first if it's not an local url
         if os.path.isdir(url) or url.startswith('file:'):
             return True
         if url.startswith('ssh:'):
             # in case of invalid uri or authentication issues, sshpeer will
             # throw an exception.
             sshpeer(repoui or ui.ui(), url).lookup('tip')
             return True
         url_prefix = None
         if '+' in url[:url.find('://')]:
             url_prefix, url = url.split('+', 1)
         handlers = []

kallithea/tests/vcs/test_git.py

➞

Show inline comments

@@ @@ -682,96 +682,136 @@ class GitSpecificWithRepoTest(_BackendTe @@
                 'author': 'Joe Doe <joe.doe@example.com>',
                 'date': datetime.datetime(2010, 1, 1, 22),
                 'added': [
                     FileNode('foo2', content='foo2'),
                 ],
             },
+        ]
     def test_paths_slow_traversing(self):
         cs = self.repo.get_changeset()
         self.assertEqual(cs.get_node('foobar').get_node('static').get_node('js')
             .get_node('admin').get_node('base.js').content, 'base')
     def test_paths_fast_traversing(self):
         cs = self.repo.get_changeset()
         self.assertEqual(cs.get_node('foobar/static/js/admin/base.js').content,
             'base')
     def test_workdir_get_branch(self):
         self.repo.run_git_command(['checkout', '-b', 'production'])
         # Regression test: one of following would fail if we don't check
         # .git/HEAD file
         self.repo.run_git_command(['checkout', 'production'])
         self.assertEqual(self.repo.workdir.get_branch(), 'production')
         self.repo.run_git_command(['checkout', 'master'])
         self.assertEqual(self.repo.workdir.get_branch(), 'master')
     def test_get_diff_runs_git_command_with_hashes(self):
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(0, 1)
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U3', '--full-index', '--binary', '-p', '-M', '--abbrev=40',
              self.repo._get_revision(0), self.repo._get_revision(1)])
     def test_get_diff_runs_git_command_with_str_hashes(self):
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(self.repo.EMPTY_CHANGESET, 1)
         self.repo.run_git_command.assert_called_once_with(
             ['show', '-U3', '--full-index', '--binary', '-p', '-M', '--abbrev=40',
              self.repo._get_revision(1)])
     def test_get_diff_runs_git_command_with_path_if_its_given(self):
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(0, 1, 'foo')
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U3', '--full-index', '--binary', '-p', '-M', '--abbrev=40',
              self.repo._get_revision(0), self.repo._get_revision(1), '--', 'foo'])
     def test_get_diff_does_not_sanitize_valid_context(self):
         almost_overflowed_long_int = 2**31-1
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(0, 1, 'foo', context=almost_overflowed_long_int)
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U' + str(almost_overflowed_long_int), '--full-index', '--binary', '-p', '-M', '--abbrev=40',
              self.repo._get_revision(0), self.repo._get_revision(1), '--', 'foo'])
     def test_get_diff_sanitizes_overflowing_context(self):
         overflowed_long_int = 2**31
         sanitized_overflowed_long_int = overflowed_long_int-1
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(0, 1, 'foo', context=overflowed_long_int)
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U' + str(sanitized_overflowed_long_int), '--full-index', '--binary', '-p', '-M', '--abbrev=40',
              self.repo._get_revision(0), self.repo._get_revision(1), '--', 'foo'])
     def test_get_diff_does_not_sanitize_zero_context(self):
         zero_context = 0
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(0, 1, 'foo', context=zero_context)
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U' + str(zero_context), '--full-index', '--binary', '-p', '-M', '--abbrev=40',
              self.repo._get_revision(0), self.repo._get_revision(1), '--', 'foo'])
     def test_get_diff_sanitizes_negative_context(self):
         negative_context = -10
         self.repo.run_git_command = mock.Mock(return_value=['', ''])
         self.repo.get_diff(0, 1, 'foo', context=negative_context)
         self.repo.run_git_command.assert_called_once_with(
             ['diff', '-U0', '--full-index', '--binary', '-p', '-M', '--abbrev=40',
              self.repo._get_revision(0), self.repo._get_revision(1), '--', 'foo'])
 class GitRegressionTest(_BackendTestMixin, unittest.TestCase):
     backend_alias = 'git'
     @classmethod
     def _get_commits(cls):
         return [
+            {
                 'message': 'Initial',
                 'author': 'Joe Doe <joe.doe@example.com>',
                 'date': datetime.datetime(2010, 1, 1, 20),
                 'added': [
                     FileNode('bot/__init__.py', content='base'),
                     FileNode('bot/templates/404.html', content='base'),
                     FileNode('bot/templates/500.html', content='base'),
                 ],
             },
+            {
                 'message': 'Second',
                 'author': 'Joe Doe <joe.doe@example.com>',
                 'date': datetime.datetime(2010, 1, 1, 22),
                 'added': [
                     FileNode('bot/build/migrations/1.py', content='foo2'),
                     FileNode('bot/build/migrations/2.py', content='foo2'),
                     FileNode('bot/build/static/templates/f.html', content='foo2'),
                     FileNode('bot/build/static/templates/f1.html', content='foo2'),
                     FileNode('bot/build/templates/err.html', content='foo2'),
                     FileNode('bot/build/templates/err2.html', content='foo2'),
                 ],
             },
+        ]
     def test_similar_paths(self):
         cs = self.repo.get_changeset()
         paths = lambda *n:[x.path for x in n]
         self.assertEqual(paths(*cs.get_nodes('bot')), ['bot/build', 'bot/templates', 'bot/__init__.py'])
         self.assertEqual(paths(*cs.get_nodes('bot/build')), ['bot/build/migrations', 'bot/build/static', 'bot/build/templates'])
         self.assertEqual(paths(*cs.get_nodes('bot/build/static')), ['bot/build/static/templates'])
         # this get_nodes below causes troubles !
         self.assertEqual(paths(*cs.get_nodes('bot/build/static/templates')), ['bot/build/static/templates/f.html', 'bot/build/static/templates/f1.html'])
         self.assertEqual(paths(*cs.get_nodes('bot/build/templates')), ['bot/build/templates/err.html', 'bot/build/templates/err2.html'])
         self.assertEqual(paths(*cs.get_nodes('bot/templates/')), ['bot/templates/404.html', 'bot/templates/500.html'])
 class GitHooksTest(unittest.TestCase):
     """
     Tests related to hook functionality of Git repositories.
     """

kallithea/tests/vcs/test_hg.py

➞

Show inline comments

 import os
 import mock
 from kallithea.lib.vcs.backends.hg import MercurialRepository, MercurialChangeset
 from kallithea.lib.vcs.exceptions import RepositoryError, VCSError, NodeDoesNotExistError
 from kallithea.lib.vcs.nodes import NodeKind, NodeState
 from kallithea.tests.vcs.conf import TEST_HG_REPO, TEST_HG_REPO_CLONE, \
     TEST_HG_REPO_PULL
 from kallithea.lib.vcs.utils.compat import unittest
 class MercurialRepositoryTest(unittest.TestCase):
     def __check_for_existing_repo(self):
         if os.path.exists(TEST_HG_REPO_CLONE):
             self.fail('Cannot test mercurial clone repo as location %s already '
                       'exists. You should manually remove it first.'
                       % TEST_HG_REPO_CLONE)
     def setUp(self):
         self.repo = MercurialRepository(TEST_HG_REPO)
     def test_wrong_repo_path(self):
         wrong_repo_path = '/tmp/errorrepo'
         self.assertRaises(RepositoryError, MercurialRepository, wrong_repo_path)
     def test_unicode_path_repo(self):
         self.assertRaises(VCSError,lambda:MercurialRepository(u'iShouldFail'))
     def test_repo_clone(self):
         self.__check_for_existing_repo()
         repo = MercurialRepository(TEST_HG_REPO)
         repo_clone = MercurialRepository(TEST_HG_REPO_CLONE,
             src_url=TEST_HG_REPO, update_after_clone=True)
         self.assertEqual(len(repo.revisions), len(repo_clone.revisions))
         # Checking hashes of changesets should be enough
         for changeset in repo.get_changesets():
             raw_id = changeset.raw_id
             self.assertEqual(raw_id, repo_clone.get_changeset(raw_id).raw_id)
     def test_repo_clone_with_update(self):
         repo = MercurialRepository(TEST_HG_REPO)
         repo_clone = MercurialRepository(TEST_HG_REPO_CLONE + '_w_update',
             src_url=TEST_HG_REPO, update_after_clone=True)
         self.assertEqual(len(repo.revisions), len(repo_clone.revisions))
         #check if current workdir was updated
         self.assertEqual(os.path.isfile(os.path.join(TEST_HG_REPO_CLONE \
                                                     + '_w_update',
                                                     'MANIFEST.in')), True,)
@@ @@ -186,96 +189,113 @@ class MercurialRepositoryTest(unittest.T @@
         node = init_chset.get_node('vcs/')
         self.assertTrue(hasattr(node, 'kind'))
         self.assertEqual(node.kind, NodeKind.DIR)
         node = init_chset.get_node('vcs')
         self.assertTrue(hasattr(node, 'kind'))
         self.assertEqual(node.kind, NodeKind.DIR)
         node = init_chset.get_node('vcs/__init__.py')
         self.assertTrue(hasattr(node, 'kind'))
         self.assertEqual(node.kind, NodeKind.FILE)
     def test_not_existing_changeset(self):
         #rawid
         self.assertRaises(RepositoryError, self.repo.get_changeset,
             'abcd' * 10)
         #shortid
         self.assertRaises(RepositoryError, self.repo.get_changeset,
             'erro' * 4)
         #numeric
         self.assertRaises(RepositoryError, self.repo.get_changeset,
             self.repo.count() + 1)
         # Small chance we ever get to this one
         revision = pow(2, 30)
         self.assertRaises(RepositoryError, self.repo.get_changeset, revision)
     def test_changeset10(self):
         chset10 = self.repo.get_changeset(10)
         README = """===
 VCS
 ===
 Various Version Control System management abstraction layer for Python.
 Introduction
 ------------
 TODO: To be written...
 """
         node = chset10.get_node('README.rst')
         self.assertEqual(node.kind, NodeKind.FILE)
         self.assertEqual(node.content, README)
     @mock.patch('kallithea.lib.vcs.backends.hg.repository.diffopts')
     def test_get_diff_does_not_sanitize_zero_context(self, mock_diffopts):
         zero_context = 0
         self.repo.get_diff(0, 1, 'foo', context=zero_context)
         mock_diffopts.assert_called_once_with(git=True, showfunc=True, ignorews=False, context=zero_context)
     @mock.patch('kallithea.lib.vcs.backends.hg.repository.diffopts')
     def test_get_diff_sanitizes_negative_context(self, mock_diffopts):
         negative_context = -10
         zero_context = 0
         self.repo.get_diff(0, 1, 'foo', context=negative_context)
         mock_diffopts.assert_called_once_with(git=True, showfunc=True, ignorews=False, context=zero_context)
 class MercurialChangesetTest(unittest.TestCase):
     def setUp(self):
         self.repo = MercurialRepository(TEST_HG_REPO)
     def _test_equality(self, changeset):
         revision = changeset.revision
         self.assertEqual(changeset, self.repo.get_changeset(revision))
     def test_equality(self):
         self.setUp()
         revs = [0, 10, 20]
         changesets = [self.repo.get_changeset(rev) for rev in revs]
         for changeset in changesets:
             self._test_equality(changeset)
     def test_default_changeset(self):
         tip = self.repo.get_changeset('tip')
         self.assertEqual(tip, self.repo.get_changeset())
         self.assertEqual(tip, self.repo.get_changeset(revision=None))
         self.assertEqual(tip, list(self.repo[-1:])[0])
     def test_root_node(self):
         tip = self.repo.get_changeset('tip')
         self.assertTrue(tip.root is tip.get_node(''))
     def test_lazy_fetch(self):
         """
         Test if changeset's nodes expands and are cached as we walk through
         the revision. This test is somewhat hard to write as order of tests
         is a key here. Written by running command after command in a shell.
         """
         self.setUp()
         chset = self.repo.get_changeset(45)
         self.assertTrue(len(chset.nodes) == 0)
         root = chset.root
         self.assertTrue(len(chset.nodes) == 1)
         self.assertTrue(len(root.nodes) == 8)
         # accessing root.nodes updates chset.nodes
         self.assertTrue(len(chset.nodes) == 9)
         docs = root.get_node('docs')
         # we haven't yet accessed anything new as docs dir was already cached
         self.assertTrue(len(chset.nodes) == 9)
         self.assertTrue(len(docs.nodes) == 8)
         # accessing docs.nodes updates chset.nodes
         self.assertTrue(len(chset.nodes) == 17)

0 comments (0 inline, 0 general)