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

OPTIONS returns "200 OK" even though there is no body #156

Open
cd1 opened this issue Sep 10, 2016 · 6 comments
Open

OPTIONS returns "200 OK" even though there is no body #156

cd1 opened this issue Sep 10, 2016 · 6 comments
Milestone

Comments

@cd1
Copy link

cd1 commented Sep 10, 2016

Isn't "204 No Content" a more appropriate return status for OPTIONS requests, as they don't return anything in their bodies?

AFAIU, "200 OK" should be used when the request is successful and there is some data to return, and "204 No Content" when the request is successful but there isn't data to return. "200 OK" is used currently because in https://github.com/julienschmidt/httprouter/blob/master/router.go#L382, no status is set.

When I send an OPTIONS request to my server, I get:

HTTP/1.1 200 OK
Allow: GET, HEAD, POST, OPTIONS
Date: Sat, 10 Sep 2016 23:08:40 GMT
Content-Length: 0
Content-Type: text/plain; charset=utf-8

IMO, even the Content-Type is misleading because there's no content at all.

I applied the following patch to my local code:

diff --git a/router.go b/router.go
index bb17330..bcc69bf 100644
--- a/router.go
+++ b/router.go
@@ -381,6 +381,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
                if r.HandleOPTIONS {
                        if allow := r.allowed(path, req.Method); len(allow) > 0 {
                                w.Header().Set("Allow", allow)
+                               w.WriteHeader(http.StatusNoContent)
                                return
                        }
                }

And it seemed to do the trick. If you agree with this, I can send a PR.

@sagikazarmark
Copy link

sagikazarmark commented Jan 17, 2017

According to https://tools.ietf.org/html/rfc7231#section-6.3.1 200 status code is perfectly valid for an OPTIONS request. As far as I know 204 is usually used when there is some kind of content involved (in the request for example), but the response does not have any.

I don't see any improvement in semantics by returning 204 (and in the end this is a semantical question IMO), but I am not against it either. This could be a breaking change though.

@cd1
Copy link
Author

cd1 commented Jan 18, 2017

According to that same link, returning 200 OK for an OPTIONS request is valid only when it succeeded and it has data to return:

The payload sent in a 200 response depends on the request method. For the methods defined by this specification, the intended meaning of the payload can be summarized as:
...
OPTIONS a representation of the communications options;

The server isn't returning any data for the OPTIONS request; the "communications options" are indicated via the response header Allow.

It's also more explicit in the following section:

Aside from responses to CONNECT, a 200 response always has a payload,
though an origin server MAY generate a payload body of zero length.
If no payload is desired, an origin server ought to send 204 (No
Content) instead.

Returning 204 No Content isn't necessarily related to a request with some content; for example, DELETE /resource/1 is a typical example of a request which should return 204 No Content on success and it has no content in the request as well.

I opened this issue because I'm writing some test cases for a server and I'm validating every data returned by 200 OK responses; in the case of the OPTIONS response here, the status is 200 OK but there's no data to validate - which is exactly the purpose of the status 204 No Content, IMO.

According to the RFC, a server may generate a payload body of zero length for a 200 OK response, but it seems much more appropriate to use a status which indicates just that.

@sagikazarmark
Copy link

According to that same link, returning 200 OK for an OPTIONS request is valid only when it succeeded and it has data to return

Although that's what the spec suggests, I wouldn't be too strict on that one. That's an option you can choose. It recommends to use 204 when there is no content, but it's not a strict rule.

I would say relying on the status code to determine whether there is content is not really a good choice.

My particular issue with using multiple success status codes is that clients might not be ready to handle them all, this doing this change is definitely breaking. I would rather use 200 always instead of changing from 204 to 200 if necessary. (That of course does not apply to error status codes as a client MUST handle every non-success status and have some kind of fallback logic for unknown statuses)

@julienschmidt julienschmidt added this to the v1.2 milestone Mar 24, 2017
@plroebuck
Copy link

I would say relying on the status code to determine whether there is content is not really a good choice.

Odd. RFC7230-section3.3.3 would seem to disagree with your assertion, concerning using 204.

My particular issue with using multiple success status codes is that clients might not be ready to handle them all, this doing this change is definitely breaking. I would rather use 200 always instead of changing from 204 to 200 if necessary. (That of course does not apply to error status codes as a client MUST handle every non-success status and have some kind of fallback logic for unknown statuses)

Although I'm certain someone hardcoded if status == 200, I disagree with the rest of your assertion. One would generally check for success (200<=status<300), correctable failure (400<=status<500), and uncorrectable failure (500<=status) to determine how to proceed.

Would kinda wonder about a portion of the logic in "router.go" though; wouldn't there always be an allow length? At minimum, wouldn't Allow: OPTIONS always be possible to qualify for a successful return?

@sagikazarmark
Copy link

Odd. RFC7230-section3.3.3 would seem to disagree with your assertion, concerning using 204.

I think I wasn't totally clear. I agree that 204 should mean there is actually no content. On the other hand, 200 with an empty body should be fine too. Maybe not for an OPTIONS request, but I think clients/servers with this level of RFC conformity is rare.

Ultimately my main argument is backwards compatibility which is no longer valid if we are talking about a new major version.

@julienschmidt julienschmidt modified the milestones: v1.2, v1.3 Oct 13, 2018
@julienschmidt julienschmidt modified the milestones: v1.3, v2 Sep 27, 2019
julienschmidt added a commit that referenced this issue Sep 29, 2019
julienschmidt added a commit that referenced this issue Sep 29, 2019
@julienschmidt
Copy link
Owner

Changing the status code is now possible via a global OPTIONS handler. See Automatic OPTIONS responses and CORS.

I will leave this issue open, since 204 should be considered as the default response code in v2.

similark pushed a commit to similarweb/httprouter that referenced this issue May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants