Incorrect header validation about tornado

[bdarnell]

Tornado is definitely a product of the View Source era of web development and Postel's law - there are probably a lot of places like this where we're more permissive than the standards. I'm inclined to be cautious about becoming less permissive (unless there's a concrete risk as with underscores in Content-Length), though, since there's always the possibility that someone is relying on the old behavior.

The way the set of allowed header characters was documented changed between RFC 2616 and 7230. 2616 said "anything but control and separators", while 7230 listed the allowed characters. I don't think that's a semantic change (if so it wasn't noted in the "differences from 2616" section), but the new form is much clearer. In particular, it was less obvious that only ascii was allowed (I was under the impression that ISO-8859-1 was used), leading to confusion in #2043. 7230 does document some changes in this area, in particular forbidding leading/trailing whitespace.

I agree with your suggestions; nuls and whitespace seem like the most problematic characters here and the least likely to cause backwards-compatibility problems. I'm also inclined to forbid all non-ascii characters. But I'd probably queue these changes up for Tornado 7.0 instead of putting them in a minor release.

[kenballus]

Tornado is definitely a product of the View Source era of web development and Postel's law - there are probably a lot of places like this where we're more permissive than the standards. I'm inclined to be cautious about becoming less permissive (unless there's a concrete risk as with underscores in Content-Length), though, since there's always the possibility that someone is relying on the old behavior.

This is understandable. I think most users expect a pretty strict interpretation of the standards these days, so it might be worth keeping a list of those places in which Tornado is more permissive than the standards recommend.

I agree with your suggestions; nuls and whitespace seem like the most problematic characters here and the least likely to cause backwards-compatibility problems. I'm also inclined to forbid all non-ascii characters. But I'd probably queue these changes up for Tornado 7.0 instead of putting them in a minor release.

I think it makes sense to delay changing behavior on non-ascii headers until 7.0, but nul could potentially be handled in a minor release, given that nul bytes aren't in any non-null UTF-8 characters.