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 ?
Thanks,
Jérémy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx/attachments/20230826/08bfeb5f/attachment.htm>
> On 26 Aug 2023, at 18:21, Jérémy Lal <kapouer at melix.org> 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 ?
>
Note that the response line with empty reason phrase
is properly generated since nginx 1.5.6.
The exception remains is proxying FastCGI responses
as there is no distinguished response line in CGI syntax.
The reason is that Status is a CGI header field, and hence
it is parsed by a generic routine that cuts trailing spaces.
But it can have a trailing space per RFC 3875, section 6.3.3.
So it needs a special treatment to preserve SP before empty reason
phrase. The below patch should help; although it doesn't look
efficient and can be polished, I think this is quite enough for
valid use cases.
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1693238094 -14400
# Mon Aug 28 19:54:54 2023 +0400
# Node ID f621c60dfa277774ab5fadb3c8b805957ca3f281
# Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
FastCGI: preserved SP for empty Status header reason phrase.
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,25 @@ 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) {
+ /* preserve SP for empty reason phrase */
+
+ u->headers_in.status_line.data = ngx_pnalloc(r->pool,
+ 5);
+ if (u->headers_in.status_line.data == NULL) {
+ return NGX_ERROR;
+ }
+
+ ngx_memcpy(u->headers_in.status_line.data,
+ status_line->data, 3);
+ u->headers_in.status_line.data[3] = ' ';
+ u->headers_in.status_line.data[4] = '\0';
+ u->headers_in.status_line.len = 4;
+
+ } else {
+ u->headers_in.status_line = *status_line;
+ }
} else if (u->headers_in.location) {
u->headers_in.status_n = 302;
Alternatively, the reason-phrase value from FastCGI response
can be ignored, nginx will translate it with its own value:
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1693241054 -14400
# Mon Aug 28 20:44:14 2023 +0400
# Node ID f1a452cbd57ff4fba1caf3f36cb3624bd45411ea
# Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
FastCGI: preserved SP for empty Status header reason phrase.
As per RFC 3875 sec 6.3.3, the status code is always followed by SP.
The header is parsed by the common routine, and it can be challenging
to properly translate empty reason phrase to HTTP/1.1 status line, as
that would require an additional memory allocation. For simplicity,
the reason phrase is now ignored; the header filter will take care of
it by inserting its own reason phrase for well known status codes.
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,6 @@ ngx_http_fastcgi_process_header(ngx_http
}
u->headers_in.status_n = status;
- u->headers_in.status_line = *status_line;
} else if (u->headers_in.location) {
u->headers_in.status_n = 302;
--
Sergey Kandaurov
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.
The tricky part is an attempt to use "Status" header to sent
status with a trailing space, "404 ". HTTP header values cannot
contain trailing spaces, see here:
https://www.rfc-editor.org/rfc/rfc9112.html#name-field-syntax
https://www.rfc-editor.org/rfc/rfc9112.html#section-5.1-3
: A field line value might be preceded and/or followed by optional
: whitespace (OWS); a single SP preceding the field line value is
: preferred for consistent readability by humans. The field line
: value does not include that leading or trailing whitespace: OWS
: occurring before the first non-whitespace octet of the field line
: value, or after the last non-whitespace octet of the field line
: value, is excluded by parsers when extracting the field line value
: from a field line.
As such, both "Status: 404" and "Status: 404 " are equivalent and
both contain value "404", without any trailing spaces. And this
is what nginx uses in the response, and this is what is seen in
the original report.
Following the original CGI specification, which uses HTTP headers
syntax in the response[1], this basically means that an
application is not allowed to sent Status with an empty reason
phrase.
[1] https://web.archive.org/web/20100212084036/http://hoohoo.ncsa.illinois.edu/cgi/out.html
: The output of scripts begins with a small header. This header
: consists of text lines, in the same format as an HTTP header,
: terminated by a blank line (a line with only a linefeed or CR/LF).
Similarly, original CGI specification RFC draft,
draft-robinson-www-interface-00 explicitly defines a generic
syntax for CGI response headers, which does not use implied LWS
rule (as in HTTP), but explicitly allows whitespaces between
tokens in the headers:
https://datatracker.ietf.org/doc/html/draft-robinson-www-interface-00#section-9.2
The CGI headers have the generic syntax:
generic-header = field-name ":" [ field-value ] NL
field-name = 1*<any CHAR, excluding CTLs, SP and ":">
field-value = *( field-content | LWSP )
field-content = *( token | tspecial | quoted-string)
While it does not explicitly state, like HTTP specification, that
leading and trailing LWSPs are not part of the field value, this
is the only approach which makes the resulting specification
usable.
Similarly, RFC 3875 uses the same generic syntax for headers, and
also tries to clarify whitespace usage:
https://datatracker.ietf.org/doc/html/rfc3875#section-6.3
... Whitespace is permitted between the ":"
and the field-value (but not between the field-name and the ":"), and
also between tokens in the field-value.
Though in contrast to draft-robinson-www-interface-00, it only
defines generic syntax for non-CGI headers. CGI headers, that is,
"Content-Type", "Location", and "Status", use their own syntax
defined separately. In particular, the Status header is defined
as follows:
https://datatracker.ietf.org/doc/html/rfc3875#section-6.3.3
Status = "Status:" status-code SP reason-phrase NL
status-code = "200" | "302" | "400" | "501" | extension-code
extension-code = 3digit
reason-phrase = *TEXT
Note that whitespaces are _not_ allowed by the syntax. This,
however, rules out responses like "Status: 200 OK" due to the
space between "Status:" and "200 OK". As such, it looks
reasonable to assume this is a specification bug, and at least
some whitespaces are expected to be allowed.
Overall, it does not look like RFC 3875 provides a consistent
approach to whitespace handling, and using the same approach as
for HTTP seem to be the best available option.
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).
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 at 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;
--
Maxim Dounin
http://mdounin.ru/
Hello!
On Mon, Aug 28, 2023 at 08:59:28PM +0400, Sergey Kandaurov wrote:
>
> > On 26 Aug 2023, at 18:21, Jérémy Lal <kapouer at melix.org> 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 ?
> >
>
> Note that the response line with empty reason phrase
> is properly generated since nginx 1.5.6.
> The exception remains is proxying FastCGI responses
> as there is no distinguished response line in CGI syntax.
>
> The reason is that Status is a CGI header field, and hence
> it is parsed by a generic routine that cuts trailing spaces.
> But it can have a trailing space per RFC 3875, section 6.3.3.
> So it needs a special treatment to preserve SP before empty reason
> phrase. The below patch should help; although it doesn't look
> efficient and can be polished, I think this is quite enough for
> valid use cases.
I very much doubt that RFC 3875 properly defines whitespace
handling, see my response to the original report. In this
particular case, it seems to define a header which cannot be
correctly parsed if reason-phrase is empty.
>
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1693238094 -14400
> # Mon Aug 28 19:54:54 2023 +0400
> # Node ID f621c60dfa277774ab5fadb3c8b805957ca3f281
> # Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
> FastCGI: preserved SP for empty Status header reason phrase.
>
> 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,25 @@ 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) {
> + /* preserve SP for empty reason phrase */
> +
> + u->headers_in.status_line.data = ngx_pnalloc(r->pool,
> + 5);
> + if (u->headers_in.status_line.data == NULL) {
> + return NGX_ERROR;
> + }
> +
> + ngx_memcpy(u->headers_in.status_line.data,
> + status_line->data, 3);
> + u->headers_in.status_line.data[3] = ' ';
> + u->headers_in.status_line.data[4] = '\0';
> + u->headers_in.status_line.len = 4;
> +
> + } else {
> + u->headers_in.status_line = *status_line;
> + }
>
> } else if (u->headers_in.location) {
> u->headers_in.status_n = 302;
Do you think we really need this complexity here? I tend to think
that
if (status_line->len > 3) {
u->headers_in.status_line = *status_line;
}
would be enough, so nginx will generate status line by itself for
such headers.
The only downside I can see is that it will provide no way to
actually generate a response with well-known status code, yet with
an empty reason phrase.
(Also note that uwsgi and scgi modules needs to be patched
similarly.)
> Alternatively, the reason-phrase value from FastCGI response
> can be ignored, nginx will translate it with its own value:
>
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1693241054 -14400
> # Mon Aug 28 20:44:14 2023 +0400
> # Node ID f1a452cbd57ff4fba1caf3f36cb3624bd45411ea
> # Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
> FastCGI: preserved SP for empty Status header reason phrase.
>
> As per RFC 3875 sec 6.3.3, the status code is always followed by SP.
> The header is parsed by the common routine, and it can be challenging
> to properly translate empty reason phrase to HTTP/1.1 status line, as
> that would require an additional memory allocation. For simplicity,
> the reason phrase is now ignored; the header filter will take care of
> it by inserting its own reason phrase for well known status codes.
>
> 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,6 @@ ngx_http_fastcgi_process_header(ngx_http
> }
>
> u->headers_in.status_n = status;
> - u->headers_in.status_line = *status_line;
>
> } else if (u->headers_in.location) {
> u->headers_in.status_n = 302;
I don't think it's a good idea, since this always drops the
reason phrase provided by the upstream server. It can contain
some meaningful information which will be lost as a result, most
notably for non-standard error codes.
--
Maxim Dounin
http://mdounin.ru/
> On 29 Aug 2023, at 08:33, Maxim Dounin <mdounin at mdounin.ru> wrote:
>
> Hello!
>
> On Mon, Aug 28, 2023 at 08:59:28PM +0400, Sergey Kandaurov wrote:
>
>>
>>> On 26 Aug 2023, at 18:21, Jérémy Lal <kapouer at melix.org> 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 ?
>>>
>>
>> Note that the response line with empty reason phrase
>> is properly generated since nginx 1.5.6.
>> The exception remains is proxying FastCGI responses
>> as there is no distinguished response line in CGI syntax.
>>
>> The reason is that Status is a CGI header field, and hence
>> it is parsed by a generic routine that cuts trailing spaces.
>> But it can have a trailing space per RFC 3875, section 6.3.3.
>> So it needs a special treatment to preserve SP before empty reason
>> phrase. The below patch should help; although it doesn't look
>> efficient and can be polished, I think this is quite enough for
>> valid use cases.
>
> I very much doubt that RFC 3875 properly defines whitespace
> handling, see my response to the original report. In this
> particular case, it seems to define a header which cannot be
> correctly parsed if reason-phrase is empty.
>
Yes, it is quite dubious how this can be parsed correctly.
Although it is valid to have a trailing space in Status,
this contradicts to header field value syntax per RFC 3875:
field-content = *( token | separator | quoted-string )
>>
>> # HG changeset patch
>> # User Sergey Kandaurov <pluknet at nginx.com>
>> # Date 1693238094 -14400
>> # Mon Aug 28 19:54:54 2023 +0400
>> # Node ID f621c60dfa277774ab5fadb3c8b805957ca3f281
>> # Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
>> FastCGI: preserved SP for empty Status header reason phrase.
>>
>> 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,25 @@ 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) {
>> + /* preserve SP for empty reason phrase */
>> +
>> + u->headers_in.status_line.data = ngx_pnalloc(r->pool,
>> + 5);
>> + if (u->headers_in.status_line.data == NULL) {
>> + return NGX_ERROR;
>> + }
>> +
>> + ngx_memcpy(u->headers_in.status_line.data,
>> + status_line->data, 3);
>> + u->headers_in.status_line.data[3] = ' ';
>> + u->headers_in.status_line.data[4] = '\0';
>> + u->headers_in.status_line.len = 4;
>> +
>> + } else {
>> + u->headers_in.status_line = *status_line;
>> + }
>>
>> } else if (u->headers_in.location) {
>> u->headers_in.status_n = 302;
>
> Do you think we really need this complexity here? I tend to think
> that
See above comment about efficiency.
I doubt empty reason phrase is seen often in the wild,
so this is a reasonable cost for kinda abusing protocol.
But this complexity also reflects the ugliness, that's
why I preferred the second version, also for its brevity.
>
> if (status_line->len > 3) {
> u->headers_in.status_line = *status_line;
> }
>
> would be enough, so nginx will generate status line by itself for
> such headers.
This is a nice trade-off between handling empty reason phrase
with extra allocation and dropping it altogether, I like it.
>
> The only downside I can see is that it will provide no way to
> actually generate a response with well-known status code, yet with
> an empty reason phrase.
>
> (Also note that uwsgi and scgi modules needs to be patched
> similarly.)
These modules use a distinct routine to parse status line,
that is ngx_http_parse_status_line, which respects spaces,
and they expect the first line to look like a status line
(similar to the proxy module).
So I don't see immediate necessity to patch them as well.
>
>> Alternatively, the reason-phrase value from FastCGI response
>> can be ignored, nginx will translate it with its own value:
>>
>> # HG changeset patch
>> # User Sergey Kandaurov <pluknet at nginx.com>
>> # Date 1693241054 -14400
>> # Mon Aug 28 20:44:14 2023 +0400
>> # Node ID f1a452cbd57ff4fba1caf3f36cb3624bd45411ea
>> # Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
>> FastCGI: preserved SP for empty Status header reason phrase.
>>
>> As per RFC 3875 sec 6.3.3, the status code is always followed by SP.
>> The header is parsed by the common routine, and it can be challenging
>> to properly translate empty reason phrase to HTTP/1.1 status line, as
>> that would require an additional memory allocation. For simplicity,
>> the reason phrase is now ignored; the header filter will take care of
>> it by inserting its own reason phrase for well known status codes.
>>
>> 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,6 @@ ngx_http_fastcgi_process_header(ngx_http
>> }
>>
>> u->headers_in.status_n = status;
>> - u->headers_in.status_line = *status_line;
>>
>> } else if (u->headers_in.location) {
>> u->headers_in.status_n = 302;
>
> I don't think it's a good idea, since this always drops the
> reason phrase provided by the upstream server. It can contain
> some meaningful information which will be lost as a result, most
> notably for non-standard error codes.
It is little to no benefit to carry reason phrase,
also this is consistent with HTTP/1.1:
: A client SHOULD ignore the reason-phrase content because
: it is not a reliable channel for information
Note also that HTTP/2 and then HTTP/3 do not define a way
to carry the reason phrase that is included in HTTP/1.1.
Personally I'd proceed with your approach for fastcgi module
only, though I won't insist to patch other modules as well.
Although this would introduce an extra condition in these
other modules without a reason (as per my above analysis),
this would keep sources in sync between them, which is fine,
and will allow to stop using ngx_http_parse_status_line().
For the latter, uwsgi seems to use HTTP syntax in response,
so it is ok to call ngx_http_parse_status_line() here, but
scgi seems to follows CGI syntax where status line is
carried in the Status header field, so the call would
always return NGX_ERROR. Luckily this is already handled
by falling back to ngx_http_scgi_process_header().
--
Sergey Kandaurov
Hello!
On Wed, Aug 30, 2023 at 04:20:15PM +0400, Sergey Kandaurov wrote:
> > On 29 Aug 2023, at 08:33, Maxim Dounin <mdounin at mdounin.ru> wrote:
> >
> > On Mon, Aug 28, 2023 at 08:59:28PM +0400, Sergey Kandaurov wrote:
> >
> >>> On 26 Aug 2023, at 18:21, Jérémy Lal <kapouer at melix.org> 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 ?
> >>>
> >>
> >> Note that the response line with empty reason phrase
> >> is properly generated since nginx 1.5.6.
> >> The exception remains is proxying FastCGI responses
> >> as there is no distinguished response line in CGI syntax.
> >>
> >> The reason is that Status is a CGI header field, and hence
> >> it is parsed by a generic routine that cuts trailing spaces.
> >> But it can have a trailing space per RFC 3875, section 6.3.3.
> >> So it needs a special treatment to preserve SP before empty reason
> >> phrase. The below patch should help; although it doesn't look
> >> efficient and can be polished, I think this is quite enough for
> >> valid use cases.
> >
> > I very much doubt that RFC 3875 properly defines whitespace
> > handling, see my response to the original report. In this
> > particular case, it seems to define a header which cannot be
> > correctly parsed if reason-phrase is empty.
> >
>
> Yes, it is quite dubious how this can be parsed correctly.
> Although it is valid to have a trailing space in Status,
> this contradicts to header field value syntax per RFC 3875:
> field-content = *( token | separator | quoted-string )
Note that field-value syntax does no apply to the "Status" header,
its syntax is defined separately.
On the other hand, the "Status" header syntax does not allow any
spaces after the colon, which rules out headers like "Status: 200 OK",
and makes the "Status" header syntax highly questionable.
As already suggested in my response to the original report, I tend
to think that the best available option is to ignore RFC 3875 idea
about headers syntax, and use HTTP instead.
>
> >>
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet at nginx.com>
> >> # Date 1693238094 -14400
> >> # Mon Aug 28 19:54:54 2023 +0400
> >> # Node ID f621c60dfa277774ab5fadb3c8b805957ca3f281
> >> # Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
> >> FastCGI: preserved SP for empty Status header reason phrase.
> >>
> >> 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,25 @@ 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) {
> >> + /* preserve SP for empty reason phrase */
> >> +
> >> + u->headers_in.status_line.data = ngx_pnalloc(r->pool,
> >> + 5);
> >> + if (u->headers_in.status_line.data == NULL) {
> >> + return NGX_ERROR;
> >> + }
> >> +
> >> + ngx_memcpy(u->headers_in.status_line.data,
> >> + status_line->data, 3);
> >> + u->headers_in.status_line.data[3] = ' ';
> >> + u->headers_in.status_line.data[4] = '\0';
> >> + u->headers_in.status_line.len = 4;
> >> +
> >> + } else {
> >> + u->headers_in.status_line = *status_line;
> >> + }
> >>
> >> } else if (u->headers_in.location) {
> >> u->headers_in.status_n = 302;
> >
> > Do you think we really need this complexity here? I tend to think
> > that
>
> See above comment about efficiency.
> I doubt empty reason phrase is seen often in the wild,
> so this is a reasonable cost for kinda abusing protocol.
> But this complexity also reflects the ugliness, that's
> why I preferred the second version, also for its brevity.
>
> >
> > if (status_line->len > 3) {
> > u->headers_in.status_line = *status_line;
> > }
> >
> > would be enough, so nginx will generate status line by itself for
> > such headers.
>
> This is a nice trade-off between handling empty reason phrase
> with extra allocation and dropping it altogether, I like it.
Please see my response to the original report for the full patch.
> >
> > The only downside I can see is that it will provide no way to
> > actually generate a response with well-known status code, yet with
> > an empty reason phrase.
> >
> > (Also note that uwsgi and scgi modules needs to be patched
> > similarly.)
>
> These modules use a distinct routine to parse status line,
> that is ngx_http_parse_status_line, which respects spaces,
> and they expect the first line to look like a status line
> (similar to the proxy module).
> So I don't see immediate necessity to patch them as well.
Sure, but these are distinct parsing modes: parsing status line as
in raw HTTP responses (which, in particular, are used by NPH scripts),
or parsing the Status header, as in CGI responses. Both should
work correctly.
In particular, current SCGI specification
(https://python.ca/scgi/protocol.txt) uses "Status: ..." in the
example (whereas original version used "HTTP/1.0 ...",
https://web.archive.org/web/20020403050958/http://python.ca/nas/scgi/protocol.txt).
I see no reasons why "Status: 404 " should work correctly in
FastCGI, but shouldn't in SCGI.
> >> Alternatively, the reason-phrase value from FastCGI response
> >> can be ignored, nginx will translate it with its own value:
> >>
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet at nginx.com>
> >> # Date 1693241054 -14400
> >> # Mon Aug 28 20:44:14 2023 +0400
> >> # Node ID f1a452cbd57ff4fba1caf3f36cb3624bd45411ea
> >> # Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
> >> FastCGI: preserved SP for empty Status header reason phrase.
> >>
> >> As per RFC 3875 sec 6.3.3, the status code is always followed by SP.
> >> The header is parsed by the common routine, and it can be challenging
> >> to properly translate empty reason phrase to HTTP/1.1 status line, as
> >> that would require an additional memory allocation. For simplicity,
> >> the reason phrase is now ignored; the header filter will take care of
> >> it by inserting its own reason phrase for well known status codes.
> >>
> >> 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,6 @@ ngx_http_fastcgi_process_header(ngx_http
> >> }
> >>
> >> u->headers_in.status_n = status;
> >> - u->headers_in.status_line = *status_line;
> >>
> >> } else if (u->headers_in.location) {
> >> u->headers_in.status_n = 302;
> >
> > I don't think it's a good idea, since this always drops the
> > reason phrase provided by the upstream server. It can contain
> > some meaningful information which will be lost as a result, most
> > notably for non-standard error codes.
>
> It is little to no benefit to carry reason phrase,
> also this is consistent with HTTP/1.1:
> : A client SHOULD ignore the reason-phrase content because
> : it is not a reliable channel for information
>
> Note also that HTTP/2 and then HTTP/3 do not define a way
> to carry the reason phrase that is included in HTTP/1.1.
In my practice, it is quite usable when debugging and analyzing
the protocol flow. HTTP/2 decision to drop it looks highly
questionable to me, and generally reflects the fact that HTTP/2 is
not designed for debugging.
> Personally I'd proceed with your approach for fastcgi module
> only, though I won't insist to patch other modules as well.
> Although this would introduce an extra condition in these
> other modules without a reason (as per my above analysis),
> this would keep sources in sync between them, which is fine,
> and will allow to stop using ngx_http_parse_status_line().
>
> For the latter, uwsgi seems to use HTTP syntax in response,
> so it is ok to call ngx_http_parse_status_line() here, but
> scgi seems to follows CGI syntax where status line is
> carried in the Status header field, so the call would
> always return NGX_ERROR. Luckily this is already handled
> by falling back to ngx_http_scgi_process_header().
SCGI does not define a response syntax at all, and at least
"Status: ..." and "HTTP/1.0 ..." were seen in the different
versions of the specification, see above. As such, I tend to
think that both variants needs to be (correctly) supported.
(On the other hand, for uwsgi I'm not sure if "Status: ..." is
allowed, it seems to contradict the protocol (see
https://uwsgi-docs.readthedocs.io/en/latest/Protocol.html, "Every
uwsgi request generates a response in the uwsgi format"). We
might consider removing Status header parsing in uwsgi, but this
is probably out of the scope of this issue.)
--
Maxim Dounin
http://mdounin.ru/
> On 31 Aug 2023, at 14:28, Maxim Dounin <mdounin at mdounin.ru> wrote:
>
> Hello!
>
> On Wed, Aug 30, 2023 at 04:20:15PM +0400, Sergey Kandaurov wrote:
>
>>> On 29 Aug 2023, at 08:33, Maxim Dounin <mdounin at mdounin.ru> wrote:
>>>
>>> On Mon, Aug 28, 2023 at 08:59:28PM +0400, Sergey Kandaurov wrote:
>>>
>>>>> On 26 Aug 2023, at 18:21, Jérémy Lal <kapouer at melix.org> 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 ?
>>>>>
>>>>
>>>> Note that the response line with empty reason phrase
>>>> is properly generated since nginx 1.5.6.
>>>> The exception remains is proxying FastCGI responses
>>>> as there is no distinguished response line in CGI syntax.
>>>>
>>>> The reason is that Status is a CGI header field, and hence
>>>> it is parsed by a generic routine that cuts trailing spaces.
>>>> But it can have a trailing space per RFC 3875, section 6.3.3.
>>>> So it needs a special treatment to preserve SP before empty reason
>>>> phrase. The below patch should help; although it doesn't look
>>>> efficient and can be polished, I think this is quite enough for
>>>> valid use cases.
>>>
>>> I very much doubt that RFC 3875 properly defines whitespace
>>> handling, see my response to the original report. In this
>>> particular case, it seems to define a header which cannot be
>>> correctly parsed if reason-phrase is empty.
>>>
>>
>> Yes, it is quite dubious how this can be parsed correctly.
>> Although it is valid to have a trailing space in Status,
>> this contradicts to header field value syntax per RFC 3875:
>> field-content = *( token | separator | quoted-string )
>
> Note that field-value syntax does no apply to the "Status" header,
> its syntax is defined separately.
Well, per RFC 3875 BNF, Status is CGI-field, which if generalized
to other-field, consists of field-content.
>
> On the other hand, the "Status" header syntax does not allow any
> spaces after the colon, which rules out headers like "Status: 200 OK",
> and makes the "Status" header syntax highly questionable.
Again, if generalize the "Status" header to header-field, then
there's a note about whitespace between the ":" and the field-value.
>
> As already suggested in my response to the original report, I tend
> to think that the best available option is to ignore RFC 3875 idea
> about headers syntax, and use HTTP instead.
I don't mind.
[..]
>>>
>>> (Also note that uwsgi and scgi modules needs to be patched
>>> similarly.)
>>
>> These modules use a distinct routine to parse status line,
>> that is ngx_http_parse_status_line, which respects spaces,
>> and they expect the first line to look like a status line
>> (similar to the proxy module).
>> So I don't see immediate necessity to patch them as well.
>
> Sure, but these are distinct parsing modes: parsing status line as
> in raw HTTP responses (which, in particular, are used by NPH scripts),
> or parsing the Status header, as in CGI responses. Both should
> work correctly.
>
> In particular, current SCGI specification
> (https://python.ca/scgi/protocol.txt) uses "Status: ..." in the
> example (whereas original version used "HTTP/1.0 ...",
> https://web.archive.org/web/20020403050958/http://python.ca/nas/scgi/protocol.txt).
>
Ahh ok, didn't know that.
That explains why all three modules should be patches.
> I see no reasons why "Status: 404 " should work correctly in
> FastCGI, but shouldn't in SCGI.
>
>>>> Alternatively, the reason-phrase value from FastCGI response
>>>> can be ignored, nginx will translate it with its own value:
>>>>
>>>> # HG changeset patch
>>>> # User Sergey Kandaurov <pluknet at nginx.com>
>>>> # Date 1693241054 -14400
>>>> # Mon Aug 28 20:44:14 2023 +0400
>>>> # Node ID f1a452cbd57ff4fba1caf3f36cb3624bd45411ea
>>>> # Parent 2880f60a80c3e2706151dc7b48dc1267e39c47a9
>>>> FastCGI: preserved SP for empty Status header reason phrase.
>>>>
>>>> As per RFC 3875 sec 6.3.3, the status code is always followed by SP.
>>>> The header is parsed by the common routine, and it can be challenging
>>>> to properly translate empty reason phrase to HTTP/1.1 status line, as
>>>> that would require an additional memory allocation. For simplicity,
>>>> the reason phrase is now ignored; the header filter will take care of
>>>> it by inserting its own reason phrase for well known status codes.
>>>>
>>>> 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,6 @@ ngx_http_fastcgi_process_header(ngx_http
>>>> }
>>>>
>>>> u->headers_in.status_n = status;
>>>> - u->headers_in.status_line = *status_line;
>>>>
>>>> } else if (u->headers_in.location) {
>>>> u->headers_in.status_n = 302;
>>>
>>> I don't think it's a good idea, since this always drops the
>>> reason phrase provided by the upstream server. It can contain
>>> some meaningful information which will be lost as a result, most
>>> notably for non-standard error codes.
>>
>> It is little to no benefit to carry reason phrase,
>> also this is consistent with HTTP/1.1:
>> : A client SHOULD ignore the reason-phrase content because
>> : it is not a reliable channel for information
>>
>> Note also that HTTP/2 and then HTTP/3 do not define a way
>> to carry the reason phrase that is included in HTTP/1.1.
>
> In my practice, it is quite usable when debugging and analyzing
> the protocol flow. HTTP/2 decision to drop it looks highly
> questionable to me, and generally reflects the fact that HTTP/2 is
> not designed for debugging.
>
>> Personally I'd proceed with your approach for fastcgi module
>> only, though I won't insist to patch other modules as well.
>> Although this would introduce an extra condition in these
>> other modules without a reason (as per my above analysis),
>> this would keep sources in sync between them, which is fine,
>> and will allow to stop using ngx_http_parse_status_line().
>>
>> For the latter, uwsgi seems to use HTTP syntax in response,
>> so it is ok to call ngx_http_parse_status_line() here, but
>> scgi seems to follows CGI syntax where status line is
>> carried in the Status header field, so the call would
>> always return NGX_ERROR. Luckily this is already handled
>> by falling back to ngx_http_scgi_process_header().
>
> SCGI does not define a response syntax at all, and at least
> "Status: ..." and "HTTP/1.0 ..." were seen in the different
> versions of the specification, see above. As such, I tend to
> think that both variants needs to be (correctly) supported.
>
Sure, it makes sense then to keep both variants.
> (On the other hand, for uwsgi I'm not sure if "Status: ..." is
> allowed, it seems to contradict the protocol (see
> https://uwsgi-docs.readthedocs.io/en/latest/Protocol.html, "Every
> uwsgi request generates a response in the uwsgi format"). We
> might consider removing Status header parsing in uwsgi, but this
> is probably out of the scope of this issue.)
--
Sergey Kandaurov
> On 29 Aug 2023, at 08:14, Maxim Dounin <mdounin at 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 at 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
Hello!
On Thu, Aug 31, 2023 at 03:45:06PM +0400, Sergey Kandaurov wrote:
> > On 31 Aug 2023, at 14:28, Maxim Dounin <mdounin at mdounin.ru> wrote:
> >
> > Hello!
> >
> > On Wed, Aug 30, 2023 at 04:20:15PM +0400, Sergey Kandaurov wrote:
> >
> >>> On 29 Aug 2023, at 08:33, Maxim Dounin <mdounin at mdounin.ru> wrote:
> >>>
> >>> On Mon, Aug 28, 2023 at 08:59:28PM +0400, Sergey Kandaurov wrote:
> >>>
> >>>>> On 26 Aug 2023, at 18:21, Jérémy Lal <kapouer at melix.org> 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 ?
> >>>>>
> >>>>
> >>>> Note that the response line with empty reason phrase
> >>>> is properly generated since nginx 1.5.6.
> >>>> The exception remains is proxying FastCGI responses
> >>>> as there is no distinguished response line in CGI syntax.
> >>>>
> >>>> The reason is that Status is a CGI header field, and hence
> >>>> it is parsed by a generic routine that cuts trailing spaces.
> >>>> But it can have a trailing space per RFC 3875, section 6.3.3.
> >>>> So it needs a special treatment to preserve SP before empty reason
> >>>> phrase. The below patch should help; although it doesn't look
> >>>> efficient and can be polished, I think this is quite enough for
> >>>> valid use cases.
> >>>
> >>> I very much doubt that RFC 3875 properly defines whitespace
> >>> handling, see my response to the original report. In this
> >>> particular case, it seems to define a header which cannot be
> >>> correctly parsed if reason-phrase is empty.
> >>>
> >>
> >> Yes, it is quite dubious how this can be parsed correctly.
> >> Although it is valid to have a trailing space in Status,
> >> this contradicts to header field value syntax per RFC 3875:
> >> field-content = *( token | separator | quoted-string )
> >
> > Note that field-value syntax does no apply to the "Status" header,
> > its syntax is defined separately.
>
> Well, per RFC 3875 BNF, Status is CGI-field, which if generalized
> to other-field, consists of field-content.
Syntax is as follows
(https://datatracker.ietf.org/doc/html/rfc3875#section-6.3):
header-field = CGI-field | other-field
CGI-field = Content-Type | Location | Status
other-field = protocol-field | extension-field
protocol-field = generic-field
extension-field = generic-field
generic-field = field-name ":" [ field-value ] NL
CGI-field and other-field are mutually exclusive variants of
header-field, and there is no generalization.
Generalization was present in the original RFC draft
(https://datatracker.ietf.org/doc/html/draft-robinson-www-interface-00#section-9.2),
but was removed during work on RFC 3875 (in
https://datatracker.ietf.org/doc/html/draft-coar-cgi-v11-03). Not
sure about the reasons, but might be because Status indeed does not
conform to the generic-field syntax, and this was an attempt to
fix it.
Either way, the fact is: RFC 3875 does not correctly specify
whitespace handling, and shouldn't be relied upon.
[...]
--
Maxim Dounin
http://mdounin.ru/
Hello!
On Thu, Aug 31, 2023 at 03:45:18PM +0400, Sergey Kandaurov wrote:
> > On 29 Aug 2023, at 08:14, Maxim Dounin <mdounin at 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 at 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.
Pushed to http://mdounin.ru/hg/nginx, thanks for the review.
--
Maxim Dounin
http://mdounin.ru/