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

decorator to add per handler annotations #4174

Closed
gjcarneiro opened this issue Oct 10, 2019 · 6 comments
Closed

decorator to add per handler annotations #4174

gjcarneiro opened this issue Oct 10, 2019 · 6 comments

Comments

@gjcarneiro
Copy link
Contributor

Long story short

With my custom authentication, trying to have an authentication middleware that requires login by default, except for the handlers specifically annotated with a decorator.

def login_exempt(func: F) -> F:
    func.weblogin_login_exempt = True  # type: ignore
    return func

@web.middleware
async def auth_middleware(
    request: web.Request,
    handler: Callable[[web.Request], Awaitable[web.StreamResponse]],
) -> web.StreamResponse:
    print(handler, hasattr(handler, "weblogin_login_exempt"))
    if not hasattr(handler, "weblogin_login_exempt"):
        session = await weblogin.fetch_session(request)
        if not session:
            raise web.HTTPNotFound(
                text=json.dumps(weblogin.SESSION_NOT_FOUND),
                content_type="application/json",
            )
    return await handler(request)


def create_web_app() -> web.Application:
    app = web.Application(
        middlewares=[weblogin.session_middleware, auth_middleware]
    )
    app.on_startup.append(async_init)
    app.on_shutdown.append(do_shutdown)
    app.router.add_route("GET", "/healthz", healthz)

    app.add_subapp("/s/weblogin", weblogin.create_web_app())
    app.add_subapp("/s/userprefs", userprefs.create_web_app())

Expected behaviour

I expect the handler to have weblogin_login_exempt when it has been decorator, causing the framework to skip the usual auth checks.

Actual behaviour

Instead, at least in the case of a subapp, the handler is a functools.partial(), which means the actual handler function is hidden away.

functools.partial(<function _fix_request_current_app.<locals>.impl at 0x7efbf9fb48c8>, handler=<function login at 0x7efbf9f7ba60>)

Only for the toplevel Application is the handler unmodified. For a subapp, handler is wrapped as a functools.partial() object.

In general, I am either missing something obvious, or there is a lack of an API in aiohttp to achieve this sort of thing. I would like to be able to annotate handlers, somehow, and for middleware to fetch the handler annotations.

Steps to reproduce

Your environment

aiohttp 3.6.1, python 37.3.

@asvetlov
Copy link
Member

You are writing a middleware that is applicable to only a subset of application handlers.
I don't think that this is a good idea.

@gjcarneiro
Copy link
Contributor Author

It is only "not a good idea" because it's not possible to do it atm. But I think I have a great use case.

In my use case, I want by default to require authentication for all handlers. But there needs to be exceptions. So the middleware needs to find out what are the exceptions.

I solved it for now by having a simple constant, with a list of paths that the middleware will skip authentication, such as login. But it's not ideal.

Otherwise you would have to decorate all handlers with a @login_required decorator or some such. If you forget one, you can have a serious security vulnerability in the web app.

@gjcarneiro
Copy link
Contributor Author

I've been looking at how Django handles this, and I think it's an easy fix.

Consider a simple wrappers:

from functools import update_wrapper, wraps, partial


def my_decorator1(func):
    @wraps(func)
    def wrapper():
        return func()
    wrapper.xpto = "xpto"
    return wrapper


def my_decorator2(func):
    @wraps(func)
    def wrapper():
        return func()
    wrapper.zbr = "zbr"
    return wrapper

@my_decorator1
@my_decorator2
def handler(request, xxx):
    return


print(getattr(handler, "xpto"))
print(getattr(handler, "zbr"))

It prints:

xpto
zbr

That's because functools.wraps() will copy attributes over. But if you do functools.partial() it breaks the chain, and attributes are lost:

handler1 = partial(handler, xxx=2)

print(getattr(handler1, "xpto"))
print(getattr(handler1, "zbr"))

You get:

AttributeError: 'functools.partial' object has no attribute 'xpto'

It's easy to fix: just call functools.update_wrapper() after functools.partial().

handler1 = partial(handler, xxx=2)

update_wrapper(handler1, handler)
print(getattr(handler1, "xpto"))
print(getattr(handler1, "zbr"))

Now you get the expected result:

xpto
zbr

So I think we don't need a new API for tagging handlers. We merely need to be careful in aiohttp: whenever using functools.partial() on a handler, call functools.update_wrapper() after.

@asvetlov
Copy link
Member

You can try but I suspect the proposed fix doesn't work.

@gjcarneiro
Copy link
Contributor Author

It works but only if the middleware is the last one. For the other middlewares, the "handler" callable is another middleware instead of the view.

Anyway, it's better than nothing, and is a trivial change to aiohttp, so I hope you don't mind merging it.

I wish there was a better way. But it will require further research. Surely frameworks like Django have to deal with this already?...

@gjcarneiro
Copy link
Contributor Author

Actually, no, I take it back. It seems to be working with any middleware order. That's because the patch will copy attributes from the view handler into the middleware handler. functools.update_wraper() does modify the function, but anyway since we are already using functools.partial() before that, all middleware functions are aready implicitly copied for every request, so there is no risk of attributes leaking into middleware functions between one request and the next.

Been testing with my patch, and it seems to completely solve it. Any order of middlewares works (within reason, my auth middleware obviously needs the session middleware, long story).

netbsd-srcmastr referenced this issue in NetBSD/pkgsrc Oct 24, 2020
This fixes py-yarl in pkgsrc being too new for py-aiohttp.


3.7.0 (2020-10-24)
==================

Features
--------

- Response headers are now prepared prior to running ``on_response_prepare`` hooks, directly before headers are sent to the client.
  `#1958 <https://github.com/aio-libs/aiohttp/issues/1958>`_
- Add a ``quote_cookie`` option to ``CookieJar``, a way to skip quotation wrapping of cookies containing special characters.
  `#2571 <https://github.com/aio-libs/aiohttp/issues/2571>`_
- Call ``AccessLogger.log`` with the current exception available from ``sys.exc_info()``.
  `#3557 <https://github.com/aio-libs/aiohttp/issues/3557>`_
- `web.UrlDispatcher.add_routes` and `web.Application.add_routes` return a list
  of registered `AbstractRoute` instances. `AbstractRouteDef.register` (and all
  subclasses) return a list of registered resources registered resource.
  `#3866 <https://github.com/aio-libs/aiohttp/issues/3866>`_
- Added properties of default ClientSession params to ClientSession class so it is available for introspection
  `#3882 <https://github.com/aio-libs/aiohttp/issues/3882>`_
- Don't cancel web handler on peer disconnection, raise `OSError` on reading/writing instead.
  `#4080 <https://github.com/aio-libs/aiohttp/issues/4080>`_
- Implement BaseRequest.get_extra_info() to access a protocol transports' extra info.
  `#4189 <https://github.com/aio-libs/aiohttp/issues/4189>`_
- Added `ClientSession.timeout` property.
  `#4191 <https://github.com/aio-libs/aiohttp/issues/4191>`_
- allow use of SameSite in cookies.
  `#4224 <https://github.com/aio-libs/aiohttp/issues/4224>`_
- Use ``loop.sendfile()`` instead of custom implementation if available.
  `#4269 <https://github.com/aio-libs/aiohttp/issues/4269>`_
- Apply SO_REUSEADDR to test server's socket.
  `#4393 <https://github.com/aio-libs/aiohttp/issues/4393>`_
- Use .raw_host instead of slower .host in client API
  `#4402 <https://github.com/aio-libs/aiohttp/issues/4402>`_
- Allow configuring the buffer size of input stream by passing ``read_bufsize`` argument.
  `#4453 <https://github.com/aio-libs/aiohttp/issues/4453>`_
- Pass tests on Python 3.8 for Windows.
  `#4513 <https://github.com/aio-libs/aiohttp/issues/4513>`_
- Add `method` and `url` attributes to `TraceRequestChunkSentParams` and `TraceResponseChunkReceivedParams`.
  `#4674 <https://github.com/aio-libs/aiohttp/issues/4674>`_
- Add ClientResponse.ok property for checking status code under 400.
  `#4711 <https://github.com/aio-libs/aiohttp/issues/4711>`_
- Don't ceil timeouts that are smaller than 5 seconds.
  `#4850 <https://github.com/aio-libs/aiohttp/issues/4850>`_
- TCPSite now listens by default on all interfaces instead of just IPv4 when `None` is passed in as the host.
  `#4894 <https://github.com/aio-libs/aiohttp/issues/4894>`_
- Bump ``http_parser`` to 2.9.4
  `#5070 <https://github.com/aio-libs/aiohttp/issues/5070>`_


Bugfixes
--------

- Fix keepalive connections not being closed in time
  `#3296 <https://github.com/aio-libs/aiohttp/issues/3296>`_
- Fix failed websocket handshake leaving connection hanging.
  `#3380 <https://github.com/aio-libs/aiohttp/issues/3380>`_
- Fix tasks cancellation order on exit. The run_app task needs to be cancelled first for cleanup hooks to run with all tasks intact.
  `#3805 <https://github.com/aio-libs/aiohttp/issues/3805>`_
- Don't start heartbeat until _writer is set
  `#4062 <https://github.com/aio-libs/aiohttp/issues/4062>`_
- Fix handling of multipart file uploads without a content type.
  `#4089 <https://github.com/aio-libs/aiohttp/issues/4089>`_
- Preserve view handler function attributes across middlewares
  `#4174 <https://github.com/aio-libs/aiohttp/issues/4174>`_
- Fix the string representation of ``ServerDisconnectedError``.
  `#4175 <https://github.com/aio-libs/aiohttp/issues/4175>`_
- Raising RuntimeError when trying to get encoding from not read body
  `#4214 <https://github.com/aio-libs/aiohttp/issues/4214>`_
- Remove warning messages from noop.
  `#4282 <https://github.com/aio-libs/aiohttp/issues/4282>`_
- Raise ClientPayloadError if FormData re-processed.
  `#4345 <https://github.com/aio-libs/aiohttp/issues/4345>`_
- Fix a warning about unfinished task in ``web_protocol.py``
  `#4408 <https://github.com/aio-libs/aiohttp/issues/4408>`_
- Fixed 'deflate' compression. According to RFC 2616 now.
  `#4506 <https://github.com/aio-libs/aiohttp/issues/4506>`_
- Fixed OverflowError on platforms with 32-bit time_t
  `#4515 <https://github.com/aio-libs/aiohttp/issues/4515>`_
- Fixed request.body_exists returns wrong value for methods without body.
  `#4528 <https://github.com/aio-libs/aiohttp/issues/4528>`_
- Fix connecting to link-local IPv6 addresses.
  `#4554 <https://github.com/aio-libs/aiohttp/issues/4554>`_
- Fix a problem with connection waiters that are never awaited.
  `#4562 <https://github.com/aio-libs/aiohttp/issues/4562>`_
- Always make sure transport is not closing before reuse a connection.

  Reuse a protocol based on keepalive in headers is unreliable.
  For example, uWSGI will not support keepalive even it serves a
  HTTP 1.1 request, except explicitly configure uWSGI with a
  ``--http-keepalive`` option.

  Servers designed like uWSGI could cause aiohttp intermittently
  raise a ConnectionResetException when the protocol poll runs
  out and some protocol is reused.
  `#4587 <https://github.com/aio-libs/aiohttp/issues/4587>`_
- Handle the last CRLF correctly even if it is received via separate TCP segment.
  `#4630 <https://github.com/aio-libs/aiohttp/issues/4630>`_
- Fix the register_resource function to validate route name before splitting it so that route name can include python keywords.
  `#4691 <https://github.com/aio-libs/aiohttp/issues/4691>`_
- Improve typing annotations for ``web.Request``, ``aiohttp.ClientResponse`` and
  ``multipart`` module.
  `#4736 <https://github.com/aio-libs/aiohttp/issues/4736>`_
- Fix resolver task is not awaited when connector is cancelled
  `#4795 <https://github.com/aio-libs/aiohttp/issues/4795>`_
- Fix a bug "Aiohttp doesn't return any error on invalid request methods"
  `#4798 <https://github.com/aio-libs/aiohttp/issues/4798>`_
- Fix HEAD requests for static content.
  `#4809 <https://github.com/aio-libs/aiohttp/issues/4809>`_
- Fix incorrect size calculation for memoryview
  `#4890 <https://github.com/aio-libs/aiohttp/issues/4890>`_
- Add HTTPMove to _all__.
  `#4897 <https://github.com/aio-libs/aiohttp/issues/4897>`_
- Fixed the type annotations in the ``tracing`` module.
  `#4912 <https://github.com/aio-libs/aiohttp/issues/4912>`_
- Fix typing for multipart ``__aiter__``.
  `#4931 <https://github.com/aio-libs/aiohttp/issues/4931>`_
- Fix for race condition on connections in BaseConnector that leads to exceeding the connection limit.
  `#4936 <https://github.com/aio-libs/aiohttp/issues/4936>`_
- Add forced UTF-8 encoding for ``application/rdap+json`` responses.
  `#4938 <https://github.com/aio-libs/aiohttp/issues/4938>`_
- Fix inconsistency between Python and C http request parsers in parsing pct-encoded URL.
  `#4972 <https://github.com/aio-libs/aiohttp/issues/4972>`_
- Fix connection closing issue in HEAD request.
  `#5012 <https://github.com/aio-libs/aiohttp/issues/5012>`_
- Fix type hint on BaseRunner.addresses (from ``List[str]`` to ``List[Any]``)
  `#5086 <https://github.com/aio-libs/aiohttp/issues/5086>`_
- Make `web.run_app()` more responsive to Ctrl+C on Windows for Python < 3.8. It slightly
  increases CPU load as a side effect.
  `#5098 <https://github.com/aio-libs/aiohttp/issues/5098>`_


Improved Documentation
----------------------

- Fix example code in client quick-start
  `#3376 <https://github.com/aio-libs/aiohttp/issues/3376>`_
- Updated the docs so there is no contradiction in ``ttl_dns_cache`` default value
  `#3512 <https://github.com/aio-libs/aiohttp/issues/3512>`_
- Add 'Deploy with SSL' to docs.
  `#4201 <https://github.com/aio-libs/aiohttp/issues/4201>`_
- Change typing of the secure argument on StreamResponse.set_cookie from ``Optional[str]`` to ``Optional[bool]``
  `#4204 <https://github.com/aio-libs/aiohttp/issues/4204>`_
- Changes ``ttl_dns_cache`` type from int to Optional[int].
  `#4270 <https://github.com/aio-libs/aiohttp/issues/4270>`_
- Simplify README hello word example and add a documentation page for people coming from requests.
  `#4272 <https://github.com/aio-libs/aiohttp/issues/4272>`_
- Improve some code examples in the documentation involving websockets and starting a simple HTTP site with an AppRunner.
  `#4285 <https://github.com/aio-libs/aiohttp/issues/4285>`_
- Fix typo in code example in Multipart docs
  `#4312 <https://github.com/aio-libs/aiohttp/issues/4312>`_
- Fix code example in Multipart section.
  `#4314 <https://github.com/aio-libs/aiohttp/issues/4314>`_
- Update contributing guide so new contributors read the most recent version of that guide. Update command used to create test coverage reporting.
  `#4810 <https://github.com/aio-libs/aiohttp/issues/4810>`_
- Spelling: Change "canonize" to "canonicalize".
  `#4986 <https://github.com/aio-libs/aiohttp/issues/4986>`_
- Add ``aiohttp-sse-client`` library to third party usage list.
  `#5084 <https://github.com/aio-libs/aiohttp/issues/5084>`_


Misc
----

- `#2856 <https://github.com/aio-libs/aiohttp/issues/2856>`_, `#4218 <https://github.com/aio-libs/aiohttp/issues/4218>`_, `#4250 <https://github.com/aio-libs/aiohttp/issues/4250>`_
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

3 participants