Kallithea issues archive

When attempting to view a changeset in a Git repository while passing-in a large context value to the controller, an internal server error (500) will be thrown at user.

This affects both the default and stable branch. Below reproduction steps etc are centered around default branch, stable branch will have similar result with different tracebacks.

Reproduction steps:

1. Set-up Kallithea (minimal development environment described in contributing.rst should suffice. Below instructions assume the user will be called admin for simplicity sake.

2. Log-in into Kallithea and create a new Git repository called test-changeset-context-500.

3. Clone the empty repository:

   cd /tmp/
   git clone http://admin@localhost:5000/test-changeset-context-500
   

4. Add a file to repository:

   cd /tmp/test-changeset-context-500
   echo "This is the initial file revision." > README.rst
   git add README.rst
   git commit -m "Added README.rst."
   

5. Make a change to committed file:

   cd /tmp/test-changeset-context-500
   echo "This is the second file revision." > README.rst
   git add README.rst
   git commit -m "Added README.rst."
   

6. Push the changes (at the moment of this writing some errors might end-up being reported by Kallithea, but those are not relevant for this particular issue):

   cd /tmp/test-changeset-context-500
   git push
   

7. Log-in into Kallithea and open the changelog page for test-changeset-context-500 repository.

8. Click on the top changeset.

9. Modify the resulting URL to changeset by appending ?context=2147483648 to it, and open it in a browser. For example: http://localhost:5000/test-changeset-context-500/changeset/82bdfeb3c90a600b3cdcb4c66769cc7f0af1a017?context=2147483648.

Expected results:

1. In step (9), changeset is shown to user with all the relevant details.

Actual resuls:

1. In step (9), a 500 Internal Server Error error is shown to the user, with the following traceback:

   Traceback (most recent call last):
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/appwrappers/session.py", line 71, in __call__
       response = self.next_handler(controller, environ, context)
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/appwrappers/i18n.py", line 71, in __call__
       return self.next_handler(controller, environ, context)
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/wsgiapp.py", line 285, in _dispatch
       return controller(environ, context)
     File "/home/user/projects/kallithea/kallithea/lib/base.py", line 553, in __call__
       return super(BaseController, self).__call__(environ, context)
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/controllers/dispatcher.py", line 119, in __call__
       response = self._perform_call(context)
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/controllers/dispatcher.py", line 108, in _perform_call
       r = self._call(action, params, remainder=remainder, context=context)
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/controllers/decoratedcontroller.py", line 119, in _call
       output = controller_caller(context_config, bound_controller_callable, remainder, params)
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/decorators.py", line 44, in _decorated_controller_caller
       return application_controller_caller(tg_config, controller, remainder, params)
     File "/home/user/.virtualenvs/kallithea/lib/python2.7/site-packages/tg/configuration/app_config.py", line 127, in call_controller
       return controller(*remainder, **params)
     File "<decorator-gen-50>", line 2, in index
   
     File "/home/user/projects/kallithea/kallithea/lib/auth.py", line 810, in __wrapper
       return func(*fargs, **fkwargs)
     File "<decorator-gen-49>", line 2, in index
   
     File "/home/user/projects/kallithea/kallithea/lib/auth.py", line 860, in __wrapper
       return func(*fargs, **fkwargs)
     File "/home/user/projects/kallithea/kallithea/controllers/changeset.py", line 332, in index
       return self._index(revision, method=method)
     File "/home/user/projects/kallithea/kallithea/controllers/changeset.py", line 278, in _index
       diff_limit=diff_limit)
     File "/home/user/projects/kallithea/kallithea/lib/diffs.py", line 302, in __init__
       self.parsed = self._parse_gitdiff(inline_diff=inline_diff)
     File "/home/user/projects/kallithea/kallithea/lib/diffs.py", line 378, in _parse_gitdiff
       chunks, added, deleted = _parse_lines(diff_lines)
     File "/home/user/projects/kallithea/kallithea/lib/diffs.py", line 570, in _parse_lines
       raise Exception('error parsing diff @@ line %r' % line)
   Exception: error parsing diff @@ line u'@@ -2147483649,4294967295- +2147483649,4294967295- @@\n'
   

Additional notes:

The issue stems from Git itself, most likely due to use of long int for storing request context passed-in as part of a git diff call. Looking at kallithea.lib.vcs.backends.git.repository.GitRepository.get_diff one can see the diff will be called with:

    git diff -U2147483648 --full-index --binary -p -M --abbrev=40 CHANGESET1 CHANGESET2

If we run the same command direclty with git on cloned repository, the output will look roughly as:

    diff --git a/README.rst b/README.rst
    index 6fdfae0f1c6e146d76d31504b1cfabb5b63ab8c8..a262de6fb2424cf167b409671354ce706add7153 100644
    --- a/README.rst
    +++ b/README.rst
    @@ -2147483649,4294967295- +2147483649,4294967295- @@
    -This is the initial file revision.
    +This is the second file revision.

Note that the @@ line essentially looks like it essentially suffers from integer overflow. Should we try to reduce the context by one (to 2147483647), we'll get output:

    diff --git a/README.rst b/README.rst
    index 6fdfae0f1c6e146d76d31504b1cfabb5b63ab8c8..a262de6fb2424cf167b409671354ce706add7153 100644
    --- a/README.rst
    +++ b/README.rst
    @@ -1 +1 @@
    -This is the initial file revision.
    +This is the second file revision.

Within Kallithea itself, the problem arises when the regex kallithea.lib.diffs._chunk_re fails to match old_line, old_end, new_line, new_end at kallithea/lib/diffs.py:567, due to extra minus sign after 4294967295.

Mercurial does not seem to suffer from this limitation, even when passing huge values to -U.

Ideally, this should be fixed in Git, but it might be much easier to fix it in Kallithea. Not sure what the best course of action should be, with some possibilities being:

1. Throw a specific error at user stating that the context value has been exceeded.

2. Silently "truncate" the context value to the next valid one.

3. "Truncate" the context value to the next valid one, but show a warning message to user. This might give the best consistency accross Mercurial/Git, while providing at least some feedback on passed-in value being incorrect.

Another thing to keep in mind is that this could be done either at controller or kallithea.lib.vcs.backends.git.repository.GitRepository.get_diff level. Doing it at get_diff level might be more future-proof, but then comes the issue of how to pass on a warning to controller. Maybe doing it on both sides could prove useful - so controller will sanitize and produce warning message for user, while get_diff would sanitize and log a warning instead (so any possible future controller implementation would be aware of it in some way).