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.
>
