New issue 306: Internal server error when passing-in large context value for
changeset with Git repository
https://bitbucket.org/conservancy/kallithea/issues/306/internal-server-error-when-passing-in
Branko Majic:
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](http://localhost:5000/test-changeset-context-500/changelog).
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).
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
