Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebOb does not validate if environment variables are str #247

Closed
bimusiek opened this issue Apr 18, 2016 · 3 comments · Fixed by #390
Closed

WebOb does not validate if environment variables are str #247

bimusiek opened this issue Apr 18, 2016 · 3 comments · Fixed by #390

Comments

@bimusiek
Copy link

WebOb changes Location header to make it absolute.
So ("Location", "/pay/select/") becomes ("Location", u"http://hern.as/pay/select/").
Problem is that it combine strings using plus sign +.

Problem lies for this specific case in _request_uri method that does

url += url_quote(script_name)

which from str makes unicode.

This leads to that kind of exception:

Internal Server Error

Traceback (most recent call last):
  File "/Users/bim/Developer/Inventorum/site/eggs/waitress-0.9.0-py2.7.egg/waitress/channel.py", line 338, in service
    task.service()
  File "/Users/bim/Developer/Inventorum/site/eggs/waitress-0.9.0-py2.7.egg/waitress/task.py", line 169, in service
    self.execute()
  File "/Users/bim/Developer/Inventorum/site/eggs/waitress-0.9.0-py2.7.egg/waitress/task.py", line 399, in execute
    app_iter = self.channel.server.application(env, start_response)
  File "/Users/bim/Developer/Inventorum/site/eggs/Paste-2.0.3-py2.7.egg/paste/urlmap.py", line 216, in __call__
    return app(environ, start_response)
  File "/Users/bim/Developer/Inventorum/site/eggs/fanstatic-1.0a7-py2.7.egg/fanstatic/publisher.py", line 224, in __call__
    return self.app(environ, start_response)
  File "/Users/bim/Developer/Inventorum/site/eggs/fanstatic-1.0a7-py2.7.egg/fanstatic/injector.py", line 86, in __call__
    return response(environ, start_response)
  File "/Users/bim/Developer/Inventorum/site/eggs/WebOb-1.6.0-py2.7.egg/webob/response.py", line 1037, in __call__
    start_response(self.status, headerlist)
  File "/Users/bim/Developer/Inventorum/site/eggs/waitress-0.9.0-py2.7.egg/waitress/task.py", line 375, in start_response
    'Header value %r is not a string in %r' % (v, (k, v))
AssertionError: Header value u'http://127.0.0.1:5012/de/pay/select' is not a string in ('Location', u'http://127.0.0.1:5012/de/pay/select')

Originally reported at Pylons/waitress#125
Fix for Django (to not send unicode path info and script name): django/django#6472

@digitalresistor
Copy link
Member

Thank you :-).

@digitalresistor
Copy link
Member

This issue is limited to Python 2.x only:

def test_location_unicode():
    environ = {
        'REQUEST_METHOD': 'GET',
        'wsgi.url_scheme': 'http',
        'HTTP_HOST': u'test.com',
    }
    res = Response()
    res.status = '301'
    res.location = '/test.html'

    def start_response(status, headerlist):
        for (header, val) in headerlist:
            if header.lower() == 'location':
                assert val == 'http://test.com/test.html'
                assert isinstance(val, str)

    res(environ, start_response)

I've added a new test that is marked as xfail on Py 2.x. I am not sure what the best way forward is to fix this when something messes with the environ and thereby violates the requirements as set forth by PEP333 and PEP3333.

digitalresistor added a commit that referenced this issue Nov 18, 2016
In this case a downstream application messes with the environ, thereby
causing WebOb to emit a unicode Location header.

Only affects Py2.
@bimusiek
Copy link
Author

Honestly, I liked the idea that it should just raise exception explaining what went wrong. And the bug is in Django, no idea if someone fixed it as they did not merge my PR ( django/django#6472 ) because of lack of follow-up. I don't have time to work on it though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants