Skip to content

Handle errors during create_url_adapter #2994

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.

Sign up for GitHub

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

Jump to bottom
Merged
davidism merged 3 commits into from
Nov 13, 2018
Merged

Handle errors during create_url_adapter #2994

davidism merged 3 commits into from
Nov 13, 2018

Conversation

@jarek
Copy link
Contributor

@jarek jarek commented Nov 12, 2018 •
edited
Loading

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?

@jarek
Handle errors during create_url_adapter
ed9775f 
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 commented Nov 12, 2018

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

@jarek
Copy link
Contributor Author

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.

@jarek
Correct tests
9b0bd91 
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
@jarek jarek deleted the werkzeug-640-exceptions-during-bind branch November 13, 2018 18:56
@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.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@jarek @davidism