[PATCH 0/4] httpd fixes

[PATCH 0/4] httpd fixes

[Eric Wong]

Could still use a bit more testing, but some progress is better
than none.

Eric Wong (4):
  t/httpd-unit: extra test for fencepost errors
  t/mwrap-httpd: better errors for local socket failures
  rproxy: fix for `plackup --path=/$PREFIX/' mounts
  httpd: fix leaks when using persistent connections

 lib/Devel/Mwrap/Rproxy.pm |  1 +
 mwrap_httpd.h             |  4 ++++
 t/httpd-unit.t            |  7 +++++++
 t/mwrap-httpd.t           | 19 +++++++++----------
 4 files changed, 21 insertions(+), 10 deletions(-)

[PATCH 1/4] t/httpd-unit: extra test for fencepost errors

[Eric Wong]

I'm terrible at this stuff :<
---
 t/httpd-unit.t | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/httpd-unit.t b/t/httpd-unit.t
index 049d5fc..30d1717 100644
--- a/t/httpd-unit.t
+++ b/t/httpd-unit.t
@@ -95,4 +95,11 @@ open STDERR, '>', $err;
 isnt(system(@vg, $exe, "socket_dir:$s"), 0, "listen dir on socket fails");
 like($end_err->(), qr/stat.*directory/, 'stat failure shown');
 
+# check for fencepost errors
+my $len = 108 - length("$tmp");
+$len -= length("\0//$TEST_PID.sock");
+my $max = "$tmp/".('x'x$len);
+is(system(@vg, $exe, "socket_dir:$max"), 0, "listen dir on max");
+isnt(system(@vg, $exe, "socket_dir:$max+"), 0, "listen dir too long");
+
 done_testing;

[PATCH 2/4] t/mwrap-httpd: better errors for local socket failures

[Eric Wong]

From: Eric Wong <normalperson@yhbt.net>

The syswrite() needs to be replaced by send(.*MSG_NOSIGNAL),
but I'm ignoring SIGPIPE anyways in case connect() somehow
triggers it.  In any case, I'm noticing the local socket
created in Perl (not curl) sometimes fails the first send/write
and/or fails the second send/write syscall.  Maybe this can
help diagnose things...
---
 t/mwrap-httpd.t | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/t/mwrap-httpd.t b/t/mwrap-httpd.t
index a1bf333..b7076de 100644
--- a/t/mwrap-httpd.t
+++ b/t/mwrap-httpd.t
@@ -37,16 +37,16 @@ my $cleanup = sub {
 END { $cleanup->() }
 
 my $sock = "$mwrap_tmp/$pid.sock";
-my $n = 10000;
-my $c;
 my %o = (Peer => $sock , Type => SOCK_STREAM);
-while (!$c && --$n > 0) {
-	$c = IO::Socket::UNIX->new(%o) or
-		select undef, undef, undef, 0.011;
+local $SIG{PIPE} = 'IGNORE';
+my $c;
+for (1..10000) {
+	last if -S $sock && ($c = IO::Socket::UNIX->new(%o));
+	select undef, undef, undef, 0.011;
 }
 ok(-S $sock, 'socket created');
 ok($c, 'socket connected');
-is(syswrite($c, 'GET'), 3, 'trickled 3 bytes');
+is(send($c, 'GET', MSG_NOSIGNAL), 3, 'trickled 3 bytes') or diag "send: $!";
 
 my $cout = "$mwrap_tmp/cout";
 my $rc = system(qw(curl -vsSf --unix-socket), $sock, '-o', $cout,
@@ -67,12 +67,11 @@ SKIP: {
 	is($rc, 0, 'curl /reset');
 };
 
-local $SIG{PIPE} = 'IGNORE';
 {
 	my $req = " /$pid/each/20000 HTTP/1.0\r\n\r\n";
-	is(syswrite($c, $req), length($req), 'wrote rest of response') or
-		diag "syswrite: $!";
-	my $x = do { local $/; <$c> } or diag "read: $!";
+	is(send($c, $req, MSG_NOSIGNAL), length($req),
+		'wrote rest of response') or diag "send: $!";
+	my $x = do { local $/; <$c> } or diag "readline: $!";
 	like($x, qr!</html>\n?\z!s, 'got complete HTML response');
 }
 

[PATCH 3/4] rproxy: fix for `plackup --path=/$PREFIX/' mounts

[Eric Wong]

Plack::App::URLMap (used by plackup --path=)
adjusts SCRIPT_NAME and PATH_INFO, but not REQUEST_URI.
We need to adjust REQUEST_URI since we don't want the
URI-unescaped PATH_INFO.
---
 lib/Devel/Mwrap/Rproxy.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/Devel/Mwrap/Rproxy.pm b/lib/Devel/Mwrap/Rproxy.pm
index 7955f55..811b503 100644
--- a/lib/Devel/Mwrap/Rproxy.pm
+++ b/lib/Devel/Mwrap/Rproxy.pm
@@ -56,6 +56,7 @@ sub list {
 sub call { # PSGI entry point
 	my ($self, $env) = @_;
 	my $uri = $env->{REQUEST_URI};
+	$uri =~ s!\A\Q$env->{SCRIPT_NAME}\E!!;
 	my $method = $env->{REQUEST_METHOD};
 	return list(@_) if $uri eq '/' && $method eq 'GET';
 

[PATCH 4/4] httpd: fix leaks when using persistent connections

[Eric Wong]

Persistent connections is probably not going to be a real use
case, but I may end up supporting them anyways.
---
 mwrap_httpd.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mwrap_httpd.h b/mwrap_httpd.h
index fe9f292..ef215c8 100644
--- a/mwrap_httpd.h
+++ b/mwrap_httpd.h
@@ -173,6 +173,9 @@ static enum mw_qev h1_send_flush(struct mw_h1 *h1)
 	struct mw_wbuf *wbuf = h1->wbuf;
 	struct msghdr mh = { 0 };
 
+	free(h1->h1r);
+	h1->h1r = NULL;
+
 	mh.msg_iov = wbuf->iov + wbuf->iov_written;
 	mh.msg_iovlen = wbuf->iov_nr;
 	do {
@@ -674,6 +677,7 @@ static enum mw_qev h1_parse_harder(struct mw_h1 *h1, struct mw_h1req *h1r)
 	char *end;
 	struct phr_header *hdr = h1r->hdr;
 
+	h1->prev_len = 0;
 	h1->has_input = 0;
 	h1->persist = h1r->minor_ver >= 1 ? 1 : 0;
 	h1->in_len = 0;

Re: [PATCH 1/4] t/httpd-unit: extra test for fencepost errors

[Eric Wong]

Eric Wong <e@80x24.org> wrote:
> +# check for fencepost errors
> +my $len = 108 - length("$tmp");

FreeBSD only uses sun_path[104] whereas Linux uses 108

diff --git a/t/httpd-unit.t b/t/httpd-unit.t
index 30d1717..768b2db 100644
--- a/t/httpd-unit.t
+++ b/t/httpd-unit.t
@@ -96,10 +96,16 @@ isnt(system(@vg, $exe, "socket_dir:$s"), 0, "listen dir on socket fails");
 like($end_err->(), qr/stat.*directory/, 'stat failure shown');
 
 # check for fencepost errors
-my $len = 108 - length("$tmp");
-$len -= length("\0//$TEST_PID.sock");
-my $max = "$tmp/".('x'x$len);
-is(system(@vg, $exe, "socket_dir:$max"), 0, "listen dir on max");
-isnt(system(@vg, $exe, "socket_dir:$max+"), 0, "listen dir too long");
+my $len;
+if ($^O eq 'linux') { $len = 108 }
+elsif ($^O eq 'freebsd') { $len = 104 }
+SKIP: {
+	skip "length unknown on $^O OS", 2 if !defined($len);
+	$len -= length("$tmp");
+	$len -= length("\0//$TEST_PID.sock");
+	my $max = "$tmp/".('x'x$len);
+	is(system(@vg, $exe, "socket_dir:$max"), 0, "listen dir on max");
+	isnt(system(@vg, $exe, "socket_dir:$max+"), 0, "listen dir too long");
+}
 
 done_testing;