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

Unhandled exception when httptools.parse_url() returns a None path #58

Closed
thedrow opened this issue Jan 29, 2018 · 3 comments
Closed

Unhandled exception when httptools.parse_url() returns a None path #58

thedrow opened this issue Jan 29, 2018 · 3 comments

Comments

@thedrow
Copy link

thedrow commented Jan 29, 2018

I encountered the following exception while running an apistar application using uvicorn 0.0.15 on CPython 3.6.4 with SSL enabled:

Unhandled exception in event loop
Traceback (most recent call last):
  File "httptools/parser/parser.pyx", line 241, in httptools.parser.parser.cb_on_url
  File "/home/omer/.local/share/virtualenvs/myproject--RkZn49j/lib/python3.6/site-packages/uvicorn/protocols/http.py", line 278, in on_url
    'path': parsed.path.decode('ascii'),
AttributeError: 'NoneType' object has no attribute 'decode'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "uvloop/handles/stream.pyx", line 784, in uvloop.loop.__uv_stream_on_read_impl
  File "uvloop/handles/stream.pyx", line 563, in uvloop.loop.UVStream._on_read
  File "/home/omer/.pyenv/versions/3.6.4/lib/python3.6/asyncio/sslproto.py", line 514, in data_received
    self._app_protocol.data_received(chunk)
  File "/home/omer/.local/share/virtualenvs/myproject--RkZn49j/lib/python3.6/site-packages/uvicorn/protocols/http.py", line 238, in data_received
    self.request_parser.feed_data(data)
  File "httptools/parser/parser.pyx", line 189, in httptools.parser.parser.HttpParser.feed_data
httptools.parser.errors.HttpParserCallbackError: the on_url callback failed

The exception occurs when the empty HTTP message b' HTTP/1.1\r\n' is provided in https://github.com/encode/uvicorn/blob/master/uvicorn/protocols/http.py#L238

Unfortunately I'm unable to reproduce this problem with requests or curl.
So far the only HTTP client that reproduces this bug is OSQuery 2.11.2 with the remote API enabled. OSQuery 2.8.0 does not present the same problem.

I'm running uvicorn with the following parameters:

uvicorn myproject.app:app \
            --workers=8 \
            --bind=localhost:8080 \
            --keyfile=./server.pem \
            --certfile=./server.pem \
            --ca-certs=./ca.pem \
            --log-level DEBUG \
            --access-logfile=- \
            --do-handshake-on-connect \
            --ciphers TLSv1.2

and OSQuery with the following parameters:

sudo osqueryd --pidfile /tmp/osqueryd.pid --verbose --debug \
    --database_path /tmp/osquery.db/ \
    --tls_hostname localhost:8080 \
    --config_plugin tls \
    --tls_server_certs /usr/local/share/ca-certificates/ca.crt \
    --config_tls_endpoint /collection/configuration \
    --logger_tls_endpoint /collection/log_event \
    --logger_plugin tls \
    --enroll_tls_endpoint /collection/enroll \
    --disable_distributed=false --distributed_plugin=tls --distributed_interval=60 \
    --distributed_tls_read_endpoint /collection/queries \
    --distributed_tls_write_endpoint /collection/results \
    --watchdog_level -1 \
    --enroll_secret_path ./etc/test_enroll_secret.txt

In order to reproduce the problem you'll need to generate your own certificates.
I recommend trustme for that purpose.

I'm going to try to reverse proxy with Nginx next to figure out if OSQuery is at fault here but in any case, the uvicorn should handle malformed (or incomplete? not sure about that) HTTP requests more gracefully.

@thedrow
Copy link
Author

thedrow commented Jan 29, 2018

We're lucky that the fix is pretty easy :)

@tomchristie
Copy link
Member

Doesn't look like a valid HTTP message to me.
I'm not sure what behavior is expected from the parser when invalid sequences are encountered - happy to consider this in more detail once #63 is in, but #59 doesn't seem like the right approach.

@thedrow
Copy link
Author

thedrow commented Apr 9, 2018

I'm currently using nginx as a proxy and it handles this correctly.
I simply did not have the time to create a test case for this issue.

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

No branches or pull requests

2 participants