From: florian <florian@openbsd.org>
Date: Mon, 17 May 2021 09:26:52 +0000 (+0000)
Subject: Do not try to chunk encode an empty http body coming from an fcgi
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=5a7cb2b148a35dd1d3f272c6aacb7b960adb58b6;p=openbsd

Do not try to chunk encode an empty http body coming from an fcgi
upstream.

Found the hard way by Chris Narkiewicz who tracked failing uploads in
the nextcloud mobile app down to httpd(8) trying to chunk encode a
"204 No Content" resonse.

Testing by Steve Williams
Testing & OK stsp
---

diff --git a/usr.sbin/httpd/httpd.h b/usr.sbin/httpd/httpd.h
index b3a40b3af68..4df7de216c2 100644
--- a/usr.sbin/httpd/httpd.h
+++ b/usr.sbin/httpd/httpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: httpd.h,v 1.156 2021/04/20 21:11:56 dv Exp $	*/
+/*	$OpenBSD: httpd.h,v 1.157 2021/05/17 09:26:52 florian Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -300,6 +300,7 @@ struct fcgi_data {
 	int			 end;
 	int			 status;
 	int			 headersdone;
+	int			 headerssent;
 };
 
 struct range {
diff --git a/usr.sbin/httpd/server_fcgi.c b/usr.sbin/httpd/server_fcgi.c
index eb142add48c..31d7322e9f7 100644
--- a/usr.sbin/httpd/server_fcgi.c
+++ b/usr.sbin/httpd/server_fcgi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_fcgi.c,v 1.85 2021/05/15 15:08:31 florian Exp $	*/
+/*	$OpenBSD: server_fcgi.c,v 1.86 2021/05/17 09:26:52 florian Exp $	*/
 
 /*
  * Copyright (c) 2014 Florian Obser <florian@openbsd.org>
@@ -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? */