search

8thlight / cob_spec

A fitnesse suite for a web server
24 stars 33 forks source link

Expect 501 status code for "bogus" request #74

Closed bgerstle closed 5 years ago

bgerstle commented 6 years ago

RFC 7320 Section 3.1.1 (Request Line) states:

Recipients of an invalid request-line SHOULD respond with either a 400 (Bad Request) error or a 301 (Moved Permanently) redirect with the request-target properly encoded

Valid methods are defined here: https://tools.ietf.org/pdf/rfc7231.pdf#section-4

Therefore, cob_spec's "Bogus Request" should result in a 400, not 405.

trptcolin commented 6 years ago

This is interesting - there's some tension here. At first I was on board, due to both the "invalid request-line" snippet from RFC 7230 that you included, and from the definition of 405 in RFC 7231:

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.

... which totally supports this change, since the method in BogusRequest is generated from a random string of eight characters.

But then there's also this, from the end of section 4.1 of RFC 7231:

Additional methods, outside the scope of this specification, have been standardized for use in HTTP. All such methods ought to be registered within the "Hypertext Transfer Protocol (HTTP) Method Registry" maintained by IANA, as defined in Section 8.1.

The set of methods allowed by a target resource can be listed in an Allow header field (Section 7.4.1). However, the set of allowed methods can change dynamically. When a request method is received that is unrecognized or not implemented by an origin server, the origin server SHOULD respond with the 501 (Not Implemented) status code. When a request method is received that is known by an origin server but not allowed for the target resource, the origin server SHOULD respond with the 405 (Method Not Allowed) status code.

I'm reading this as being explicitly about the use case in this cob_spec test, and with that reading, it sounds like we might really want a 501 here. Does that sound right to you? If the request line was ill-formed, missing syntax in the protocol version, etc. I'd be 👍on a 400, but it seems like this last section is explicitly saying 501 ought to be the response for the requests we're sending (unless the random method turns out to be some added 8-character method like CHECKOUT that someone's server is implemented to support - see the current http method registry).

I'm reconciling this tension in my head by saying that RFC 7230 3.1.1 is about the syntax of the request-line, not its semantics. Not sure if that's the right reading, but it seems reasonable to me, particularly given the BNF in there.

jdesrosiers commented 6 years ago

I agree 501 Not Implemented is the best response. @trptcolin beat me to the explanation, but I have a little more to add.

The quote from RFC 7230 3.1.1 is in the context of improperly encoded request target (URI). That's what the 301 part of that quote is referring to.

400 Bad Request was originally intended for syntax errors in the message. The definition has been expanded to be a catchall for any semantic errors that don't have specific status code associated with them.

@bgerstle, thanks for putting in the effort to fix these things!

bgerstle commented 5 years ago

@jdesrosiers @trptcolin updated to expect 501 now

trptcolin commented 5 years ago

:shipit: