Aiohttp: match_info missing in cloned request

[achimnol]

Long story short

I am cloning the given request object when there is a special header X-Method-Override so that my users can bypass some overly-restrictive firewalls that doesn't allow HTTP methods other than GET/POST.

To implement this feature, I wrote a middleware like this:

async def api_middleware_factory(app, handler):
    async def api_middleware_handler(request):
        method_override = request.headers.get('X-Method-Override', None)
        if method_override:
            request = request.clone(method=method_override)
        if request.match_info.http_exception is not None:
            return request.match_info.http_exception
        return (await handler(request))
    return api_middleware_handler

And I have a test case like below, using pytest-aiohttp plugin:

async def test_api_method_override(test_client):
    inner_request = None
    app = web.Application()

    async def dummy_handler(request):
        nonlocal inner_request
        inner_request = request
        return web.Response(body=b'test')

    app.router.add_post('/v{version:\d+}/version', dummy_handler)
    app.middlewares.append(api_middleware_factory)
    client = await test_client(app)

    resp = await client.post('/v2/version', headers={
        'X-Method-Override': 'REPORT',
    })
    assert resp.status == 200
    assert inner_request.method == 'REPORT'

Expected behaviour

The test case passes: match_info is cloned as well, so checking request.match_info.http_exception should be fine.

Actual behaviour

The test case fails with:

Error handling request
Traceback (most recent call last):
  File "/Users/joongi/Projects/Lablup/backend.ai-manager/venv/lib/python3.6/site-packages/aiohttp/web_protocol.py", line 422, in start
    resp = yield from self._request_handler(request)
  File "/Users/joongi/Projects/Lablup/backend.ai-manager/venv/lib/python3.6/site-packages/aiohttp/web.py", line 306, in _handle
    resp = yield from handler(request)
  File "/Users/joongi/Projects/Lablup/backend.ai-manager/ai/backend/gateway/server.py", line 73, in api_middleware_handler
    if request.match_info.http_exception is not None:
AttributeError: 'NoneType' object has no attribute 'http_exception'

It seems that match_info is missing in the cloned request.

I also found a suspective code of aiohttp which explicitly retry resolving of the request object after cloning and assigning the private member _match_info of the cloned request with the new resolve result. Is this the only way for my use-case or am I missing something?

Steps to reproduce

Simplified sample codes are given above.

Your environment

macOS 10.13, Python 3.6.3 via homebrew + pyenv, aiohttp 2.2.5, pytest-aiohttp 0.1.3

[achimnol]

Hm... I just realized that middleware might not be a right place to override methods of requests.
I need to do resolve again, but to execute my middleware in the first place, the original request also need to be resolved. This means, I need to configure duplicate routes for both POST and other exotic methods. This may be acceptable, and doing so makes the test case happy.

Still, I feel this like a dirty hacking, because I need to use a private API (request._match_info) and cannot deduplicate the routes.

async def api_middleware_factory(app, handler):
    async def api_middleware_handler(request):
        method_override = request.headers.get('X-Method-Override', None)
        if method_override:
            request = request.clone(method=method_override)
            # resolve again with the new exotic method
            new_match_info = await app.router.resolve(request)
            # however, the handler bound to POST is already set by aiohttp
            request._match_info = new_match_info  # need to use private API :(
        assert request.match_info is not None
        if request.match_info.http_exception is not None:
            return request.match_info.http_exception
        return (await handler(request))
    return api_middleware_handler

async def test_api_method_override(test_client):
    observed_method = None
    app = web.Application()

    async def service_handler(request):
        nonlocal observed_method
        observed_method = request.method
        return web.Response(body=b'test')

    async def noop_handler(request):
        return web.Response(body=b'noop')

    app.router.add_route('POST', '/v{version:\d+}/version', service_handler)  # actual handler
    app.router.add_route('REPORT', '/v{version:\d+}/version', noop_handler)  # placeholder
    app.middlewares.append(api_middleware_factory)
    client = await test_client(app)

    resp = await client.post('/v2/version', headers={
        'X-Method-Override': 'REPORT',
    })
    assert resp.status == 200
    assert (await resp.read()) == b'test'
    assert observed_method == 'REPORT'

    observed_method = None
    resp = await client.delete('/v2/version')
    assert resp.status == 405
    assert observed_method is None

[asvetlov]

@achimnol thanks for report.
I'll make a fix for clone() today.
Hope to publish next aiohtp release tomorrow.

[achimnol]

I think the original implementation makes sense: changing/cloning a request in the middleware should lead to resolving it again, though there are some abstraction holes currently.

[asvetlov]

Well, double resolving definitely makes sense in some cases but match_info should be cloned by default.

[asvetlov]

Regarding to your initial request: I not sure if method changing should be done by middleware.
But honestly I have no idea for the best solution.
I suspect your need is very rare.

[lock[bot]]

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].