[PATCH] login: preserve GET arguments throughout login redirection (issue #104)

[Mads Kiilerich]

On 05/20/2015 01:04 PM, Thomas De Schampheleire wrote:
> On Wed, May 20, 2015 at 12:16 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 05/20/2015 09:22 AM, Thomas De Schampheleire wrote:
>>> # HG changeset patch
>>> # User Thomas De Schampheleire <thomas.de.schampheleire at gmail.com>
>>> # Date 1432065035 -7200
>>> #      Tue May 19 21:50:35 2015 +0200
>>> # Node ID cedc3ee5ef792a77a515997c90b38099f4688166
>>> # Parent  579110ca5178f13254e7e4c7b6043767a11b92a2
>>> login: preserve GET arguments throughout login redirection (issue #104)
>>>
>>> When redirecting a user to the login page and while handling this login
>>> and
>>> redirecting to the original page, the GET arguments passed to the original
>>> URL are lost through the login redirection process.
>>>
>>> For example, when creating a pull request for a specific revision from the
>>> repository changelog, there are rev_start and rev_end arguments passed in
>>> the URL. Through the login redirection, they are lost.
>>>
>>> Fix the issue by passing along the GET arguments to the login page, in the
>>> login form action, and when redirecting back to the original page.
>>> Tests are added to cover these cases.
>>>
>>> diff --git a/kallithea/controllers/login.py
>>> b/kallithea/controllers/login.py
>>> --- a/kallithea/controllers/login.py
>>> +++ b/kallithea/controllers/login.py
>>> @@ -100,7 +100,12 @@ class LoginController(BaseController):
>>>                log.error('Suspicious NETLOC detected %s for url %s server
>>> url '
>>>                          'is: %s' % (parsed.netloc, parsed, server_parsed))
>>>                came_from = url('home')
>>> -        return came_from
>>> +        return came_from.encode('ascii')
>>
>> Why this? Should the type of the result from this function really be
>> changed? What if the content cannot be encoded to ascii?
>>
>> (Might be worth adding a test for this.)
> The url method does not accept unicode. If you do not make this change
> you get an error about it.
> It is possible to move the encoding to the index method, but I think
> it boils down to the same thing.
>
> What is an example of an url that cannot be encoded to ascii?

http://localhost:5000/?Strauß=blåbærgrød which in URL encoding is
http://localhost:5000/?Strau%C3%9F=bl%C3%A5b%C3%A6rgr%C3%B8d

Redirect from such a URL will with this change fail with
Error - <type 'exceptions.UnicodeDecodeError'>: 'ascii' codec can't 
decode byte 0xc3 in position 5: ordinal not in range(128)

I guess the right solution is that the url method _should_ accept 
unicode and do url encoding of it.

> Unicode characters in repo names are not yet fully supported in
> Kallithea anyway. For example, I just created a repository with name
> repo_Strauß. The creation itself worked fine, but pushing into it
> fails with 500 Internal Error, caused by sqlalchemy error:
>
> ProgrammingError: (ProgrammingError) You must not use 8-bit
> bytestrings unless you use a text_factory that can interpret 8-bit
> bytestrings (like text_factory = str). It is highly recommended that
> you instead just switch your application to Unicode strings.

I guess that is something someone might want to fix one day ;-)

Until then, we shouldn't make it worse. Especially here, where 
unauthenticated users can spew internal errors in the server log.

/Mads