Closed samuelcolvin closed 7 years ago
bump, any news one this? I think it's important and requires a fix.
i am not entirely sure what you are proposing? Default methods or way how to add default methods?
I'm proposing a new method add
which allows HEAD requests as well as GET requests.
Then change in documentation to encourage people to use that method instead of add_get
with a comment saying "If you really want views which do not allow HEAD requests (which is contrary to the spirit of http) you can use the add_get
method."
Perhaps easiest if I do a pull request then you can comment on that.
Shouldn't allow GET also allow HEAD?
yes it should, but I do understand why it's confusing if add_get()
allowed HEAD
so we need another method but add_get_or_head()
would be annoying so I think just add()
would work.
I'm easy, could just modify add_get()
.
i think i'm missing something. you can do add_route('HEAD', ...)
app.router.add_get('/', index, name='index')
app.router.add_route('HEAD', '/', index)
Is a pretty ugly preferred way of adding a standard endpoint.
we can do something like router.add_get(...., enable_head=True)
and add add_head()
which can override default head
how that sound?
makes sense, for me enable_head
should be True by default, but I understand this is a significant change.
Perhaps have enable_head=False
by default but give a warning if enable_head
is not supplied saying "default behaviour will change in future to enabled_head=True
"?
Sounds good for me, just only may be allow_head=True
would be a bit more correct.
False
is good default. i fill allow_head
implies something else, i prefer enable_head
.
we can also add setting for dispatcher which can define default behavior of add_get()
It follows the same naming policy as HTTP has: you Allow
methods via same header, not enable them. Same story for CORS. If with this option False
we throw HTTP 405 on HEAD request then it's quite logical name.
but this would look strange:
app.router.add_get(..., allow_head=False)
app.router.add_head(...)
maybe enable_default_head
?
The enable_default_head
a bit more specific. But this looks strange in any way since you first disallow/disable HEAD and then you explicitly add it support. Why? (rhetorical question)
What if instead of:
app.router.add_get(..., allow_head=False)
app.router.add_head(...)
Have:
app.router.add_get(..., head_handler=custom_handler)
A bit out of rules, but since GET and HEAD are heavy bounded, could this be legal?
i like head_handler
!
@samuelcolvin ?
Brrr...ignore my last post.
Case 1: we're fine to handle HEAD requests with GET handler.
app.router.add_get(..., allow_head=True) # default since some time
Case 2: we need to disallow HEAD requests for GET endpoints
app.router.add_get(..., allow_head=False) # what we have now
Case 3: we want to have custom HEAD handler at the same endpoint
app.router.add_get(...)
app.router.add_head(...)
E.g. you don't need to explicitly disable default HEAD handler or care about if you define own a bit later. add_head
just simply overrides what we have by default.
API remains consistent and clear without mandatory flags or additional handlers.
Ah, one more:
Case 4: I don't know what I want to have
app.router.add_get(..., allow_head=False)
app.router.add_head(...)
(:
Yes, you, probably, can write something like that, but why? (not a rhetorical question)
but i like head_handler,
signature could be like this def add_get(head_handler=default_handler, *args, **kwargs):
this would cover all cases
I don't understand what head_handler
is. Is it a custom handler for process head requests?
That sounds wrong, HEAD requests should be processed by the same handler as get so the response code and headers are the same as GET.
@samuelcolvin Btw, that's a good point. Different handler may produce different response which will violate HEAD semantic. So it seems it just about one flag?
ok, it seems it is about flag only.
then allow_head
seems reasonable.
I think just allow_head
is fine.
Perhaps default_allow_head
as a kwarg to Application
so one can change the default behaviour?
technically default_allow_head
belongs to UrlDispatcher
True, but it's very rare to initialise UrlDispatcher directly, either an Application kwarg or a method:
app.router.set_default(allow_head=True)
app.router.add_get('/', index) # this will allow head requests
I can imagine there might be other default behaviour to configure on router
in the future.
@asvetlov do you have ideas about default_allow_head
or router.set_default(allow_head=True)
@samuelcolvin would you like to work on this feature?
yes will do.
cool, thanks.
Just checked and both flask and django by default allow
HEAD
requests (but notPOST
,PUT
etc.) to the default routed endpoints.The closest equivalent in aiohttp is
add_get
but understandably that doesn't allowHEAD
requests and returns 405.aiohttp should add a method to add routes which accept the "noop" or safe http methods eg.
GET
,HEAD
andOPTIONS
.To be consistent with other servers and act in the spirit of http, this method should be the default. AFAIK returning 405 to
HEAD
requests toGET
endpoints is wrong.I'm trying to think about the best name for such a method which is both clear and not verbose, it's not obvious. Wikipedia calls these "safe methods" but
add_safe
could be a confusing; making people think aiohttp is giving some kind of protection which it isn't.Perhaps the simplest options would be just
add
.So code would be
What exactly it should do with
OPTIONS
requests is debatable. The fact it should gracefully acceptHEAD
requests is a no-brainer for me.