On Mon, May 17, 2021 at 09:44:00AM +0200, Florian Obser wrote: > I had send an improved diff to tech and forgot to post it here, too: > Subject: "httpd(8): don't try to chunk-encode an empty body" > > This delays sending of the http headers until we know if the body is > empty or not and then doesn't try to chunk encode an empty body. > I think this should cover all cases as long as the fcgi server is > conforming to specs. > > This will also prevent chunk encoding an e.g 200 response with > Content-Lenght: 0, which would not work well either before.
Yes, this is a better more generic fix. Still works as expected. ok stsp@ > diff --git httpd.h httpd.h > index b3a40b3af68..c4adfba232d 100644 > --- httpd.h > +++ httpd.h > @@ -300,6 +300,7 @@ struct fcgi_data { > int end; > int status; > int headersdone; > + int headerssent; > }; > > struct range { > diff --git server_fcgi.c server_fcgi.c > index 64a0e9d2abb..e1e9704c920 100644 > --- server_fcgi.c > +++ server_fcgi.c > @@ -114,6 +114,7 @@ server_fcgi(struct httpd *env, struct client *clt) > clt->clt_fcgi.toread = sizeof(struct fcgi_record_header); > clt->clt_fcgi.status = 200; > clt->clt_fcgi.headersdone = 0; > + clt->clt_fcgi.headerssent = 0; > > if (clt->clt_srvevb != NULL) > evbuffer_free(clt->clt_srvevb); > @@ -544,22 +545,20 @@ server_fcgi_read(struct bufferevent *bev, void *arg) > if (!clt->clt_fcgi.headersdone) { > clt->clt_fcgi.headersdone = > server_fcgi_getheaders(clt); > - if (clt->clt_fcgi.headersdone) { > - if (server_fcgi_header(clt, > - clt->clt_fcgi.status) > - == -1) { > - server_abort_http(clt, > - 500, > - "malformed fcgi " > - "headers"); > - return; > - } > - } > if (!EVBUFFER_LENGTH(clt->clt_srvevb)) > break; > } > /* FALLTHROUGH */ > case FCGI_END_REQUEST: > + if (clt->clt_fcgi.headersdone && > + !clt->clt_fcgi.headerssent) { > + if (server_fcgi_header(clt, > + clt->clt_fcgi.status) == -1) { > + server_abort_http(clt, 500, > + "malformed fcgi headers"); > + return; > + } > + } > if (server_fcgi_writechunk(clt) == -1) { > server_abort_http(clt, 500, > "encoding error"); > @@ -600,6 +599,8 @@ server_fcgi_header(struct client *clt, unsigned int code) > char tmbuf[32]; > struct kv *kv, *cl, key; > > + clt->clt_fcgi.headerssent = 1; > + > if (desc == NULL || (error = server_httperror_byid(code)) == NULL) > return (-1); > > @@ -615,6 +616,12 @@ server_fcgi_header(struct client *clt, unsigned int code) > if (kv_add(&resp->http_headers, "Server", HTTPD_SERVERNAME) == NULL) > return (-1); > > + if (clt->clt_fcgi.type == FCGI_END_REQUEST || > + EVBUFFER_LENGTH(clt->clt_srvevb) == 0) { > + /* Can't chunk encode an empty body. */ > + clt->clt_fcgi.chunked = 0; > + } > + > /* Set chunked encoding */ > if (clt->clt_fcgi.chunked) { > /* XXX Should we keep and handle Content-Length instead? */ > > > > > > >> diff --git server_fcgi.c server_fcgi.c > >> index 8d3b581568f..0ac80c27d11 100644 > >> --- server_fcgi.c > >> +++ server_fcgi.c > >> @@ -615,6 +615,10 @@ server_fcgi_header(struct client *clt, unsigned int > >> code) > >> if (kv_add(&resp->http_headers, "Server", HTTPD_SERVERNAME) == NULL) > >> return (-1); > >> > >> + /* we cannot chunk-encode no-content */ > >> + if (code == 204) > >> + clt->clt_fcgi.chunked = 0; > >> + > >> /* Set chunked encoding */ > >> if (clt->clt_fcgi.chunked) { > >> /* XXX Should we keep and handle Content-Length instead? */ > >> > >> > >> > >> > Cheerio, > >> > Chris > >> > > >> > >> -- > >> I'm not entirely sure you are real. > >> > >> > > > > -- > I'm not entirely sure you are real. >