Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

How to Prevent the 307 Temporary Redirect When There's a Missing Trailing Slash #2060

Closed
Reapor-Yurnero opened this issue Sep 16, 2020 · 21 comments
Labels
question Question or problem question-migrate

Comments

@Reapor-Yurnero
Copy link

Description

Just like the author of #731, I don't want a 307 temporary redirect which is automatically sent by uvicorn when there's a missing trailing slash in the api call. However, the solution given in that issue, i.e. route path like "/?" no longer works in the versions after this April as reported in in #1787, #1648 and else. Certain developers states this is an unexpected behavior and won't be supported in the future. In this case, I'm wondering what is the current elegant way to realize this. Or there's any way to handle both "" and "/" two paths simultaneously?

@Reapor-Yurnero Reapor-Yurnero added the question Question or problem label Sep 16, 2020
@falkben
Copy link
Contributor

falkben commented Sep 29, 2020

You can have multiple decorators with path routes w/ and w/o the trailing slash. Not incredibly elegant because then you get duplicate endpoints in your swagger docs.

It should be mentioned this is a Starlette issue. There are several issues about this in the repo, here is one of them: encode/starlette#1008

If FastAPI could handle this, it might be to somehow identify and remove the duplicate entries in swagger docs.

@luebke-dev
Copy link

@falkben just use include_in_schema=False on one decorator.

@Reapor-Yurnero
Copy link
Author

Yours answers together is a very good workaround!

@nikhilshinday
Copy link

nikhilshinday commented Jan 29, 2021

Hello! To extend the responses of @SebastianLuebke and @falkben, I think I have a good solution that minimizes the verbosity of doing double annotations.

Effectively, the following code just wraps an endpoint in two calls to the router.

class RouterWrapper:
    def __init__(self, _router: APIRouter):
        self.router = _router
        self.http_verbs = [
            "get",
            "post",
            "put",
            "patch",
            "delete",
            "head",
            "trace",
            "websocket",
        ]

    def __getattr__(self, verb):
        if verb in self.http_verbs:

            def _outer_callable(*router_args, **router_kwargs):

                if len(router_args) == 1:
                    (original_path,) = router_args
                elif "path" in router_kwargs:
                    original_path = router_kwargs.get("path")
                    router_kwargs.pop("path")
                else:
                    raise ValueError("path argument isn't filled in")

                if original_path.endswith("/"):
                    alternate_path = original_path[:-1]
                else:
                    alternate_path = f"{original_path}/"

                def _inner_callable(endpoint_func: Callable):
                    router_annotation = getattr(self.router, verb)
                    router_annotation(
                        alternate_path, include_in_schema=False, **router_kwargs
                    )(endpoint_func)
                    return router_annotation(original_path, **router_kwargs)(
                        endpoint_func
                    )

                return _inner_callable

            return _outer_callable

Usage:

router = APIRouter()
wrapper = RouterWrapper(router)


@wrapper.get('/', response_model=SomePydanticModel)
def endpoint(
    user: User = Depends(get_current_active_user),
    *,
    params: ListParams = Depends()
):
    ...

is equivalent to:

router = APIRouter()


@router.get('', response_model=SomePydanticModel, include_in_schema=False)
@router.get('/', response_model=SomePydanticModel)
def endpoint(
    user: User = Depends(get_current_active_user),
    *,
    params: ListParams = Depends()
):
    ...

I know this obfuscates the usage of the router, but I think it makes larger projects easier to handle.

I also know that this is a frequently encountered problem based on reading the issues around it, so cc @tiangolo in case anyone else is grumbling about the redirect behavior, this seems like a reasonable shim for now.

@malthunayan
Copy link

malthunayan commented Apr 27, 2021

Just wanted to share a similar solution to @nikhilshinday here:

from typing import Any, Callable

from fastapi import APIRouter as FastAPIRouter
from fastapi.types import DecoratedCallable


class APIRouter(FastAPIRouter):
    def api_route(
        self, path: str, *, include_in_schema: bool = True, **kwargs: Any
    ) -> Callable[[DecoratedCallable], DecoratedCallable]:
        if path.endswith("/"):
            path = path[:-1]

        alternate_path = path + "/"
        super().api_route(alternate_path, include_in_schema=False, **kwargs)
        return super().api_route(
            path, include_in_schema=include_in_schema, **kwargs
        )

Usage:

from module.with.custom_class import APIRouter

router = APIRouter()


@router.get("/", response_model=Foo)
def endpoint(bar: Bar = Depends()):
    ...


@router.post("", response_model=Spam)
def another_endpoint(ham: Ham = Depends()):
    ....

Which translates to:

from fastapi import APIRouter

router = APIRouter()


@router.get("/", response_model=Foo, include_in_schema=False)
@router.get("", response_model=Foo)
def endpoint(bar: Bar = Depends()):
    ...


@router.post("/", response_model=Spam, include_in_schema=False)
@router.post("", response_model=Spam)
def another_endpoint(ham: Ham = Depends()):
    ....

This will consistently display no trailing slashes in the docs, but it will also handle cases were the originally decorated function has included_in_schema as False. For example:

from some.module import APIRouter

router = APIRouter()


@router.get("/", response_model=Foo, include_in_schema=False)
def endpoint(bar: Bar = Depends()):
    ...

Would translate to:

from fastapi import APIRouter

router = APIRouter()

@router.get("", response_model=Foo, include_in_schema=False)
@router.get("/", response_model=Foo, include_in_schema=False)
def endpoint(bar: Bar = Depends()):
    ...

Edit: the implementation above has a bug, read on below for working implementations.

@hjoukl
Copy link

hjoukl commented May 7, 2021

Thanks @malthunayan for sharing this, you set me in the right direction.

However, the proposed solution doesn't quite work imho because the inner decorator function (https://github.com/tiangolo/fastapi/blob/c646eaa6bb1886dc64ba6281184e76c4dcb1c044/fastapi/routing.py#L550) of api_route(...) is actually never called. Thus, no route is added for the alternate_path.

Tricky thing is that "307 Temporary Redirect" is still in place - so you'd get answers even without the alternate routes in place - unless you set

router = APIRouter(redirect_slashes=False)
app.include_router(router)

and patch the default app router:

app.router.redirect_slashes = False 

(don't know why this is necessary in addition - all my routes are placed on router, not the app)

To make this recipe work you could do this instead:

from typing import Any, Callable
         
from fastapi import APIRouter as FastAPIRouter
from fastapi.types import DecoratedCallable
         
      
class APIRouter(FastAPIRouter):
    def add_api_route(
            self, path: str, endpoint: Callable[..., Any], *,
            include_in_schema: bool = True, **kwargs: Any
            ) -> None:
        if path.endswith("/"):
            alternate_path = path[:-1]
        else:           
            alternate_path = path + "/"
        super().add_api_route(                                                 
            alternate_path, endpoint, include_in_schema=False, **kwargs)
        return super().add_api_route(
            path, endpoint, include_in_schema=include_in_schema, **kwargs)

I. e. override FastAPIRouter.add_api_route(), not api_route().

(EDIT: Fixed add_api_route() return value type annotation to properly match the original base class method)

Note that I slightly modified the path/alternate_path logic so that the oas-documented version is always the one set as the explicit path, and an alternate_path is always added as a secondary route. Should be easily adaptable to your tastes.

Wow, it's trickier than I thought to make FastAPI work properly behind a HAProxy reverse proxy and path prefixes, x-forwarded-* headers...
Starlette's trailing-slashes redirect magic is a bit of a pain here as it doesn't seem to take these headers into account so you end up receiving a redirect with an (unreachable) backend URL.

Up to now everything FastAPI has been so pretty darn easy :-)

Best regards, Holger

@malthunayan
Copy link

Hey, @hjoukl,
Thanks for bringing that issue to my attention, I actually hadn't noticed the issue with my implementation.

The bug slipped through cause mainly I needed a way for all my paths to end without a trailing slash regardless of how it was given in the path decorator. I went ahead and made a hotfix to the implementation above, I've lightly tested it and it seems to be working without any issues:

from typing import Any, Callable

from fastapi import APIRouter as FastAPIRouter
from fastapi.types import DecoratedCallable


class APIRouter(FastAPIRouter):
    def api_route(
        self, path: str, *, include_in_schema: bool = True, **kwargs: Any
    ) -> Callable[[DecoratedCallable], DecoratedCallable]:
        if path.endswith("/"):
            path = path[:-1]

        add_path = super().api_route(
            path, include_in_schema=include_in_schema, **kwargs
        )

        alternate_path = path + "/"
        add_alternate_path = super().api_route(
            alternate_path, include_in_schema=False, **kwargs
        )

        def decorator(func: DecoratedCallable) -> DecoratedCallable:
            add_alternate_path(func)
            return add_path(func)

        return decorator

The reason why I have not chosen to override the add_api_route method was because that implementation seemed more nuanced. Also, it was being used by the include_router method, so I didn't wanna override it and have it cause weird behavior that would be difficult to track down. api_route seemed more isolated and simpler to override, which made a better candidate for tracking bugs down related to its overridden method.

@hjoukl
Copy link

hjoukl commented May 12, 2021

Hey @malthunayan, thanks for getting back - nice variant :-). Looks like this should do the trick.
And while looking at it I realized I got the return value type annotation wrong for the alternative add_api_route() solution - now corrected.

@BrandonEscamilla
Copy link

HI all, just wondering which one is the final solution?

@malthunayan
Copy link

Hello, @BrandonEscamilla,
Any of the last two solutions above work, choose whichever suits your needs best.

@socar-humphrey
Copy link

Hi folks,

Any plan for making this as one of features of APIRouter? (btw this thread helped me out of 2 wks long pain. you guys lit 👍)
It would be awesome to make it as a parameter option or another APIRouter implementation.

@pietdaniel
Copy link

Ran into this recently, would love to have this upstream. Perhaps configurable to keep compatibility.

@joeherm
Copy link

joeherm commented Oct 25, 2021

Also running into this and think it would be helpful to have upstream changes made.

@phillipuniverse
Copy link

phillipuniverse commented Nov 19, 2021

@malthunayan @hjoukl - thank you guys SO MUCH for this implementation. That worked almost perfectly for me.

The part that doesn't work is adding a / route:

router = RouterWithSlashes()

@router.get('/', include_in_schema=False)
def test():
    return {'hello': 'world'}

app.include_router(router)

This fails with the following exception on the app.include_router line:

Traceback (most recent call last):
  File "/Users/phillip/genesis/main.py", line 464, in <module>
    app.include_router(router)
  File "/Users/phillip/Library/Caches/pypoetry/virtualenvs/genesis-mBtHrm7W-py3.7/lib/python3.7/site-packages/fastapi/applications.py", line 359, in include_router
    callbacks=callbacks,
  File "/Users/phillip/Library/Caches/pypoetry/virtualenvs/genesis-mBtHrm7W-py3.7/lib/python3.7/site-packages/fastapi/routing.py", line 656, in include_router
    f"Prefix and path cannot be both empty (path operation: {name})"
Exception: Prefix and path cannot be both empty (path operation: test)

Here is my final implementation:

class HandleTrailingSlashRouter(APIRouter):
    """
    Registers endpoints for both a non-trailing-slash and a trailing slash. In regards to the exported API schema only the non-trailing slash will be included.

    Examples:

        @router.get("", include_in_schema=False) - not included in the OpenAPI schema, responds to both the naked url (no slash) and /

        @router.get("/some/path") - included in the OpenAPI schema as /some/path, responds to both /some/path and /some/path/

        @router.get("/some/path/") - included in the OpenAPI schema as /some/path, responds to both /some/path and /some/path/

    Co-opted from https://github.com/tiangolo/fastapi/issues/2060#issuecomment-974527690
    """

    def api_route(
            self, path: str, *, include_in_schema: bool = True, **kwargs: Any
    ) -> Callable[[DecoratedCallable], DecoratedCallable]:
        given_path = path
        path_no_slash = given_path[:-1] if given_path.endswith("/") else given_path

        add_nontrailing_slash_path = super().api_route(
            path_no_slash, include_in_schema=include_in_schema, **kwargs
        )

        add_trailing_slash_path = super().api_route(
            path_no_slash + "/", include_in_schema=False, **kwargs
        )

        def add_path_and_trailing_slash(func: DecoratedCallable) -> DecoratedCallable:
            add_trailing_slash_path(func)
            return add_nontrailing_slash_path(func)

        return add_trailing_slash_path if given_path == "/" else add_path_and_trailing_slash

@lucastonelli
Copy link

lucastonelli commented Apr 9, 2022

Hey, just for the record, to add another possible solution, I had the same problem and I solved it differently. I'm currently using the bit below to remove trailing slashes and avoid redirects:

for route in api_router.routes:
    if route.path.endswith("/"):
        route.path = route.path[:-1]

It is being used on the uppermost APIRouter, so it applies to every router on my application. It works like this:

"""Main app execution."""

from types import MethodType
from fastapi import FastAPI
from fastapi.exceptions import RequestValidationError


from src import api_router
from src.dependencies import custom_openapi
from src.dependencies import validation_exception_handler

app = FastAPI(redoc_url=None)

for route in api_router.routes:
    if route.path.endswith("/"):
        route.path = route.path[:-1]

app.include_router(api_router)

app.add_exception_handler(RequestValidationError, validation_exception_handler)

app.openapi = MethodType(custom_openapi, app)

Everything is working fine at the moment.

@ghost
Copy link

ghost commented Apr 16, 2022

Slightly different approach building on @lucastonelli

I prefer to prevent the application starting with trailing slashes - then there is no chance of me wondering later why I have trailing slashes that are ignored.

for route in app.routes:
    if route.path.endswith('/'):
        if route.path == '/':
            # trailing slash OK for root
            continue
        print(f'Aborting: paths may not end with a slash, route with problem is: {route.path}')
        sys.exit(1)

@d9i2r2t1
Copy link

@phillipuniverse @malthunayan thank you for sharing your solutions! They were very helpful to me.

But there is a small problem with this: when the path is /, it is not included in the Open API schema.

For example, I have a router: router = HandleTrailingSlashRouter(prefix ="/v1/products").
When I use a decorator like @router.post("/"), this route is also not included in the OpenAPI scheme.

I used your and @malthunayan solutions to fix this:

from typing import Any, Callable

from fastapi import APIRouter as FastAPIRouter
from fastapi.types import DecoratedCallable


class APIRouter(FastAPIRouter):
    def api_route(
        self, path: str, *, include_in_schema: bool = True, **kwargs: Any
    ) -> Callable[[DecoratedCallable], DecoratedCallable]:
        if path == "/":
            return super().api_route(
                path, include_in_schema=include_in_schema, **kwargs
            )

        if path.endswith("/"):
            path = path[:-1]
        add_path = super().api_route(
            path, include_in_schema=include_in_schema, **kwargs
        )

        alternate_path = path + "/"
        add_alternate_path = super().api_route(
            alternate_path, include_in_schema=False, **kwargs
        )

        def decorator(func: DecoratedCallable) -> DecoratedCallable:
            add_alternate_path(func)
            return add_path(func)

        return decorator

Now it works the way I want it to: it doesn't fail when the path is / and is also included in the Open API schema.

But you should keep in mind that if you want to use an empty path with a router prefix, you need to specify an empty path, not /:

router = APIRouter(prefix="/v1/products")

@router.post("")
async def products(request: Request):
   # ...

I hope this solution will be useful to someone :)

@rjnocelli
Copy link

rjnocelli commented Sep 6, 2022

Building on @malthunayan solution. Why not just evaluate the len of path?

from typing import Any, Callable

from fastapi import APIRouter as FastAPIRouter
from fastapi.types import DecoratedCallable


class APIRouter(FastAPIRouter):
    def api_route(
        self, path: str, *, include_in_schema: bool = True, **kwargs: Any
    ) -> Callable[[DecoratedCallable], DecoratedCallable]:
        if path.endswith("/") and len(path) > 1:
            path = path[:-1]

        add_path = super().api_route(
            path, include_in_schema=include_in_schema, **kwargs
        )

        alternate_path = path + "/"
        add_alternate_path = super().api_route(
            alternate_path, include_in_schema=False, **kwargs
        )

        def decorator(func: DecoratedCallable) -> DecoratedCallable:
            add_alternate_path(func)
            return add_path(func)

        return decorator

@papb
Copy link
Contributor

papb commented Oct 12, 2022

Related: #5491, #731

@tiangolo
Copy link
Member

Thanks for the help here everyone! 👏 🙇

Thanks for reporting back and closing the issue @Reapor-Yurnero 👍

Sorry for the long delay! 🙈 I wanted to personally address each issue/PR and they piled up through time, but now I'm checking each one in order.

@Qazzquimby
Copy link

I think when using subrouters with prefixes, you do want to affect a single "/" path.

fixed by changing len(path) to len(self.prefix+path)

if path.endswith("/") and len(self.prefix+path) > 1:
    path = path[:-1]

@fastapi fastapi locked and limited conversation to collaborators Feb 28, 2023
@tiangolo tiangolo converted this issue into discussion #7298 Feb 28, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question Question or problem question-migrate
Projects
None yet
Development

No branches or pull requests