Skip to content

Fix StaticResource routes not resolving #12255

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.

Sign up for GitHub

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

Open
BlindChickens wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix StaticResource routes not resolving #12255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b832fb
Fix StaticResource routes not resolving
BlindChickens Mar 18, 2026
06c1886
Fix docs
BlindChickens Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGES/12255.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fixed :py:class:`~aiohttp.web.StaticResource` not resolving urls when adding them via the internal ``_routes`` property.
You had to also add the method to the ``_allowed_methods`` property, which is not ideal.

Added :py:meth:`~aiohttp.web.AbstractResource.add_route` to :py:class:`~aiohttp.web.AbstractResource`.
-- by :user:`BlindChickens`.
5 changes: 5 additions & 0 deletions CHANGES/12255.feature.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fixed :py:class:`~aiohttp.web.StaticResource` not resolving urls when adding them via the internal ``_routes`` property.
You had to also add the method to the ``_allowed_methods`` property, which is not ideal.

Added :py:meth:`~aiohttp.web.AbstractResource.add_route` to :py:class:`~aiohttp.web.AbstractResource`.
-- by :user:`BlindChickens`.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ Ivan Larin
J. Nick Koston
Jacob Champion
Jacob Henner
Jacques Combrink
Jaesung Lee
Jake Davis
Jakob Ackermann
Expand Down
67 changes: 31 additions & 36 deletions aiohttp/web_urldispatcher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ class _InfoDict(TypedDict, total=False):
class AbstractResource(Sized, Iterable["AbstractRoute"]):
def __init__(self, *, name: str | None = None) -> None:
self._name = name
self._routes: dict[str, ResourceRoute] = {}
self._any_route: ResourceRoute | None = None
self._allowed_methods: set[str] = set()

@property
def name(self) -> str | None:
Expand All @@ -119,6 +122,33 @@ def canonical(self) -> str:
def url_for(self, **kwargs: str) -> URL:
"""Construct url for resource with additional params."""

def add_route(
self,
method: str,
handler: type[AbstractView] | Handler,
*,
expect_handler: _ExpectHandler | None = None,
) -> "ResourceRoute":
if route := self._routes.get(method, self._any_route):
raise RuntimeError(
"Added route will never be executed, "
f"method {route.method} is already "
"registered"
)

route_obj = ResourceRoute(method, handler, self, expect_handler=expect_handler)
self.register_route(route_obj)
return route_obj

def register_route(self, route: "ResourceRoute") -> None:
assert isinstance(
route, ResourceRoute
), f"Instance of Route class is required, got {route!r}"
if route.method == hdrs.METH_ANY:
self._any_route = route
self._allowed_methods.add(route.method)
self._routes[route.method] = route

@abc.abstractmethod # pragma: no branch
async def resolve(self, request: Request) -> _Resolve:
"""Resolve resource.
Expand Down Expand Up @@ -308,36 +338,6 @@ async def _default_expect_handler(request: Request) -> None:
class Resource(AbstractResource):
def __init__(self, *, name: str | None = None) -> None:
super().__init__(name=name)
self._routes: dict[str, ResourceRoute] = {}
self._any_route: ResourceRoute | None = None
self._allowed_methods: set[str] = set()

def add_route(
self,
method: str,
handler: type[AbstractView] | Handler,
*,
expect_handler: _ExpectHandler | None = None,
) -> "ResourceRoute":
if route := self._routes.get(method, self._any_route):
raise RuntimeError(
"Added route will never be executed, "
f"method {route.method} is already "
"registered"
)

route_obj = ResourceRoute(method, handler, self, expect_handler=expect_handler)
self.register_route(route_obj)
return route_obj

def register_route(self, route: "ResourceRoute") -> None:
assert isinstance(
route, ResourceRoute
), f"Instance of Route class is required, got {route!r}"
if route.method == hdrs.METH_ANY:
self._any_route = route
self._allowed_methods.add(route.method)
self._routes[route.method] = route

async def resolve(self, request: Request) -> _Resolve:
if (match_dict := self._match(request.rel_url.path_safe)) is None:
Expand Down Expand Up @@ -589,12 +589,7 @@ def get_info(self) -> _InfoDict:
}

def set_options_route(self, handler: Handler) -> None:
if "OPTIONS" in self._routes:
raise RuntimeError("OPTIONS route was set already")
self._routes["OPTIONS"] = ResourceRoute(
"OPTIONS", handler, self, expect_handler=self._expect_handler
)
self._allowed_methods.add("OPTIONS")
self.add_route("OPTIONS", handler)

async def resolve(self, request: Request) -> _Resolve:
path = request.rel_url.path_safe
Expand Down
56 changes: 33 additions & 23 deletions docs/web_reference.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2052,7 +2052,7 @@ second one.
User should never instantiate resource classes but give it by
:meth:`UrlDispatcher.add_resource` call.

After that he may add a :term:`route` by calling :meth:`Resource.add_route`.
After that he may add a :term:`route` by calling :meth:`AbstractResource.add_route`.

:meth:`UrlDispatcher.add_route` is just shortcut for::

Expand Down Expand Up @@ -2098,6 +2098,26 @@ Resource classes hierarchy::

.. versionadded:: 3.3

.. method:: add_route(method, handler, *, expect_handler=None)

Add a :term:`web-handler` to resource.

:param str method: HTTP method for route. Should be one of
``'GET'``, ``'POST'``, ``'PUT'``,
``'DELETE'``, ``'PATCH'``, ``'HEAD'``,
``'OPTIONS'`` or ``'*'`` for any method.

The parameter is case-insensitive, e.g. you
can push ``'get'`` as well as ``'GET'``.

The method should be unique for resource.

:param collections.abc.Callable handler: route handler.

:param collections.abc.Coroutine expect_handler: optional *expect* header handler.

:returns: new :class:`ResourceRoute` instance.

.. method:: resolve(request)
:async:

Expand Down Expand Up @@ -2135,27 +2155,6 @@ Resource classes hierarchy::
A base class for new-style resources, inherits :class:`AbstractResource`.


.. method:: add_route(method, handler, *, expect_handler=None)

Add a :term:`web-handler` to resource.

:param str method: HTTP method for route. Should be one of
``'GET'``, ``'POST'``, ``'PUT'``,
``'DELETE'``, ``'PATCH'``, ``'HEAD'``,
``'OPTIONS'`` or ``'*'`` for any method.

The parameter is case-insensitive, e.g. you
can push ``'get'`` as well as ``'GET'``.

The method should be unique for resource.

:param collections.abc.Callable handler: route handler.

:param collections.abc.Coroutine expect_handler: optional *expect* header handler.

:returns: new :class:`ResourceRoute` instance.


.. class:: PlainResource
:canonical: aiohttp.web_urldispatcher.PlainResource

Expand Down Expand Up @@ -2203,10 +2202,21 @@ Resource classes hierarchy::
be called as ``resource.url_for(to='val1', param='val2')``


.. class:: PrefixResource
:canonical: aiohttp.web_urldispatcher.PrefixResource

A resource, inherited from :class:`AbstractResource`.

.. attribute:: canonical

Read-only *canonical path* associate with the resource. Returns the prefix
used to create the PrefixResource. For example ``/prefix``


.. class:: StaticResource
:canonical: aiohttp.web_urldispatcher.StaticResource

A resource, inherited from :class:`Resource`.
A resource, inherited from :class:`PrefixResource`.

The class corresponds to resources for :ref:`static file serving
<aiohttp-web-static-file-handling>`.
Expand Down
13 changes: 13 additions & 0 deletions tests/test_urldispatch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,19 @@ async def test_add_static_access_resources(router: web.UrlDispatcher) -> None:
assert allowed_methods == {hdrs.METH_GET, hdrs.METH_OPTIONS, hdrs.METH_HEAD}


async def test_add_static_access_resources_method(router: web.UrlDispatcher) -> None:
"""Test adding a route via add_route to static resource."""
resource = router.add_static(
"/st", pathlib.Path(aiohttp.__file__).parent, name="static"
)
resource.add_route(hdrs.METH_POST, make_handler())
mapping, allowed_methods = await resource.resolve(
make_mocked_request("POST", "/st/path")
Comment on lines +556 to +561
Copy link
Copy Markdown
Member

@Dreamsorcerer Dreamsorcerer Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what is happening here?

It looks to me like you're trying to add a POST handler to a static resource. I can't understand how that makes any sense.

Can you please start by describing the issue you are supposedly fixing? Or the use case of this? As the documentation says, .add_static() is meant to emulate something similar to nginx static file handling, for use in local development only. Adding a POST handler makes no sense to me.

Copy link
Copy Markdown
Author

@BlindChickens BlindChickens Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what you are saying. The POST in the test case would be confusing. But there are other methods, so I can change it to something else, if that would help.

But to me it seems that you feel this cannot be useful because of this: .add_static() is meant to emulate something similar to nginx static file handling.

So if that's the case, then there is no use keeping this open.

If not, if I change the test case to add any other method handler through the new interface, would it be acceptable?

Copy link
Copy Markdown
Member

@Dreamsorcerer Dreamsorcerer Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not, if I change the test case to add any other method handler through the new interface, would it be acceptable?

I just don't understand what you're trying to achieve... A StaticResource should resolve to a file on the filesystem, why would you want to try adding a custom handler to it? It seems to me like you should be using a regular PlainResource through the router.add_post() etc. methods (https://docs.aiohttp.org/en/stable/web_reference.html#aiohttp.web.UrlDispatcher.add_routes).

Copy link
Copy Markdown
Author

@BlindChickens BlindChickens Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I did this purely because we had a regression when updating past #9911, and when I investigated and saw that it also regressed something done in aiohttp-cors as indicated in the PR description of #9976.

aiohttp-cors accesses the protected self._routes here and relies on being able to add new methods

So I was like, oh, they also want to add methods, let's fix that.

Our stuff that broke is obviously legacy shenanigans and we need to change it. But for now I just added an extra line: self._allowed_methods.add("POST"), because you know stuff needs to keep moving.

Copy link
Copy Markdown
Member

@Dreamsorcerer Dreamsorcerer Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was like, oh, they also want to add methods, let's fix that.

OK, I understand now. I think we probably want to reevaluate the aiohttp-cors integration before deciding that we want to add this to the public API. So, we'll probably avoid merging this PR currently until we have time to evaluate that properly.

)
assert mapping is not None
assert allowed_methods == {hdrs.METH_GET, hdrs.METH_POST, hdrs.METH_HEAD}


async def test_add_static_set_options_route(router: web.UrlDispatcher) -> None:
"""Ensure set_options_route works as expected."""
resource = router.add_static(
Expand Down
Loading