status-line trailing SP is missing ?
Maxim Dounin
mdounin at mdounin.ru
Tue Aug 29 04:14:58 UTC 2023
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/
More information about the nginx
mailing list