> On 29 Aug 2023, at 08:14, Maxim Dounin <mdou...@mdounin.ru> wrote: > > Hello! > > On Sat, Aug 26, 2023 at 04:21:07PM +0200, Jérémy Lal wrote: > >> Hi, >> >> https://bugs.debian.org/1050571 >> reports that the Status line doesn't always end with space, which seems >> contradictory to RFC9112 which states: >> "A server MUST send the space that separates the status-code from the >> reason-phrase even when the reason-phrase is absent (i.e., the status-line >> would end with the space)." >> >> Is it a documented nginx 1.24.0 behavior ? > > Interesting. > > As you can see from the report referenced, nginx returns in the > HTTP status what is sent by the FastCGI application in the > "Status" response header. > > [..] > > Summing the above, I tend to think that it is generally a bad idea > to use Status header without a reason-phrase, as it is going to > result in missing SP sooner or later. At least if you do care > about missing SP in the status line (AFAIK, it causes no practical > issues, though I'm not really tested).
Agree. > > As for the nginx behaviour, I don't think we want to try to > implement custom parsing for the Status header to preserve > trailing SP if it's present. We can, however, consider using > only the status code from such Status headers, so nginx will > provide reason phrase by itself. > > Something like this should do the trick: > > # HG changeset patch > # User Maxim Dounin <mdou...@mdounin.ru> > # Date 1693282407 -10800 > # Tue Aug 29 07:13:27 2023 +0300 > # Node ID 10aec7047ed8c8e429e8e9b9d676a83751899bc6 > # Parent 44536076405cf79ebdd82a6a0ab27bb3aed86b04 > Upstream: fixed handling of Status headers without reason-phrase. > > Status header with an empty reason-phrase, such as "Status: 404 ", is > valid per CGI specification, but looses the trailing space during parsing. > Currently, this results in "HTTP/1.1 404" HTTP status line in the response, > which violates HTTP specification due to missing trailing space. > > With this change, only the status code is used from such short Status > header lines, so nginx will generate status line itself, with the space > and appropriate reason phrase if available. > > Reported at: > https://mailman.nginx.org/pipermail/nginx/2023-August/EX7G4JUUHJWJE5UOAZMO5UD6OJILCYGX.html > > diff --git a/src/http/modules/ngx_http_fastcgi_module.c > b/src/http/modules/ngx_http_fastcgi_module.c > --- a/src/http/modules/ngx_http_fastcgi_module.c > +++ b/src/http/modules/ngx_http_fastcgi_module.c > @@ -2048,7 +2048,10 @@ ngx_http_fastcgi_process_header(ngx_http > } > > u->headers_in.status_n = status; > - u->headers_in.status_line = *status_line; > + > + if (status_line->len > 3) { > + u->headers_in.status_line = *status_line; > + } > > } else if (u->headers_in.location) { > u->headers_in.status_n = 302; > diff --git a/src/http/modules/ngx_http_scgi_module.c > b/src/http/modules/ngx_http_scgi_module.c > --- a/src/http/modules/ngx_http_scgi_module.c > +++ b/src/http/modules/ngx_http_scgi_module.c > @@ -1153,7 +1153,10 @@ ngx_http_scgi_process_header(ngx_http_re > } > > u->headers_in.status_n = status; > - u->headers_in.status_line = *status_line; > + > + if (status_line->len > 3) { > + u->headers_in.status_line = *status_line; > + } > > } else if (u->headers_in.location) { > u->headers_in.status_n = 302; > diff --git a/src/http/modules/ngx_http_uwsgi_module.c > b/src/http/modules/ngx_http_uwsgi_module.c > --- a/src/http/modules/ngx_http_uwsgi_module.c > +++ b/src/http/modules/ngx_http_uwsgi_module.c > @@ -1381,7 +1381,10 @@ ngx_http_uwsgi_process_header(ngx_http_r > } > > u->headers_in.status_n = status; > - u->headers_in.status_line = *status_line; > + > + if (status_line->len > 3) { > + u->headers_in.status_line = *status_line; > + } > > } else if (u->headers_in.location) { > u->headers_in.status_n = 302; > > After discussion in the adjacent thread, I think the change is fine. -- Sergey Kandaurov _______________________________________________ nginx mailing list nginx@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx
