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

Handle errors during create_url_adapter #2994

Merged
merged 3 commits into from Nov 13, 2018
Merged

Handle errors during create_url_adapter #2994

merged 3 commits into from Nov 13, 2018

Conversation

@jarek
Copy link
Contributor

@jarek jarek commented Nov 12, 2018

If create_url_adapter raises (which it can if werkzeug cannot bind environment, for example on non-ASCII non-printable Host header), we should handle it as other routing exceptions rather than raising through. This allows us to return a HTTP 400, rather than a 500 from unhandled exception.

This came up in pallets/werkzeug#640

Do we need to add it to any documentation or changelogs?

If create_url_adapter raises (which it can if werkzeug cannot bind
environment, for example on non-ASCII Host header), we handle it as
other routing exceptions rather than raising through.

ref pallets/werkzeug#640
@jarek
Copy link
Contributor Author

@jarek jarek commented Nov 12, 2018

Will take a look at the tests, It Worked On My Machine™

Loading

@jarek
Copy link
Contributor Author

@jarek jarek commented Nov 13, 2018

Failures are in the test doing response = app.test_client().get('/', headers={'host': 'ąśź.com'})

On my local machine:

  • on Python 3.5.2, testing with get('/', headers={'host': 'ąöж'}) fails (HTTP 200 is returned?!)
  • on Python 2.7.12, testing with get('/', headers={'host': 'ąöж'}) works (HTTP 400 is returned)
  • on Python 3.5.2, testing with get('/', headers={'host': '\x8a'}) (the test case given in original issue) works as expected (HTTP 400 is returned)
  • on Python 2.7.12, testing with get('/', headers={'host': '\x8a'}) raises exception in the test code (I think?):
venv2/local/lib/python2.7/site-packages/werkzeug/test.py:830: in get
    return self.open(*args, **kw)
flask/testing.py:200: in open
    follow_redirects=follow_redirects
venv2/local/lib/python2.7/site-packages/werkzeug/test.py:803: in open
    response = self.run_wsgi_app(environ, buffered=buffered)
venv2/local/lib/python2.7/site-packages/werkzeug/test.py:718: in run_wsgi_app
    self.cookie_jar.extract_wsgi(environ, rv[2])
venv2/local/lib/python2.7/site-packages/werkzeug/test.py:191: in extract_wsgi
    U2Request(get_current_url(environ)),
venv2/local/lib/python2.7/site-packages/werkzeug/wsgi.py:99: in get_current_url
    return uri_to_iri(''.join(tmp))
venv2/local/lib/python2.7/site-packages/werkzeug/urls.py:614: in uri_to_iri
    uri = url_parse(to_unicode(uri, charset))
venv2/local/lib/python2.7/site-packages/werkzeug/_compat.py:206: in to_unicode
    return x.decode(charset, errors)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

input = 'http://\x8a/', errors = 'strict'

    def decode(input, errors='strict'):
>       return codecs.utf_8_decode(input, errors, True)
E       UnicodeDecodeError: 'utf8' codec can't decode byte 0x8a in position 7: invalid start byte

http://\x8a/ looks like it's trying to do more with that Host header than I expected. Will dig in some more.

Loading

We've discovered that passing Unicode in Host actually works, except for
test client limitations on Python 2 - and the only things that don't
work are non-printable characters.
@davidism davidism added this to the 1.1 milestone Nov 13, 2018
@davidism davidism merged commit ca23b7b into pallets:master Nov 13, 2018
2 checks passed
Loading
@jarek jarek deleted the werkzeug-640-exceptions-during-bind branch Nov 13, 2018
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants