> On 29 Aug 2023, at 08:14, Maxim Dounin <mdounin@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 <mdounin@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