[BUG] default method-not-allowed response misses HTTP header Allow:

LeSpocky commented 2 years ago

Prerequisites

[x] Put an X between the brackets on this line if you have checked that your issue isn't already filed: https://github.com/search?l=&q=repo%3Aetr%2Flibhttpserver&type=Issues

Description

RFC 7231 states in section 6.5.5. 405 Method Not Allowed:

The 405 (Method Not Allowed) status code indicates that the method received in the request-line is known by the origin server but not supported by the target resource. The origin server MUST generate an Allow header field in a 405 response containing a list of the target resource's currently supported methods.

That Allow: header is not sent by libhttpserver.

Steps to Reproduce

build with examples
run examples/allowing_disallowing_methods
send POST request, e.g. with curl -XPOST -v http://localhost:8080/hello
inspect HTTP headers

Expected behavior: send that header based on allowed_methods member of class http_resource

Actual behavior: HTTP header Allow: is not sent

Reproduces how often: always, at least unless webserver is constructed with a custom method_not_allowed_resource.

Versions

OS version: Debian GNU/Linux 10 (buster)
libhttpserver version: 0.18.2-22-g263f2bd (current master)
libmicrohttpd version: 0.9.62 (Debian package)

Additional Information

curl output (tail):

* Connected to localhost (127.0.0.1) port 8080 (#0)
> POST /hello HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.64.0
> Accept: */*
> 
< HTTP/1.1 405 Method Not Allowed
< Connection: Keep-Alive
< Content-Length: 18
< Content-Type: text/plain
< Date: Tue, 09 Nov 2021 13:16:31 GMT
< 
* Connection #0 to host localhost left intact
Method not Allowed

Example and explanation for allow header: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow

LeSpocky commented 2 years ago

I tried solving this. First I added a test case, you already find that in a branch in my personal fork. Then I identified the place in webserver.cpp where the header should be added. Now I'm stuck when constructing the header line itself. It would require looping over the allowed_methods private member of class httpserver::http_resource and there's no member function to do that. What would be your preferred way of extending httpserver::http_resource here?

new method to return a string with allowed methods? e.g. "GET, POST, HEAD"
new method to let someone iterate over all or allowed methods?
something else?

Would appreciate any input, because that would mean altering the public API, right?

etr commented 2 years ago

Hi @LeSpocky, thanks for raising the issue and taking it up to fix it. It is much appreciated.

I think that a public method that iterates over the allowed_methods and returns a list of allowed ones will make sense in public interface. Essentially:

std::list<std::string> http_resource::get_allowed_methods();

At the same time, I think it might be worth changing the name of the internal structure from "allowed_methods" to something else (i.e. "method_state") that indicates it doesn't contain only allowed_methods but the state of every method (allowed or not). That would avoid confusion given the name I am proposing for the method above.

etr / libhttpserver