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

@@ @@ -587,8 +587,30 @@ class GitRepository(BaseRepository): @@
         :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')

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

➞

Show inline comments

@@ @@ -244,8 +244,15 @@ class MercurialRepository(BaseRepository @@
         :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')

kallithea/tests/vcs/test_git.py

➞

Show inline comments

@@ @@ -727,6 +727,46 @@ class GitSpecificWithRepoTest(_BackendTe @@
             ['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'

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
@@ @@ -231,6 +234,23 @@ TODO: To be written... @@
         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):

0 comments (0 inline, 0 general)