fastapi icon indicating copy to clipboard operation
fastapi copied to clipboard

307 on a delete for a post request.

Open julian-r opened this issue 4 years ago • 6 comments

Describe the bug

When a delete is made on a post route with a trailing slash the server returns a 307 temporary redirect. I would either expect a 404 not found or a 405 method not allowed.

To Reproduce

Just a unittest to show the case:

from fastapi import FastAPI, Request, Response
from fastapi.testclient import TestClient

app = FastAPI()


@app.post("/bla")
async def bla():
    return {}


client = TestClient(app)


def test_read_main():
    response = client.delete("/bla/")
    print(response.headers)
    assert response.status_code == 405

Environment

  • OS: all
  • FastAPI Version: 0.53.2
  • Python Version: 3.8.2

julian-r avatar Apr 03 '20 08:04 julian-r

Your path has some error. you should use :response = client.delete("/bla")

Dustyposa avatar Apr 03 '20 14:04 Dustyposa

Interesting. This behavior is actually controlled by starlette.router.Router.redirect_slashes, so you might want to give it a try.

However on my end during the TestClient test that property is not kept, which might be a bug:

from fastapi import FastAPI, APIRouter
from starlette.testclient import TestClient

app = FastAPI()
router = APIRouter(redirect_slashes=False)

@router.post("/bla")
async def bla():
    return {}

app.include_router(router)

client = TestClient(app)


def test_read_main():
    assert router.redirect_slashes == False # success
    assert client.app.router.redirect_slashes == False #fail
    response = client.post("/bla/")
    print(response.headers)
    assert response.status_code >= 400

@Dustyposa I think they just want that path without slashes not getting redirected. It's intended.

phy25 avatar Apr 03 '20 17:04 phy25

Yep, this seems to be a bug but I can't figure out where it is coming from. The redirect_slashes=False is definitely being ignored on my end.

Starlette is only supposed to be redirecting slashes if redirect_slashes is True (which it is by default), but if you set it to False in a fastapi.APIRouter, FastAPI is in fact passing that kwarg down to starlette.routing.Router, which in turn appears to only redirect if that param is true.

Very strange.

acnebs avatar Apr 04 '20 11:04 acnebs

Just had a look into this. The main FastAPI instance has an APIRouter.

When you create separate fastapi.routing.APIRouter instances and then use app.include_router(...), behind the scenes it is actually appending the routes from this router to FastAPI#router.

This should work:

app = FastAPI()
app.router.redirect_slashes = False

Seems that redirect_slashes means "redirect paths without trailing slash to slash", NOT "redirect trailing slash to no slash".

starlette/routing.py:601

        if scope["type"] == "http" and self.redirect_slashes:
            if not scope["path"].endswith("/"):
                redirect_scope = dict(scope)
                redirect_scope["path"] += "/"

Also, if you are using a FastAPI or Flask backend via a proxy (like in CRA setupProxy.js with http-proxy-middleware), then you have some additional work to do as described here: https://github.com/chimurai/http-proxy-middleware/issues/140#issuecomment-611237707

vjpr avatar Apr 08 '20 22:04 vjpr

Now the question should be, what is default behavior and what is HTTP standard? Responding with a redirect to an other verb then GET feels wired for me, but idk if this is just subjective...

julian-r avatar Apr 23 '20 12:04 julian-r

I haven't seen anything about what should be redirected and what not, I imagine there's no standard for automatic slash redirects, it's mostly a "feature" of frameworks. In your case, I imagine the best would be to update the test to use the exact path. And if you don't want automatic redirects, I guess you could disable them and that would make things more strict.

But anyway, were you able to solve your problem? If so, you could close this issue. 🤓

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.

tiangolo avatar Nov 04 '22 19:11 tiangolo

Assuming the original need was handled, this will be automatically closed now. But feel free to add more comments or create new issues or PRs.

github-actions[bot] avatar Nov 15 '22 00:11 github-actions[bot]