Skip to content

Commit

Permalink
Root app middleware receives wrong app context (#2550)
Browse files Browse the repository at this point in the history
* add failing test showing invalid request.app in middleware

* implement UrlMappingMatchInfo.current_app attribute with context manger setter

* fix bad inlined-inlined-loop)

* Add changes
  • Loading branch information
popravich authored and asvetlov committed Nov 27, 2017
1 parent b7084cf commit cff60eb
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGES/2550.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make `request.app` point to proper application instance when using nested applications (with middlewares).
8 changes: 6 additions & 2 deletions aiohttp/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from .web_exceptions import * # noqa
from .web_fileresponse import * # noqa
from .web_middlewares import * # noqa
from .web_middlewares import _fix_request_current_app
from .web_protocol import * # noqa
from .web_request import * # noqa
from .web_response import * # noqa
Expand Down Expand Up @@ -294,6 +295,8 @@ def _prepare_middleware(self):
'see #2252'.format(m),
DeprecationWarning, stacklevel=2)
yield m, False
if self._middlewares:
yield _fix_request_current_app(self), True

@asyncio.coroutine
def _handle(self, request):
Expand Down Expand Up @@ -326,8 +329,9 @@ def _handle(self, request):
("Handler {!r} should return response instance, "
"got {!r} [middlewares {!r}]").format(
match_info.handler, type(resp),
[middleware for middleware in app.middlewares
for app in match_info.apps])
[middleware
for app in match_info.apps
for middleware in app.middlewares])
return resp

def __call__(self):
Expand Down
13 changes: 11 additions & 2 deletions aiohttp/web_middlewares.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def normalize_path_middleware(

@asyncio.coroutine
@middleware
def normalize_path_middleware(request, handler):
def impl(request, handler):
if isinstance(request.match_info.route, SystemRoute):
paths_to_check = []
if '?' in request.raw_path:
Expand All @@ -81,4 +81,13 @@ def normalize_path_middleware(request, handler):

return (yield from handler(request))

return normalize_path_middleware
return impl


def _fix_request_current_app(app):

@middleware
async def impl(request, handler):
with request.match_info.set_current_app(app):
return await handler(request)
return impl
4 changes: 2 additions & 2 deletions aiohttp/web_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -623,10 +623,10 @@ def match_info(self):
"""Result of route resolving."""
return self._match_info

@reify
@property
def app(self):
"""Application instance."""
return self._match_info.apps[-1]
return self._match_info.current_app

@asyncio.coroutine
def _prepare_hook(self, response):
Expand Down
20 changes: 20 additions & 0 deletions aiohttp/web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import warnings
from collections import namedtuple
from collections.abc import Container, Iterable, Sequence, Sized
from contextlib import contextmanager
from functools import wraps
from pathlib import Path
from types import MappingProxyType
Expand Down Expand Up @@ -195,6 +196,7 @@ def __init__(self, match_dict, route):
super().__init__(match_dict)
self._route = route
self._apps = ()
self._current_app = None
self._frozen = False

@property
Expand Down Expand Up @@ -223,8 +225,26 @@ def apps(self):
def add_app(self, app):
if self._frozen:
raise RuntimeError("Cannot change apps stack after .freeze() call")
if self._current_app is None:
self._current_app = app
self._apps = (app,) + self._apps

@property
def current_app(self):
return self._current_app

@contextmanager
def set_current_app(self, app):
assert app in self._apps, (
"Expected one of the following apps {!r}, got {!r}"
.format(self._apps, app))
prev = self._current_app
self._current_app = app
try:
yield
finally:
self._current_app = prev

def freeze(self):
self._frozen = True

Expand Down
38 changes: 38 additions & 0 deletions tests/test_web_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,44 @@ def on_signal(app):
assert [app, subapp1, subapp2] == order


@pytest.mark.parametrize('route,expected', [
('/sub/', ['app see root', 'subapp see sub']),
('/', ['app see root']),
])
@asyncio.coroutine
def test_subapp_middleware_context(loop, test_client, route, expected):
values = []

def show_app_context(appname):
@web.middleware
@asyncio.coroutine
def middleware(request, handler):
values.append('{} see {}'.format(appname, request.app['my_value']))
return (yield from handler(request))
return middleware

@asyncio.coroutine
def handler(request):
return web.Response(text='Ok')

app = web.Application()
app['my_value'] = 'root'
app.middlewares.append(show_app_context('app'))
app.router.add_get('/', handler)

subapp = web.Application()
subapp['my_value'] = 'sub'
subapp.middlewares.append(show_app_context('subapp'))
subapp.router.add_get('/', handler)
app.add_subapp('/sub/', subapp)

client = yield from test_client(app)
resp = yield from client.get(route)
assert 200 == resp.status
assert 'Ok' == (yield from resp.text())
assert expected == values


@asyncio.coroutine
def test_custom_date_header(loop, test_client):

Expand Down

0 comments on commit cff60eb

Please sign in to comment.