Hello! On Wed, May 08, 2024 at 10:59:30AM -0700, Maksim Yevmenkin wrote:
> > > it appears that nginx would happily accept http header without colon > > > (:) in the header name. the patch below tries to address this. > > > > Yes. I believe it was allowed as a part of the following change > > in nginx 0.1.29 (509:9b8c906f6e63, > > https://freenginx.org/hg/nginx/rev/9b8c906f6e63#l85.68): > > > > *) Change: nginx now passes the invalid lines in a client request > > headers or a backend response header. > > > > With introduction of "ignore_invalid_headers" in nginx 0.1.30 > > (511:c12967aadd87) this wasn't changed though, which is probably > > an oversight. > > > > Such headers are essentially recognized as headers with an > > empty value, and I don't think that this can be an issue. Still, > > I don't object hardening the parser and rejecting such headers. > > > > I don't think this change will affect any real clients, though > > potentially might affect some misbehaving backend code, such as > > loosely written fastcgi scripts. If its the case, such code needs > > to be fixed. > > > > Alternatively, we can consider marking such headers as invalid, so > > they will be ignored from clients (unless "ignore_invalid_headers off;" > > is specified) and passed from backend servers to clients. Not > > sure it worth the effort though. > > [...] > > > Removing relevant if blocks should be enough, these will be > > handled by the following "if (ch <= 0x20...)" block. Please take > > a look at the following patch: > > > > # HG changeset patch > > # User Maxim Dounin <mdou...@mdounin.ru> > > # Date 1715122480 -10800 > > # Wed May 08 01:54:40 2024 +0300 > > # Node ID e9aa91a1861dcabfd1e9ebb7d5d96cdb42c8dcd4 > > # Parent 93bbb9fbf30dd82709551610f05e22eac17717d4 > > Disabled handling of headers without a colon. > > > > Starting with nginx 0.1.29 (509:9b8c906f6e63), header names not followed > > by a colon and a value were allowed. Such headers were interpreted as > > headers with an empty value. With this change, such headers are > > unconditionally rejected. > > > > Requested by Maksim Yevmenkin. > > > > diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c > > --- a/src/http/ngx_http_parse.c > > +++ b/src/http/ngx_http_parse.c > > @@ -961,21 +961,6 @@ ngx_http_parse_header_line(ngx_http_requ > > break; > > } > > > > - if (ch == CR) { > > - r->header_name_end = p; > > - r->header_start = p; > > - r->header_end = p; > > - state = sw_almost_done; > > - break; > > - } > > - > > - if (ch == LF) { > > - r->header_name_end = p; > > - r->header_start = p; > > - r->header_end = p; > > - goto done; > > - } > > - > > /* IIS may send the duplicate "HTTP/1.1 ..." lines */ > > if (ch == '/' > > && r->upstream > > this patch works. > > !thanks > max Committed, thanks for prodding this. -- Maxim Dounin http://mdounin.ru/ -- nginx-devel mailing list nginx-devel@freenginx.org https://freenginx.org/mailman/listinfo/nginx-devel
