threads / patch / 34880

patchhttp-backend: provide Allow header for 405

Subject: [PATCH] http-backend: provide Allow header for 405

## tl;dr

4 messages between Sep 8, 2013 and Sep 11, 2013. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

brian m. carlson· Sep 8, 2013, 18:15 UTC · lore

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(-)

Show changes to http-backend.c +4 −1

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

Jonathan Nieder· Sep 9, 2013, 04:18 UTC · re: brian m. carlson · lore

Re: [PATCH] http-backend: provide Allow header for 405

Hi,

brian m. carlson wrote:

Show 13 quoted lines

> --- 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

brian m. carlson· Sep 11, 2013, 02:14 UTC · re: Jonathan Nieder · lore

Re: [PATCH] http-backend: provide Allow header for 405

On Sun, Sep 08, 2013 at 09:18:12PM -0700, Jonathan Nieder wrote:

Show 26 quoted lines

> > --- 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

Jeff King· Sep 10, 2013, 06:24 UTC · re: brian m. carlson · lore

Re: [PATCH] http-backend: provide Allow header for 405

On Sun, Sep 08, 2013 at 06:15:06PM +0000, brian m. carlson wrote:

Show 6 quoted lines

> 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.

Show 8 quoted lines

>  			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