Ross L Richardson(open...@rlr.id.au) 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? */