Re: httpd(8): fastcgi & Content-Length: 0

2021-05-20 Thread Steve Williams

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  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.cThu May 20 05:57:23 2021
+++ usr.sbin/httpd/server_fcgi.cThu 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(>http_headers, )) == NULL) {
+   if (kv_add(>http_headers,
+   "Content-Length", "0") == NULL)
+   return (-1);
+   }
}
  
  	/* Set chunked encoding */

@@ -636,13 +642,6 @@
if (kv_add(>http_headers,
"Transfer-Encoding", "chunked") == NULL)
return (-1);
-   } else {
-   key.kv_key = "Content-Length";
-   if ((kv = kv_find(>http_headers, )) == NULL) {
-   if (kv_add(>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(>http_headers,
"Transfer-Encoding", "chunked") == NULL)
return (-1);
+   } else {
+   key.kv_key = "Content-Length";
+   if ((kv = kv_find(>http_headers, )) == NULL) {
+   if (kv_add(>http_headers,
+   

Re: httpd(8): fastcgi & Content-Length: 0

2021-05-19 Thread Steve Williams

Hi,

Now that I have my head out of my ass (php/redis issue), I can confirm 
that the Nextcloud Android client does NOT work with this latest patch 
for me.


I am getting the same "Connection Error" that I do with the stock httpd 
that's supplied in 6.9.  I am running Nextcloud Andriod client version 
3.16 released May 5, 2021.


Wordpress, Nextcloud, Piwigo (photo album) are all working in Chrome and 
Firefox.



I extracted /usr/src/usr.sbin/httpd

cd /usr/src/usr.sbin/httpd
# I copied & pasted the few lines from the email into a file "p"
patch < p
make
make install
#sledgehammer used below :)
rcctl restart httpd php74_fpm redis mysqld

If there's anything I can do to assist, please let me know.

Cheers,
Steve W.

On 19/05/2021 9:59 p.m., Steve Williams wrote:

Hi,

I applied this patch to the base OpenBSD 6.9 httpd source tree, 
recompiled & installed.


Wordpress works in both Firefox and Chrome, Roundcubemail works in 
both Firefox and Chrome.


However, my Andriod Nextcloud client is now broken again.  I get a 
"Connection Error".


Is there a way I can help you troubleshoot this?

There is nothing significant when I run a httpd -d -v -v -v -v -v

Thanks,
Steve Williams


On 19/05/2021 12:44 p.m., 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(>http_headers,
  "Transfer-Encoding", "chunked") == NULL)
  return (-1);
+    } else {
+    key.kv_key = "Content-Length";
+    if ((kv = kv_find(>http_headers, )) == NULL) {
+    if (kv_add(>http_headers,
+    "Content-Length", "0") == NULL)
+    return (-1);
+    }
  }
    /* Is it a persistent connection? */





Re: httpd(8): fastcgi & Content-Length: 0

2021-05-19 Thread Steve Williams
Please disregard.  It seems like I somehow broke my Nextcloud install 
cleaning up packages :(



On 19/05/2021 9:59 p.m., Steve Williams wrote:

Hi,

I applied this patch to the base OpenBSD 6.9 httpd source tree, 
recompiled & installed.


Wordpress works in both Firefox and Chrome, Roundcubemail works in 
both Firefox and Chrome.


However, my Andriod Nextcloud client is now broken again.  I get a 
"Connection Error".


Is there a way I can help you troubleshoot this?

There is nothing significant when I run a httpd -d -v -v -v -v -v

Thanks,
Steve Williams


On 19/05/2021 12:44 p.m., 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(>http_headers,
  "Transfer-Encoding", "chunked") == NULL)
  return (-1);
+    } else {
+    key.kv_key = "Content-Length";
+    if ((kv = kv_find(>http_headers, )) == NULL) {
+    if (kv_add(>http_headers,
+    "Content-Length", "0") == NULL)
+    return (-1);
+    }
  }
    /* Is it a persistent connection? */





Re: httpd(8): fastcgi & Content-Length: 0

2021-05-19 Thread Steve Williams

Hi,

I applied this patch to the base OpenBSD 6.9 httpd source tree, 
recompiled & installed.


Wordpress works in both Firefox and Chrome, Roundcubemail works in both 
Firefox and Chrome.


However, my Andriod Nextcloud client is now broken again.  I get a 
"Connection Error".


Is there a way I can help you troubleshoot this?

There is nothing significant when I run a httpd -d -v -v -v -v -v

Thanks,
Steve Williams


On 19/05/2021 12:44 p.m., 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(>http_headers,
"Transfer-Encoding", "chunked") == NULL)
return (-1);
+   } else {
+   key.kv_key = "Content-Length";
+   if ((kv = kv_find(>http_headers, )) == NULL) {
+   if (kv_add(>http_headers,
+   "Content-Length", "0") == NULL)
+   return (-1);
+   }
}
  
  	/* Is it a persistent connection? */






Re: httpd(8): don't try to chunk-encode an empty body

2021-05-15 Thread Steve Williams

Hi,

I can confirm this patch (from the 6.9 source) makes httpd work with 
Android Nextcloud client 3.16, where the unpatched httpd does not work.


Thanks very much!

Cheers,
Steve Williams


On 15/05/2021 9:14 a.m., Florian Obser wrote:

Turns out it's not that difficult to do this correctly since we already
wait until we read all http headers from the fcgi upstream. We just need
to delay writing of the http header until we know if the body is empty
or not.

OK?


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