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

Automatically support HEAD method for all GET routes, as Starlette does #1773

Open
9 tasks done
matthewlloyd opened this issue Jul 22, 2020 · 22 comments
Open
9 tasks done
Labels
confirmed feature New feature or request reviewed

Comments

@matthewlloyd
Copy link

First check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the FastAPI documentation, with the integrated search.
  • I already searched in Google "How to X in FastAPI" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to FastAPI but to Pydantic.
  • I already checked if it is not related to FastAPI but to Swagger UI.
  • I already checked if it is not related to FastAPI but to ReDoc.
  • After submitting this, I commit to:
    • Read open issues with questions until I find 2 issues where I can help someone and add a comment to help there.
    • Or, I already hit the "watch" button in this repository to receive notifications and I commit to help at least 2 people that ask questions in the future.
    • Implement a Pull Request for a confirmed bug.

Example

Here's a self-contained minimal, reproducible, example with my use case:

from fastapi import FastAPI
import pytest
from fastapi.testclient import TestClient

app = FastAPI()

@app.get("/")
def read_root():
    return {"Hello": "World"}

client = TestClient(app)

def test_index_head():
    response = client.head("/")
    assert response.status_code == 200

Description

  • The above test fails because the HEAD request returns a 405.
  • This is counterintuitive.

The solution you would like

  • To better support the HTTP standard, all routes that handle GET methods should automatically handle HEAD methods, too.
  • This is similar to what Starlette's router already does: Better support for HEAD method encode/starlette#45
  • This should happen without requiring the developer to do any additional work.

Describe alternatives you've considered

  • It's currently possible to do this manually by adding @app.head and a helper method for each route, but it's cumbersome:
@app.head("/")
def read_root_head():
    return Response()
  • It could also be done using middleware, but that would incur a performance overhead.

Environment

  • OS: [e.g. Linux / Windows / macOS]: Linux
  • FastAPI Version [e.g. 0.3.0]: 0.60.1
  • Python version: 3.8.5
@matthewlloyd matthewlloyd added the feature New feature or request label Jul 22, 2020
@phy25
Copy link

phy25 commented Jul 23, 2020

encode/starlette#45 is only about static FileResponse. I don't know how a default should be set for a dynamic API.

@matthewlloyd
Copy link
Author

matthewlloyd commented Jul 23, 2020

Not so, if you look at the related commit at https://github.com/encode/starlette/pull/132/files in starlette/routing.py, you can see Starlette adds HEAD methods automatically for every Route that supports GET, regardless of whether they are FileResponse or not.

Many web frameworks do this by default too, e.g. Flask (https://flask.palletsprojects.com/en/1.1.x/quickstart/#http-methods) and Django. It makes sense to do that because a server should respond to an HTTP HEAD request as defined in the spec, see e.g.:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/HEAD

"The HTTP HEAD method requests the headers that are returned if the specified resource would be requested with an HTTP GET method. Such a request can be done before deciding to download a large resource to save bandwidth, for example."

Clients use this for cache invalidation. The server just needs to run the route method, respond with the headers but discard the response body. Ideally, FastAPI would automatically add an OPTIONS method handler, too. This would help make FastAPI a first class web framework.

This is safe even for a dynamic API, because to follow REST principles, HTTP GET routes should always be idempotent and have no side-effects (as should HEAD, PUT, DELETE, OPTIONS and TRACE). Only HTTP POSTs are allowed to have side effects (see https://tools.ietf.org/html/rfc7231#section-4.2.2).

This behavior could be controlled by a flag defaulting to true.

@victorphoenix3
Copy link
Contributor

working on this

@tiangolo
Copy link
Owner

Thanks @matthewlloyd for the very clear argument and especially for the self-contained example, that helps a lot.

Indeed, here in the spec: https://tools.ietf.org/html/rfc7231#section-4.3.2 it says:

The server SHOULD send the same header fields in response to a HEAD request as it would have sent if the request had been a GET [...]

And the OpenAPI spec doesn't say anything contradicting it, so FastAPI should support it.

I think ideally, it should handle HEAD requests automatically for GET requests, without adding them to the OpenAPI schema. I think HEAD operations should be added to OpenAPI only when explicitly declared in the code (ideally).


Thanks @victorphoenix3 for working on this! Let me know if you need any help with it.

@nwillems
Copy link

As a side note, today, when making a HEAD request, you get a 405 returned, which according to spec*, should include an "Allow" header. I'm not if this is FastAPI or an underlying library returning this 405 - but maybe it should be changed as well.

@Halkcyon
Copy link

Halkcyon commented Feb 5, 2021

@victorphoenix3 Any progress on this issue? This bit me today. Is it just a matter of pulling from upstream?

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Feb 5, 2021

I'm on it.

EDIT: Implemented first approach, but I didn't like it that much. I'm going to implement another solution, which is slower at startup time but doesn't have the problems I stated on that PR.

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Feb 6, 2021

I've implemented the second approach. It needs a refactor, and I need to think if there's incompatibility with route.Route from Starlette.

I'll be waiting for @tiangolo's instructions before implementing/refactoring what is missing.

@Halkcyon
Copy link

Halkcyon commented Feb 6, 2021

@Kludex with either approach, we'll need to return the same headers with get/head to be spec-compliant (as @tiangolo has already pointed out) so a lot of test logic with GET can be re-used sans body

@jw-00000
Copy link

This bit me today when implementing a Docker healthcheck using wget --spider (which sends a HEAD request). Any updates on this?

@wookiesh
Copy link

wookiesh commented Nov 9, 2021

Got the same issue as our app is monitored by uptimerobot

@r614
Copy link

r614 commented Dec 2, 2021

Bumping this thread, we are using this for health-checks, and pre-fetch verifications. Any updates would be great 👀

@wookiesh
Copy link

wookiesh commented Dec 3, 2021

Btw if it's possible to help, I'm more than willing to.

@bletvaska
Copy link

i found this issue today, and i play a bit, and this is what works for me (FastAPI 0.70.0):

@router.head("/hello")
@router.get("/hello")
def read_root():
    return {"Hello": "World"}

so with httpie this are the results. for GET method

$ http http://localhost:8080/hello
HTTP/1.1 200 OK
content-length: 17
content-type: application/json
date: Mon, 06 Dec 2021 15:14:18 GMT
server: uvicorn

{
    "Hello": "World"
}

and then for HEAD method:

$ http head http://localhost:8080/hello
HTTP/1.1 200 OK
content-length: 17
content-type: application/json
date: Mon, 06 Dec 2021 15:14:15 GMT
server: uvicorn

@zakjan
Copy link

zakjan commented Nov 28, 2022

To avoid duplicating the request line, @router.api_route can be used:

@router.api_route(url, methods=['GET', 'HEAD'])

@k2ev
Copy link

k2ev commented Mar 13, 2023

This got me today and I was unaware of this open issue. I got help from stack overflow.
[https://stackoverflow.com/questions/75711757/fastapi-get-endpoint-returns-405-method-not-allowed-response]

@juantoca
Copy link

juantoca commented Jul 24, 2023

Hey. I was taking a look at open issues I could contribute to and I stumble with this one.

A way to implement this behavior without changing Fastapi or remembering to add the head decorators is to define an startup hook that add those head endpoints for you:

  @app.on_event("startup")
  def add_default_head_endpoints(self) -> None:
        for route in self.routes:
            if isinstance(route, routing.APIRoute) and "GET" in route.methods:
                new_route = copy(route)
                new_route.methods = {"HEAD"}
                new_route.include_in_schema = False
                self.routes.append(new_route)

As this is a desired behavior for Fastapi, I created a PR that implements this hook as a builtin. Feel free to take a look at it 😄

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jul 24, 2023

Hey. I was taking a look at open issues I could contribute to and I stumble with this one.

A way to implement this behavior without changing Fastapi or remembering to add the head decorators is to define an startup hook that add those head endpoints for you:

  @app.on_event("startup")
  def add_default_head_endpoints(self) -> None:
        for route in self.routes:
            if isinstance(route, routing.APIRoute) and "GET" in route.methods:
                new_route = copy(route)
                new_route.methods = {"HEAD"}
                new_route.include_in_schema = False
                self.routes.append(new_route)

As this is a desired behavior for Fastapi, I created a PR that implements this hook as a builtin. Feel free to take a look at it 😄

It's not desired to include routes on the "startup" event. I have two PRs about it already. I'll check later which one should be kept to review.

@juantoca
Copy link

juantoca commented Jul 24, 2023

Hey. I was taking a look at open issues I could contribute to and I stumble with this one.

A way to implement this behavior without changing Fastapi or remembering to add the head decorators is to define an startup hook that add those head endpoints for you:

  @app.on_event("startup")
  def add_default_head_endpoints(self) -> None:
        for route in self.routes:
            if isinstance(route, routing.APIRoute) and "GET" in route.methods:
                new_route = copy(route)
                new_route.methods = {"HEAD"}
                new_route.include_in_schema = False
                self.routes.append(new_route)

As this is a desired behavior for Fastapi, I created a PR that implements this hook as a builtin. Feel free to take a look at it 😄

It's not desired to include routes on the "startup" event. I have two PRs about it already. I'll check later which one should be kept to review.

I did take a look at your PRs and, as they were inactive, I decided to propose another approach. However, if you are going to retake the issue you'll probably know better than me the proper design decisions to take 😁. Thanks for taking a look!!

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jul 24, 2023

Well... The PRs are not inactive... They were just not reviewed. 😅

But I improved my skills since I developed them, so I'll take another view.

@francipvb
Copy link

Hello,

What about this? I need somewhat like this.

@WilliamStam
Copy link

WilliamStam commented Nov 13, 2023

and this is biting me as well :(

HAproxy sends a HEAD request every few seconds to make sure the site is up

i used @juantoca 's startup since there is no ways im going through a f ton of endpoints to add the head method to them

a slight change since im not using a class for the app part

from fastapi.routing import APIRoute

@app.on_event("startup")
async def add_default_head_endpoints() -> None:
    for route in app.routes:
        if isinstance(route, APIRoute) and "GET" in route.methods:
            new_route = copy(route)
            new_route.methods = {"HEAD"}
            new_route.include_in_schema = False
            app.routes.append(new_route)

INFO:     xxx.xxx.xxx.xxx:42028 - "HEAD / HTTP/1.1" 200 OK
2023-11-13 09:32:26 1790 DEBUG:fastapi_events.middleware: Processing events
2023-11-13 09:32:26 1790 DEBUG:fastapi_events.middleware: Resetting event_store ctx
2023-11-13 09:32:41 1790 DEBUG:fastapi_events.middleware: Setting event_store ctx

im on fastapi-0.104.1 and python 3.12

this would probably apply to OPTIONS method as well i guess altho that would need to be on every url right? im conveniently ignoring this but im sure it "should" be something included


as a side note for anyone landing up here that ahs a proxy on the front end... this is how i solved it

# main.py
import uvicorn

if __name__ == "__main__":
    uvicorn.run(
        # ...
        proxy_headers=True, # important for getting client info
        forwarded_allow_ips='xxx.xxx.xxx.xxx' # proxy ip
    )

you only need to have the options on the / route for the proxy to pickup the api is "alive"

@router.head("/", response_class=HTMLResponse, include_in_schema=False)
async def im_alive(request: Request):
    return Response()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed feature New feature or request reviewed
Projects
None yet
Development

No branches or pull requests