Convert HTML header field names to lowercase for HTTP2 #5840
Conversation
cclauss
requested review from
bnewbold,
hornc,
mekarpeles,
cdrini and
jimchamp
cclauss
force-pushed
the
lowercase-headers-for-http2
branch
from
1d99261 to
559bd14
Compare
cclauss
force-pushed
the
lowercase-headers-for-http2
branch
from
559bd14 to
0941d38
Compare
| return web.HTTPError( | ||
| "401 Authorization Required", | ||
| { | ||
| "www-authenticate": 'Basic realm="http://openlibrary.org"', |
There was a problem hiding this comment.
In some places above we've changed Basic -> basic
There are several other locations here where we don't make that change
There was a problem hiding this comment.
Nice catch! Thanks.
| return web.HTTPError( | ||
| "401 Authorization Required", | ||
| { | ||
| "www-authenticate": 'Basic realm="http://openlibrary.org"', |
There was a problem hiding this comment.
Nice catch! Thanks.
| @@ -14,7 +14,7 @@ msgstr "" | |||
| "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" | |||
| "Language-Team: cs <LL@li.org>\n" | |||
| "MIME-Version: 1.0\n" | |||
| "Content-Type: text/plain; charset=utf-8\n" | |||
| "content-type: text/plain; charset=utf-8\n" | |||
There was a problem hiding this comment.
I don't think that Content-Type should be changed in any of the .po files. This header entry is used by gettext, and I can't find any guidance regarding case sensitivity of header entries. Better safe than sorry.
|
I'm of the mind we should close this ; it's a pretty big potentially risky change to fix an issue that we haven't observed on the site. This sort of PR is actually exactly what I was trying to avoid by posting #5418 (comment) and #5789 (comment) . I imagine case sensitivity is handled at a lower level by Sorry for the run-around, @cclauss ; I thought we were on the same page! |
HTTP2 requires that header field names be lowercase ss discussed at
https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2 and #5789 (comment)
Technical
Testing
Screenshot
Stakeholders