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

Default Aiohttp request parser HttpRequestParserC fails, parsing URLs of allowed size. #2311

Closed
knholovan opened this issue Oct 10, 2017 · 4 comments
Assignees
Labels

Comments

@knholovan
Copy link

Long story short

Default Aiohttp request parser HttpRequestParserC fails, parsing URLs of allowed size (up to 8190 bytes allowed).

https://github.com/aio-libs/aiohttp/blob/master/aiohttp/http_parser.py#L658

Expected behaviour

URL parser should not raise unexpected errors, working with URLs of allowed size and length.

Actual behaviour

url = 'http://<host>:<port>/api?ids=ac886e02-bda7-41db-b3a2-257cba5824cf&ids=...&ids=85a8b805-bf92-45ac-994b-9ceb1bfa981a'
# 100 key/values in get parameters
# Totally 4 Kb, which is half of allowed 8.19 Kb for URL size
response = await client.get(url)

ERROR aiohttp.server
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/aiohttp/web_protocol.py", line 278, in data_received
    messages, upgraded, tail = self._request_parser.feed_data(data)
  File "aiohttp/_http_parser.pyx", line 274, in aiohttp._http_parser.HttpParser.feed_data (aiohttp/_http_parser.c:4364)
  File "aiohttp/_http_parser.pyx", line 334, in aiohttp._http_parser.cb_on_url (aiohttp/_http_parser.c:5381)
  File "aiohttp/_http_parser.pyx", line 546, in aiohttp._http_parser._parse_url (aiohttp/_http_parser.c:8903)
aiohttp.http_exceptions.InvalidURLError: 400, message='invalid url b'=9a93-80409935daa8&ids=...&ids=85a8b805-bf92-45ac-994b-9ceb1bfa981a'

Invalid URL is actually a last truncated part of original.

Steps to reproduce

Leave environment variable AIOHTTP_NO_EXTENSIONS unchanged, False by default.
That makes aiohttp app use default HttpRequestParser = HttpRequestParserC.
Run your application

import asyncio
import sys

import uvloop

from aiohttp import ClientSession, web


async def api_handler(request: web.Request) -> web.Response:
    get_params = request.rel_url.query
    # ...
    return web.json_response()


def main(host: str, port: int):
    asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
    loop = asyncio.get_event_loop()

    app = web.Application(loop=loop)
    app.router.add_route('GET', '/api', api_handler)

    client = ClientSession(loop=loop)
    handler = app.make_handler(loop=loop)

    try:
        loop.run_until_complete(app.startup())
        loop.run_until_complete(loop.create_server(handler, host, port))
        loop.run_forever()
    except KeyboardInterrupt:
        ...
    finally:
        loop.run_until_complete(app.cleanup())
        loop.run_until_complete(app.shutdown())
        client.close()
        loop.close()


if __name__ == '__main__':
    sys.exit(main('localhost', 8000))

Try to access your API endpoint from remote (important - not from same localhost)
response = await client.get('http://<host_ip>:8000/api?ids=uuid1&ids=...&ids=uuid100'
Raised error, described above.

Your environment

Python 3.6.3
Aiohttp 2.2.5
Ubuntu 14.04 LTS

@tailhook
Copy link
Contributor

Basically, this is because nginx parser can feed data in chunks, but pyx code expects url to be received from parser as a whole.

Excerpt from parser sources:

http_data_cb does not return data chunks. It will be called arbitrarily
many times for each string. E.G. you might get 10 callbacks for "on_url"
each providing just a few characters more data.

By the way, it looks like the same is true even for status field.

Basically, I recommend not to use C parser until this issue is fixed as it can be a security issue.

@tailhook
Copy link
Contributor

Let me explain more, how it breaks, in this code firstly callback receives the first part of url, which is fine. Then it receives next part of the url, and instead of appending, it tries to parse this chunk as a full URL and fails.

@fafhrd91
Copy link
Member

Should be relatevly easy to fix

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants