* [PATCH] http-backend: provide Allow header for 405
@ 2013-09-08 18:15 brian m. carlson
2013-09-09 4:18 ` Jonathan Nieder
2013-09-10 6:24 ` Jeff King
0 siblings, 2 replies; 4+ messages in thread
From: brian m. carlson @ 2013-09-08 18:15 UTC (permalink / raw)
To: git; +Cc: mhagger, peff, jkoleszar, gitster
The HTTP 1.1 standard requires an Allow header for 405 Method Not Allowed:
The response MUST include an Allow header containing a list of valid methods
for the requested resource.
So provide such a header when we return a 405 to the user agent.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
http-backend.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/http-backend.c b/http-backend.c
index 0324417..8c61084 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -594,8 +594,11 @@ int main(int argc, char **argv)
if (strcmp(method, c->method)) {
const char *proto = getenv("SERVER_PROTOCOL");
- if (proto && !strcmp(proto, "HTTP/1.1"))
+ if (proto && !strcmp(proto, "HTTP/1.1")) {
http_status(405, "Method Not Allowed");
+ hdr_str("Allow", !strcmp("GET", c->method) ?
+ "GET, HEAD" : c->method);
+ }
else
http_status(400, "Bad Request");
hdr_nocache();
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] http-backend: provide Allow header for 405
2013-09-08 18:15 [PATCH] http-backend: provide Allow header for 405 brian m. carlson
@ 2013-09-09 4:18 ` Jonathan Nieder
2013-09-11 2:14 ` brian m. carlson
2013-09-10 6:24 ` Jeff King
1 sibling, 1 reply; 4+ messages in thread
From: Jonathan Nieder @ 2013-09-09 4:18 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, mhagger, peff, jkoleszar, gitster
Hi,
brian m. carlson wrote:
> --- a/http-backend.c
> +++ b/http-backend.c
> @@ -594,8 +594,11 @@ int main(int argc, char **argv)
>
> if (strcmp(method, c->method)) {
> const char *proto = getenv("SERVER_PROTOCOL");
> - if (proto && !strcmp(proto, "HTTP/1.1"))
> + if (proto && !strcmp(proto, "HTTP/1.1")) {
> http_status(405, "Method Not Allowed");
> + hdr_str("Allow", !strcmp("GET", c->method) ?
> + "GET, HEAD" : c->method);
> + }
> else
Small style nit: the closing brace should go on the same line as the
"else", like so:
if (proto && ...) {
...
} else
http_status(400, "Bad Request");
Another micronit: the comparison should be !strcmp(c->method, "GET")
--- variable first, then constant it is being compared to.
The functional change looks good.
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] http-backend: provide Allow header for 405
2013-09-09 4:18 ` Jonathan Nieder
@ 2013-09-11 2:14 ` brian m. carlson
0 siblings, 0 replies; 4+ messages in thread
From: brian m. carlson @ 2013-09-11 2:14 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, mhagger, peff, jkoleszar, gitster
[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]
On Sun, Sep 08, 2013 at 09:18:12PM -0700, Jonathan Nieder wrote:
> > --- a/http-backend.c
> > +++ b/http-backend.c
> > @@ -594,8 +594,11 @@ int main(int argc, char **argv)
> >
> > if (strcmp(method, c->method)) {
> > const char *proto = getenv("SERVER_PROTOCOL");
> > - if (proto && !strcmp(proto, "HTTP/1.1"))
> > + if (proto && !strcmp(proto, "HTTP/1.1")) {
> > http_status(405, "Method Not Allowed");
> > + hdr_str("Allow", !strcmp("GET", c->method) ?
> > + "GET, HEAD" : c->method);
> > + }
> > else
>
> Small style nit: the closing brace should go on the same line as the
> "else", like so:
>
> if (proto && ...) {
> ...
> } else
> http_status(400, "Bad Request");
>
> Another micronit: the comparison should be !strcmp(c->method, "GET")
> --- variable first, then constant it is being compared to.
>
> The functional change looks good.
I plan on doing a reroll tomorrow.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] http-backend: provide Allow header for 405
2013-09-08 18:15 [PATCH] http-backend: provide Allow header for 405 brian m. carlson
2013-09-09 4:18 ` Jonathan Nieder
@ 2013-09-10 6:24 ` Jeff King
1 sibling, 0 replies; 4+ messages in thread
From: Jeff King @ 2013-09-10 6:24 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, mhagger, jkoleszar, gitster
On Sun, Sep 08, 2013 at 06:15:06PM +0000, brian m. carlson wrote:
> The HTTP 1.1 standard requires an Allow header for 405 Method Not Allowed:
>
> The response MUST include an Allow header containing a list of valid methods
> for the requested resource.
>
> So provide such a header when we return a 405 to the user agent.
Makes sense.
> if (strcmp(method, c->method)) {
> const char *proto = getenv("SERVER_PROTOCOL");
> - if (proto && !strcmp(proto, "HTTP/1.1"))
> + if (proto && !strcmp(proto, "HTTP/1.1")) {
> http_status(405, "Method Not Allowed");
> + hdr_str("Allow", !strcmp("GET", c->method) ?
> + "GET, HEAD" : c->method);
> + }
It took me a minute to figure out what is going on here. But we seem to
convert HEAD requests into GETs elsewhere, so any "GET" service should
be able to do either.
Looks OK to me.
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-11 2:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-08 18:15 [PATCH] http-backend: provide Allow header for 405 brian m. carlson
2013-09-09 4:18 ` Jonathan Nieder
2013-09-11 2:14 ` brian m. carlson
2013-09-10 6:24 ` Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).