Hi,

I still get a connection error from my Andriod Nextcloud client and Wordpress does not work correctly on Firefox.

Please see my steps below to ensure I have tested the correct thing.  (patches are attached as well).

Nextcloud and roundcubemail on Firefox work fine.

On Chrome, everything works as expected.

Just to make sure I did this correctly:

   1.  Extract clean 6.9 httpd source from 6.9/src.tar.gz
   2.  Apply patch (p1) from May 15 email from Florian with subject
   (Re: httpd(8): don't try to chunk-encode an empty body)
   3.  Apply patch (p2) from start of this email thread (httpd(8):
   fastcgi & Content-Length: 0)
   4.  Apply patch (p3) from this email thread (Matthias)
   5.  All patches applied cleanly
   6.  make
   7. make install
   restart and test




pcengine# patch < ../p1
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|k--- httpd.h
|+++ httpd.h
--------------------------
Patching file httpd.h using Plan A...
Hunk #1 succeeded at 300.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|diff --git server_fcgi.c server_fcgi.c
|index 64a0e9d2abb..e1e9704c920 100644
|--- server_fcgi.c
|+++ server_fcgi.c
--------------------------
Patching file server_fcgi.c using Plan A...
Hunk #1 succeeded at 114.
Hunk #2 succeeded at 545.
Hunk #3 succeeded at 599.
Hunk #4 succeeded at 616.
done
pcengine# patch < ../p2
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- server_fcgi.c
|+++ server_fcgi.c
--------------------------
Patching file server_fcgi.c using Plan A...
Hunk #1 succeeded at 636.
done
pcengine# patch < ../p3
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- usr.sbin/httpd/server_fcgi.c       Thu May 20 05:57:23 2021
|+++ usr.sbin/httpd/server_fcgi.c       Thu May 20 06:03:40 2021
--------------------------
Patching file server_fcgi.c using Plan A...
Hunk #1 succeeded at 620.
Hunk #2 succeeded at 642.
done

On 19/05/2021 11:41 p.m., Florian Obser wrote:
Yes, ugh, this is much better, thanks!
I'll wait for Steve to confirm that it fixes nextclown for him, too and
then I'll put it in.

On 2021-05-20 06:43 +02, Matthias Pressfreund <m...@fn.de> wrote:
Fix works for me, too. Thanks.

It now sets the "Content-Length: 0" header for ALL traffic that
is not chunk-encoded. But chunk-encoding may be disabled already
(e.g. for http/1.0). I'd therefore suggest to move the fix to where
the handling of zero-length bodies actually takes place.


--- usr.sbin/httpd/server_fcgi.c        Thu May 20 05:57:23 2021
+++ usr.sbin/httpd/server_fcgi.c        Thu May 20 06:03:40 2021
@@ -620,6 +620,12 @@
            EVBUFFER_LENGTH(clt->clt_srvevb) == 0) {
                /* Can't chunk encode an empty body. */
                clt->clt_fcgi.chunked = 0;
+               key.kv_key = "Content-Length";
+               if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
+                       if (kv_add(&resp->http_headers,
+                           "Content-Length", "0") == NULL)
+                               return (-1);
+               }
        }
/* Set chunked encoding */
@@ -636,13 +642,6 @@
                if (kv_add(&resp->http_headers,
                    "Transfer-Encoding", "chunked") == NULL)
                        return (-1);
-       } else {
-               key.kv_key = "Content-Length";
-               if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
-                       if (kv_add(&resp->http_headers,
-                           "Content-Length", "0") == NULL)
-                               return (-1);
-               }
        }
/* Is it a persistent connection? */


On 2021-05-19 20:44, Florian Obser wrote:
The whole point of using Transfer-Encoding: chunked for fastcgi was so
that we do not need to provide a Content-Length header if upstream
doesn't give us one. (We'd need to slurp in all the data ugh).

Now turns out that if we disable chunked encoding for zero sized bodies
some browsers are picky and want a Content-Length: 0 (Firefox, Safari)
or they'll just sit there and wait for the connection to close.

Problem reported by Matthias Pressfreund with wordpress.
Debugged with the help of weerd@ who pointed out that the problem is
actually browser dependent. From there it was pretty clear what the
problem was.

OK?

diff --git server_fcgi.c server_fcgi.c
index 31d7322e9f7..b9dc4f6fe04 100644
--- server_fcgi.c
+++ server_fcgi.c
@@ -636,6 +636,13 @@ server_fcgi_header(struct client *clt, unsigned int code)
                if (kv_add(&resp->http_headers,
                    "Transfer-Encoding", "chunked") == NULL)
                        return (-1);
+       } else {
+               key.kv_key = "Content-Length";
+               if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
+                       if (kv_add(&resp->http_headers,
+                           "Content-Length", "0") == NULL)
+                               return (-1);
+               }
        }
/* Is it a persistent connection? */


k--- 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? */
--- server_fcgi.c
+++ server_fcgi.c
@@ -636,6 +636,13 @@ server_fcgi_header(struct client *clt, unsigned int code)
                if (kv_add(&resp->http_headers,
                    "Transfer-Encoding", "chunked") == NULL)
                        return (-1);
+       } else {
+               key.kv_key = "Content-Length";
+               if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
+                       if (kv_add(&resp->http_headers,
+                           "Content-Length", "0") == NULL)
+                               return (-1);
+               }
        }
 
        /* Is it a persistent connection? */
--- usr.sbin/httpd/server_fcgi.c        Thu May 20 05:57:23 2021
+++ usr.sbin/httpd/server_fcgi.c        Thu May 20 06:03:40 2021
@@ -620,6 +620,12 @@
            EVBUFFER_LENGTH(clt->clt_srvevb) == 0) {
                /* Can't chunk encode an empty body. */
                clt->clt_fcgi.chunked = 0;
+               key.kv_key = "Content-Length";
+               if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
+                       if (kv_add(&resp->http_headers,
+                           "Content-Length", "0") == NULL)
+                               return (-1);
+               }
        }
 
        /* Set chunked encoding */
@@ -636,13 +642,6 @@
                if (kv_add(&resp->http_headers,
                    "Transfer-Encoding", "chunked") == NULL)
                        return (-1);
-       } else {
-               key.kv_key = "Content-Length";
-               if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
-                       if (kv_add(&resp->http_headers,
-                           "Content-Length", "0") == NULL)
-                               return (-1);
-               }
        }
 
        /* Is it a persistent connection? */

Reply via email to