From 5a7cb2b148a35dd1d3f272c6aacb7b960adb58b6 Mon Sep 17 00:00:00 2001
From: florian <florian@openbsd.org>
Date: Mon, 17 May 2021 09:26:52 +0000
Subject: [PATCH] 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
---
 usr.sbin/httpd/httpd.h       |  3 ++-
 usr.sbin/httpd/server_fcgi.c | 31 +++++++++++++++++++------------
 2 files changed, 21 insertions(+), 13 deletions(-)

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? */
-- 
2.20.1