On 2021-05-17 09:12 +02, Stefan Sperling <s...@stsp.name> wrote:
> On Fri, May 14, 2021 at 07:07:53PM +0200, Florian Obser wrote:
>> oh, it's 204 no content. I missed that.
>> The demo url (apparently running appache) doesn't return a
>> Content-Lenght, either.
>>
>> Anyway, trying to chunk encode an empty body is not going to work well.
>>
>> This is making the spice flow again:
>
> I confirm that this fixes upload from the nextcloud app for me as well.
> There were also some occasional connection issues while browsing my
> nextcloud instance with firefox. I'll have to wait and see if those are
> now fixed. Server is running stock 6.9 with the patch below.
>
> Should we also be including 1xx codes in the check added here?
> In server_abort_http() there is this:
> if ((code >= 100 && code < 200) || code == 204)
> clenheader = NULL;
> Which matches https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.1
> A server MUST NOT send a Transfer-Encoding header field in any
> response with a status code of 1xx (Informational) or 204 (No Content).
>
> In any case, ok stsp@
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.
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.
