Respond 400 to malformed methods instead of 405

[kenballus]

The HTTP standard has a grammar rule that defines valid HTTP methods. This patch checks that methods conform to that rule. Note that this does not preclude a user from defining their own custom methods; only that such custom methods cannot contain (e.g.) null bytes and other crazy junk like that.

[bdarnell]

This makes sense, but does it ever matter in practice? It doesn't seem worth the (small) performance cost of an extra re.match call. However, I think we could merge the split and version re.match into one big regex to parse the request line and probably improve performance while being more strict about method naming.

[kenballus]

The issue is that 405 is cacheable.

When Tornado is deployed behind a cache server that parses methods differently from the way that Tornado does, there's the potential for cache poisoning.

For example, there exist caches that forward GET\x00 / as-is, but treat them as equivalent to GET /. Tornado will see this, and respond 405, which will be stored in the cache. The next time someone tries to GET /, they'll get 405.

Of course, that behavior is a bug in the cache, and needs to be fixed there too.

[bdarnell]

Ah, I see.

RFC 9110 section 9.1 also distinguishes between methods that are not recognized (501) and those that are recognized but not supported by the target resource (405). The logic behind that is probably similar.