Ross L Richardson([email protected]) on 2021.10.09 21:40:50 +1100:
> This relates to the earlier messages I sent to bugs@ in:
> https://marc.info/?t=163309376900001&r=1&w=2
>
> RFC 7231 [HTTP/1.1] section 4.3.2. "HEAD" states:
> The HEAD method is identical to GET except that the server MUST NOT
> send a message body in the response (i.e., the response terminates at
> the end of the header section).
>
> RFC 3875 [The Common Gateway Interface (CGI) Version 1.1] in
> section 4.3.2 HEAD states:
> The HEAD method requests the script to do sufficient processing to
> return the response header fields, without providing a response
> message-body. The script MUST NOT provide a response message-body
> for a HEAD request. If it does, then the server MUST discard the
> message-body when reading the response from the script.
>
> Therefore, a CGI script which sends a message body is violation of the CGI
> specification, but so is the server if it fails to elide the body.
>
>
> With httpd, we see (for example):
> ----
> $ printf "HEAD /cgi-bin/ftplist.cgi?dbversion=1
> HTTP/1.0\r\nHost:ftp.openbsd.org\r\n\r\n" \
> | nc -c ftp.openbsd.org https
> HTTP/1.0 200 OK
> Connection: close
> Content-type: text/plain
> Date: Fri, 01 Oct 2021 12:50:59 GMT
> Server: OpenBSD httpd
>
> https://mirror.aarnet.edu.au/pub/OpenBSD Canberra, Australia
> https://cdn.openbsd.org/pub/OpenBSD Fastly (CDN)
> https://cloudflare.cdn.openbsd.org/pub/OpenBSD Cloudflare (CDN)
> ...
> RND_BYTES=0xfe9832a3...
> ----
>
> So httpd isn't behaving correctly.
>
> The patch below is offered in the hope that it is a starting point for
> a proper solution. Whilst it solves the problem in a simple test case,
> I'm insufficiently familiar with the httpd code to know whether this is
> correct or sufficient!
Hi Ross,
your two diffs are a good start.
So here is a combined diff that does the following from your two patches:
* stop sending the content for head requests, even when its supplied by the
fcgi.
* If the client sends an empty body without a Content-Lenght:
do not add the Content-Lenght if it's a HEAD request.
If it's a HEAD request, the Content-Lenght should show the size of the
equivalent GET request, but we don't know how much that will be so
don't lie.
I think your interpretation of the RFCs is correct.
Additionally, when the fcgi supplies a Content-Length header, do not remove
it and set Transfer-Encoding: chunked. Instead, leave the Content-Lenght
header in place, as obviously the fcgi knows how much data will come.
I tested this with some handwritten cgi scripts and slowcgi and with
nextcloud/php-fcgi.
comments, oks?
diff --git usr.sbin/httpd/server_fcgi.c usr.sbin/httpd/server_fcgi.c
index 0f06f001a33..29a016eb456 100644
--- usr.sbin/httpd/server_fcgi.c
+++ usr.sbin/httpd/server_fcgi.c
@@ -559,6 +559,12 @@ server_fcgi_read(struct bufferevent *bev, void *arg)
return;
}
}
+ /* Don't send content for HEAD requests */
+ if (clt->clt_fcgi.headerssent &&
+ ((struct http_descriptor *)
+ clt->clt_descreq)->http_method
+ == HTTP_METHOD_HEAD)
+ return;
if (server_fcgi_writechunk(clt) == -1) {
server_abort_http(clt, 500,
"encoding error");
@@ -621,29 +627,32 @@ server_fcgi_header(struct client *clt, unsigned int code)
/* Can't chunk encode an empty body. */
clt->clt_fcgi.chunked = 0;
- /* But then we need a Content-Length... */
- key.kv_key = "Content-Length";
- if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
- if (kv_add(&resp->http_headers,
- "Content-Length", "0") == NULL)
- return (-1);
+ /* But then we need a Content-Length unless method is HEAD... */
+ if (desc->http_method != HTTP_METHOD_HEAD) {
+ key.kv_key = "Content-Length";
+ if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
+ if (kv_add(&resp->http_headers,
+ "Content-Length", "0") == NULL)
+ return (-1);
+ }
}
}
- /* Set chunked encoding */
+ /* Send chunked encoding header */
if (clt->clt_fcgi.chunked) {
- /* XXX Should we keep and handle Content-Length instead? */
+ /* but only if no Content-Length header is supplied */
key.kv_key = "Content-Length";
- if ((kv = kv_find(&resp->http_headers, &key)) != NULL)
- kv_delete(&resp->http_headers, kv);
-
- /*
- * XXX What if the FastCGI added some kind of Transfer-Encoding?
- * XXX like gzip, deflate or even "chunked"?
- */
- if (kv_add(&resp->http_headers,
- "Transfer-Encoding", "chunked") == NULL)
- return (-1);
+ if ((kv = kv_find(&resp->http_headers, &key)) != NULL) {
+ clt->clt_fcgi.chunked = 0;
+ } else {
+ /*
+ * XXX What if the FastCGI added some kind of
Transfer-Encoding?
+ * XXX like gzip, deflate or even "chunked"?
+ */
+ if (kv_add(&resp->http_headers,
+ "Transfer-Encoding", "chunked") == NULL)
+ return (-1);
+ }
}
/* Is it a persistent connection? */
