From e0cddac459dbbbec20593959ce2030d704c825c9 Mon Sep 17 00:00:00 2001 From: Irfanuddin Date: Tue, 8 Nov 2022 00:56:09 +0530 Subject: [PATCH 1/7] feat: validate if route already exists --- fastapi/exceptions.py | 7 +++++++ fastapi/routing.py | 11 ++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/fastapi/exceptions.py b/fastapi/exceptions.py index 0f50acc6c..e38f56a8d 100644 --- a/fastapi/exceptions.py +++ b/fastapi/exceptions.py @@ -34,3 +34,10 @@ class RequestValidationError(ValidationError): class WebSocketRequestValidationError(ValidationError): def __init__(self, errors: Sequence[ErrorList]) -> None: super().__init__(errors, WebSocketErrorModel) + + +class RouteAlreadyExistsError(FastAPIError): + def __init__(self, f_name: str): + self.f_name = f_name + self.message = f"Route defined for {f_name} already exists!" + super().__init__(self.message) diff --git a/fastapi/routing.py b/fastapi/routing.py index 8c0bec5e6..cd5e3a0f3 100644 --- a/fastapi/routing.py +++ b/fastapi/routing.py @@ -29,7 +29,11 @@ from fastapi.dependencies.utils import ( solve_dependencies, ) from fastapi.encoders import DictIntStrAny, SetIntStr, jsonable_encoder -from fastapi.exceptions import RequestValidationError, WebSocketRequestValidationError +from fastapi.exceptions import ( + RequestValidationError, + RouteAlreadyExistsError, + WebSocketRequestValidationError, +) from fastapi.types import DecoratedCallable from fastapi.utils import ( create_cloned_field, @@ -442,6 +446,7 @@ class APIRoute(routing.Route): get_parameterless_sub_dependant(depends=depends, path=self.path_format), ) self.body_field = get_body_field(dependant=self.dependant, name=self.unique_id) + self.hash_val = f"path:{self.path};methods:{self.methods}" self.app = request_response(self.get_route_handler()) def get_route_handler(self) -> Callable[[Request], Coroutine[Any, Any, Response]]: @@ -513,6 +518,7 @@ class APIRouter(routing.Router): self.route_class = route_class self.default_response_class = default_response_class self.generate_unique_id_function = generate_unique_id_function + self.added_routes = set() def add_api_route( self, @@ -594,6 +600,9 @@ class APIRouter(routing.Router): openapi_extra=openapi_extra, generate_unique_id_function=current_generate_unique_id, ) + if route.hash_val in self.added_routes: + raise RouteAlreadyExistsError(route.name) + self.added_routes.add(route.hash_val) self.routes.append(route) def api_route( From 3cb2eab917ae509f341c81ac560bca3849f8f020 Mon Sep 17 00:00:00 2001 From: Irfanuddin Date: Tue, 8 Nov 2022 01:04:23 +0530 Subject: [PATCH 2/7] fix: linting --- fastapi/routing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fastapi/routing.py b/fastapi/routing.py index cd5e3a0f3..06c0a7342 100644 --- a/fastapi/routing.py +++ b/fastapi/routing.py @@ -518,7 +518,7 @@ class APIRouter(routing.Router): self.route_class = route_class self.default_response_class = default_response_class self.generate_unique_id_function = generate_unique_id_function - self.added_routes = set() + self.added_routes: Set[str] = set() def add_api_route( self, From 20661e1301a41c163f1ed585758f2742ee7ff5c2 Mon Sep 17 00:00:00 2001 From: Irfanuddin Date: Tue, 8 Nov 2022 01:55:39 +0530 Subject: [PATCH 3/7] tests: added tests --- fastapi/dependencies/utils.py | 6 +- fastapi/routing.py | 7 +- tests/test_include_duplicate_path_route.py | 121 +++++++++++++++++++++ 3 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 tests/test_include_duplicate_path_route.py diff --git a/fastapi/dependencies/utils.py b/fastapi/dependencies/utils.py index 64a6c1276..d35aa1b36 100644 --- a/fastapi/dependencies/utils.py +++ b/fastapi/dependencies/utils.py @@ -12,6 +12,7 @@ from typing import ( Mapping, Optional, Sequence, + Set, Tuple, Type, Union, @@ -74,7 +75,6 @@ sequence_shape_to_type = { SHAPE_TUPLE_ELLIPSIS: list, } - multipart_not_installed_error = ( 'Form data requires "python-multipart" to be installed. \n' 'You can install "python-multipart" with: \n\n' @@ -755,3 +755,7 @@ def get_body_field(*, dependant: Dependant, name: str) -> Optional[ModelField]: ) check_file_field(final_field) return final_field + + +def get_path_hash_val(path: str, methods: Optional[Set[str]]) -> str: + return f"path:{path};methods:{methods}" diff --git a/fastapi/routing.py b/fastapi/routing.py index 06c0a7342..a923cd29f 100644 --- a/fastapi/routing.py +++ b/fastapi/routing.py @@ -26,6 +26,7 @@ from fastapi.dependencies.utils import ( get_body_field, get_dependant, get_parameterless_sub_dependant, + get_path_hash_val, solve_dependencies, ) from fastapi.encoders import DictIntStrAny, SetIntStr, jsonable_encoder @@ -446,7 +447,7 @@ class APIRoute(routing.Route): get_parameterless_sub_dependant(depends=depends, path=self.path_format), ) self.body_field = get_body_field(dependant=self.dependant, name=self.unique_id) - self.hash_val = f"path:{self.path};methods:{self.methods}" + self.hash_val = get_path_hash_val(self.path, self.methods) self.app = request_response(self.get_route_handler()) def get_route_handler(self) -> Callable[[Request], Coroutine[Any, Any, Response]]: @@ -780,6 +781,10 @@ class APIRouter(routing.Router): ) elif isinstance(route, routing.Route): methods = list(route.methods or []) + hash_val = get_path_hash_val(route.path, route.methods) + if hash_val in self.added_routes: + raise RouteAlreadyExistsError(route.name) + self.added_routes.add(hash_val) self.add_route( prefix + route.path, route.endpoint, diff --git a/tests/test_include_duplicate_path_route.py b/tests/test_include_duplicate_path_route.py new file mode 100644 index 000000000..ca191ffdd --- /dev/null +++ b/tests/test_include_duplicate_path_route.py @@ -0,0 +1,121 @@ +import pytest +from fastapi import APIRouter, FastAPI +from fastapi.exceptions import RouteAlreadyExistsError + + +def test_app_router_with_duplicate_path(): + with pytest.raises(RouteAlreadyExistsError): + app = FastAPI() + + @app.get("/items/") + def read_items(): + return + + @app.get("/items/") + def read_items2(): + return + + +def test_sub_with_duplicate_path(): + with pytest.raises(RouteAlreadyExistsError): + app = FastAPI() + router = APIRouter() + + @router.get("/items/") + def read_items(): + return + + @router.get("/items/") + def read_items2(): + return + + app.include_router(router) + + +def test_mix_app_sub_with_duplicate_path(): + with pytest.raises(RouteAlreadyExistsError): + app = FastAPI() + router = APIRouter() + + @app.get("/items/") + def read_items(): + return + + @router.get("/items/") + def read_items2(): + return + + app.include_router(router) + + +def test_sub_route_direct_duplicate_path(): + with pytest.raises(RouteAlreadyExistsError): + app = FastAPI() + router = APIRouter() + + @router.route("/items/") + def read_items(): + return + + @router.route("/items/") + def read_items2(): + return + + app.include_router(router) + + +def test_app_router_with_duplicate_path_different_method(): + app = FastAPI() + + @app.get("/items/") + def read_items(): + return + + @app.post("/items/") + def read_items2(): + return + + +def test_sub_with_duplicate_path_different_method(): + app = FastAPI() + router = APIRouter() + + @router.get("/items/") + def read_items(): + return + + @router.post("/items/") + def read_items2(): + return + + app.include_router(router) + + +def test_mix_app_sub_with_duplicate_different_method(): + app = FastAPI() + router = APIRouter() + + @app.get("/items/") + def read_items(): + return + + @router.post("/items/") + def read_items2(): + return + + app.include_router(router) + + +def test_sub_route_direct_duplicate_path_different_method(): + app = FastAPI() + router = APIRouter() + + @router.route("/items/") + def read_items(): + return + + @router.route("/items/", methods=["POST"]) + def read_items2(): + return + + app.include_router(router) From d11ed8208b7aab5f38154313f168e7f182c37528 Mon Sep 17 00:00:00 2001 From: Irfanuddin Date: Sat, 12 Nov 2022 01:00:26 +0530 Subject: [PATCH 4/7] feat: add check in websocket routes and support prefix for Starlette Routers --- fastapi/dependencies/utils.py | 3 ++- fastapi/routing.py | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/fastapi/dependencies/utils.py b/fastapi/dependencies/utils.py index d35aa1b36..1644c59e0 100644 --- a/fastapi/dependencies/utils.py +++ b/fastapi/dependencies/utils.py @@ -757,5 +757,6 @@ def get_body_field(*, dependant: Dependant, name: str) -> Optional[ModelField]: return final_field -def get_path_hash_val(path: str, methods: Optional[Set[str]]) -> str: +def get_path_hash_val(path: str, methods: Optional[Set[str]] = None) -> str: + methods = methods or {"GET"} return f"path:{path};methods:{methods}" diff --git a/fastapi/routing.py b/fastapi/routing.py index a923cd29f..58ee9adb5 100644 --- a/fastapi/routing.py +++ b/fastapi/routing.py @@ -677,6 +677,10 @@ class APIRouter(routing.Router): name=name, dependency_overrides_provider=self.dependency_overrides_provider, ) + hash_val = get_path_hash_val(route.path) + if hash_val in self.added_routes: + raise RouteAlreadyExistsError(route.name) + self.added_routes.add(hash_val) self.routes.append(route) def websocket( @@ -781,7 +785,7 @@ class APIRouter(routing.Router): ) elif isinstance(route, routing.Route): methods = list(route.methods or []) - hash_val = get_path_hash_val(route.path, route.methods) + hash_val = get_path_hash_val(prefix + route.path, route.methods) if hash_val in self.added_routes: raise RouteAlreadyExistsError(route.name) self.added_routes.add(hash_val) From c3b164a1cc9883305446facf8acacf30431e8586 Mon Sep 17 00:00:00 2001 From: Irfanuddin Date: Sat, 12 Nov 2022 01:01:23 +0530 Subject: [PATCH 5/7] tests: add test for websocket --- tests/test_include_duplicate_path_route.py | 74 +++++++++++++++------- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/tests/test_include_duplicate_path_route.py b/tests/test_include_duplicate_path_route.py index ca191ffdd..676133bc4 100644 --- a/tests/test_include_duplicate_path_route.py +++ b/tests/test_include_duplicate_path_route.py @@ -9,11 +9,11 @@ def test_app_router_with_duplicate_path(): @app.get("/items/") def read_items(): - return + return # pragma: no cover @app.get("/items/") def read_items2(): - return + return # pragma: no cover def test_sub_with_duplicate_path(): @@ -23,13 +23,13 @@ def test_sub_with_duplicate_path(): @router.get("/items/") def read_items(): - return + return # pragma: no cover @router.get("/items/") def read_items2(): - return + return # pragma: no cover - app.include_router(router) + app.include_router(router) # pragma: no cover def test_mix_app_sub_with_duplicate_path(): @@ -39,13 +39,13 @@ def test_mix_app_sub_with_duplicate_path(): @app.get("/items/") def read_items(): - return + return # pragma: no cover @router.get("/items/") def read_items2(): - return + return # pragma: no cover - app.include_router(router) + app.include_router(router) # pragma: no cover def test_sub_route_direct_duplicate_path(): @@ -55,13 +55,13 @@ def test_sub_route_direct_duplicate_path(): @router.route("/items/") def read_items(): - return + return # pragma: no cover @router.route("/items/") def read_items2(): - return + return # pragma: no cover - app.include_router(router) + app.include_router(router) # pragma: no cover def test_app_router_with_duplicate_path_different_method(): @@ -69,11 +69,11 @@ def test_app_router_with_duplicate_path_different_method(): @app.get("/items/") def read_items(): - return + return # pragma: no cover @app.post("/items/") def read_items2(): - return + return # pragma: no cover def test_sub_with_duplicate_path_different_method(): @@ -82,13 +82,13 @@ def test_sub_with_duplicate_path_different_method(): @router.get("/items/") def read_items(): - return + return # pragma: no cover @router.post("/items/") def read_items2(): - return + return # pragma: no cover - app.include_router(router) + app.include_router(router) # pragma: no cover def test_mix_app_sub_with_duplicate_different_method(): @@ -97,13 +97,13 @@ def test_mix_app_sub_with_duplicate_different_method(): @app.get("/items/") def read_items(): - return + return # pragma: no cover @router.post("/items/") def read_items2(): - return + return # pragma: no cover - app.include_router(router) + app.include_router(router) # pragma: no cover def test_sub_route_direct_duplicate_path_different_method(): @@ -112,10 +112,40 @@ def test_sub_route_direct_duplicate_path_different_method(): @router.route("/items/") def read_items(): - return + return # pragma: no cover @router.route("/items/", methods=["POST"]) def read_items2(): - return + return # pragma: no cover + + app.include_router(router) # pragma: no cover + + +def test_app_websocket_route_with_duplicate_path(): + with pytest.raises(RouteAlreadyExistsError): + app = FastAPI() + + @app.websocket("/items/") + def read_items(): + return # pragma: no cover + + @app.websocket("/items/") + def read_items2(): + return # pragma: no cover + + +def test_sub_with_duplicate_path_with_prefix(): + with pytest.raises(RouteAlreadyExistsError): + app = FastAPI() + router = APIRouter() + + @router.get("/items/") + def read_items(): + return # pragma: no cover + + @router.get("/items/") + def read_items2(): + return # pragma: no cover + + app.include_router(router, prefix="/prefix") # pragma: no cover - app.include_router(router) From 89b691e5fa76e2fe2e1ca2f7164e8d30743cc8bf Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 11 Nov 2022 19:31:48 +0000 Subject: [PATCH 6/7] =?UTF-8?q?=F0=9F=8E=A8=20[pre-commit.ci]=20Auto=20for?= =?UTF-8?q?mat=20from=20pre-commit.com=20hooks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/test_include_duplicate_path_route.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_include_duplicate_path_route.py b/tests/test_include_duplicate_path_route.py index 676133bc4..6404659d7 100644 --- a/tests/test_include_duplicate_path_route.py +++ b/tests/test_include_duplicate_path_route.py @@ -148,4 +148,3 @@ def test_sub_with_duplicate_path_with_prefix(): return # pragma: no cover app.include_router(router, prefix="/prefix") # pragma: no cover - From 8fbabc65505b6aa012a21fa8b279f683fb942bc0 Mon Sep 17 00:00:00 2001 From: Irfanuddin Shafi Ahmed Date: Sun, 9 Jul 2023 11:01:16 +0530 Subject: [PATCH 7/7] style: sort import statements --- fastapi/routing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fastapi/routing.py b/fastapi/routing.py index 7b6d9ed17..de2303341 100644 --- a/fastapi/routing.py +++ b/fastapi/routing.py @@ -42,8 +42,8 @@ from fastapi.encoders import jsonable_encoder from fastapi.exceptions import ( FastAPIError, RequestValidationError, - RouteAlreadyExistsError, ResponseValidationError, + RouteAlreadyExistsError, WebSocketRequestValidationError, ) from fastapi.types import DecoratedCallable, IncEx