On Thu, Aug 11, 2022 at 08:41:43PM +0200, Omar Polo wrote:
> On 2022/08/11 19:37:08 +0200, Claudio Jeker wrote:
> > Reading through the fcgi code of httpd I wonder if HEAD requests are
> > handled correctly. In server_fcgi_read() the function just returns when it
> > hits the HEAD case but I think this is not correct since it does not drain
> > the clt_srvevb buffer and it kind of ignores possible padding bytes.
>
> FWIW I was wondering the same but I think it's fine to `return' here.
> It's a small optimization: if we've reached FCGI_END_REQUEST there is
> no more to read and padding is very likely zero anyway. clt_srvevb is
> re-created on the next server_fcgi.
I would like to have this fixed anyway. Ideally the connection would be
reused (also httpd would benefit from multiplexing but first slowcgi needs
to support that properly).
> However, looking into this made me realize the diff I committed a few
> hours ago has a regression with HEAD requests: it prints the end
> marker when it shouldn't.
>
> > I think what the code should do is break out of the switch case and just
> > continue. This is still not quite right since httpd depends on the fcgi
> > server to close the connection (or it will do that on the next
> > sever_fcgi() call).
>
> diff /usr/src
> commit - acc148801b858e3177b9b43a93d20fd53f97d40d
> path + /usr/src
> blob - b6541b7c68235ac1dfc5a9d0243db988e5932a7f
> file + usr.sbin/httpd/server_fcgi.c
> --- usr.sbin/httpd/server_fcgi.c
> +++ usr.sbin/httpd/server_fcgi.c
> @@ -486,6 +486,7 @@ void
> server_fcgi_error(struct bufferevent *bev, short error, void *arg)
> {
> struct client *clt = arg;
> + struct http_descriptor *desc = clt->clt_descreq;
>
> if ((error & EVBUFFER_EOF) && !clt->clt_fcgi.headersdone) {
> server_abort_http(clt, 500, "malformed or no headers");
> @@ -493,7 +494,8 @@ server_fcgi_error(struct bufferevent *bev, short error
> }
>
> /* send the end marker if not already */
> - if (clt->clt_fcgi.chunked && !clt->clt_fcgi.end++)
> + if (desc->http_method != HTTP_METHOD_HEAD && clt->clt_fcgi.chunked &&
> + !clt->clt_fcgi.end++)
> server_bufferevent_print(clt, "0\r\n\r\n");
>
> server_file_error(bev, error, arg);
Indeed. HEAD requests are annoyingly special. OK claudio@
--
:wq Claudio
