[PATCH 4/7] examples: config no longer supports atomUrl

[Eric Wong]

We build the atomUrl from url, which can change
dynamically depending on what PSGI environment it
is called under.
---
 examples/public-inbox-config | 1 -
 1 file changed, 1 deletion(-)

diff --git a/examples/public-inbox-config b/examples/public-inbox-config
index 0c1db11..7fcbe0b 100644
--- a/examples/public-inbox-config
+++ b/examples/public-inbox-config
@@ -10,4 +10,3 @@
address = meta@public-inbox.org
mainrepo = /home/pi/meta-main.git
url = http://example.com/meta
-   atomUrl = http://example.com/meta
-- 
EW

--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 0/7] miscellaneous cleanups

[Eric Wong]

Only the last one (NewsGroup class removal for ::Inbox) is
likely to cause problems but I'll be checking logs for
errors.

Eric Wong (7):
  t/plack: ensure we can cascade on common endpoints
  http: clarify comments about layering violation
  Makefile.PL: allow N to be overridden
  examples: config no longer supports atomUrl
  www: remove footer_html support
  config: remove try_cat
  remove redundant NewsGroup class

 MANIFEST |  1 -
 Makefile.PL  |  2 +-
 examples/public-inbox-config |  1 -
 lib/PublicInbox/Config.pm| 49 +++---
 lib/PublicInbox/HTTP.pm  |  6 ++--
 lib/PublicInbox/Inbox.pm | 17 -
 lib/PublicInbox/NNTP.pm  |  9 ++---
 lib/PublicInbox/NNTPD.pm | 27 --
 lib/PublicInbox/NewsGroup.pm | 84 
 lib/PublicInbox/NewsWWW.pm   | 38 ++--
 lib/PublicInbox/WWW.pm   |  6 ++--
 t/config.t   |  2 ++
 t/nntp.t | 15 +---
 t/nntpd.t|  4 ++-
 t/plack.t| 12 +--
 15 files changed, 83 insertions(+), 190 deletions(-)

--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 1/7] t/plack: ensure we can cascade on common endpoints

[Eric Wong]

We don't serve things like robots.txt, favicon.ico, or
.well-known/ endpoints ourselves, but ensure we can be
used with Plack::App::Cascade for others.
---
 t/plack.t | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/t/plack.t b/t/plack.t
index 04680b2..a4f3245 100644
--- a/t/plack.t
+++ b/t/plack.t
@@ -62,16 +62,24 @@ EOF
require $psgi;
};
 
+   test_psgi($app, sub {
+   my ($cb) = @_;
+   foreach my $u (qw(robots.txt favicon.ico .well-known/foo)) {
+   my $res = $cb->(GET("http://example.com/$u;));
+   is($res->code, 404, "$u is missing");
+   }
+   });
+
# redirect with newsgroup
test_psgi($app, sub {
my ($cb) = @_;
my $from = 'http://example.com/inbox.test';
my $to = 'http://example.com/test/';
my $res = $cb->(GET($from));
-   is($res->code, 301, 'is permanent redirect');
+   is($res->code, 301, 'newsgroup name is permanent redirect');
is($to, $res->header('Location'), 'redirect location matches');
$from .= '/';
-   is($res->code, 301, 'is permanent redirect');
+   is($res->code, 301, 'newsgroup name/ is permanent redirect');
is($to, $res->header('Location'), 'redirect location matches');
});
 
-- 
EW

--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH] config: fix NewsWWW fallback for newsgroups in HTTP URLs

[Eric Wong]

Oops, added a test to prevent regressions while we're at it.
---
 lib/PublicInbox/Config.pm  |  4 +++-
 lib/PublicInbox/NewsWWW.pm |  3 ++-
 t/plack.t  | 15 +++
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 935b044..35b24af 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -56,7 +56,8 @@ sub lookup_name {
my ($self, $name) = @_;
my $rv = $self->{-by_name}->{$name};
return $rv if $rv;
-   $self->{-by_name}->{$name} = _fill($self, "publicinbox.$name");
+   $rv = _fill($self, "publicinbox.$name") or return;
+   $self->{-by_name}->{$name} = $rv;
 }
 
 sub get {
@@ -118,6 +119,7 @@ sub _fill {
my $v = $self->{"$pfx.$k"};
$rv->{$k} = $v if defined $v;
}
+   return unless $rv->{mainrepo};
my $inbox = $pfx;
$inbox =~ s/\Apublicinbox\.//;
$rv->{name} = $inbox;
diff --git a/lib/PublicInbox/NewsWWW.pm b/lib/PublicInbox/NewsWWW.pm
index 19eb596..5357059 100644
--- a/lib/PublicInbox/NewsWWW.pm
+++ b/lib/PublicInbox/NewsWWW.pm
@@ -30,7 +30,6 @@ sub call {
if (my $info = $ng_map->{$ng}) {
my $url = PublicInbox::Hval::prurl($env, $info->{url});
my $code = 301;
-   my $h = [ Location => $url, 'Content-Type' => 'text/plain' ];
if (defined $article && $article =~ /\A\d+\z/) {
my $mid = eval { ng_mid_for($ng, $info, $article) };
if (defined $mid) {
@@ -41,6 +40,8 @@ sub call {
}
}
 
+   my $h = [ Location => $url, 'Content-Type' => 'text/plain' ];
+
return [ $code, $h, [ "Redirecting to $url\n" ] ]
}
[ 404, [ 'Content-Type' => 'text/plain' ], [] ];
diff --git a/t/plack.t b/t/plack.t
index c8dd7bf..04680b2 100644
--- a/t/plack.t
+++ b/t/plack.t
@@ -30,6 +30,8 @@ foreach my $mod (@mods) { use_ok $mod; }
my %cfg = (
"$cfgpfx.address" => $addr,
"$cfgpfx.mainrepo" => $maindir,
+   "$cfgpfx.url" => 'http://example.com/test/',
+   "$cfgpfx.newsgroup" => 'inbox.test',
);
while (my ($k,$v) = each %cfg) {
is(0, system(qw(git config --file), $pi_config, $k, $v),
@@ -60,6 +62,19 @@ EOF
require $psgi;
};
 
+   # redirect with newsgroup
+   test_psgi($app, sub {
+   my ($cb) = @_;
+   my $from = 'http://example.com/inbox.test';
+   my $to = 'http://example.com/test/';
+   my $res = $cb->(GET($from));
+   is($res->code, 301, 'is permanent redirect');
+   is($to, $res->header('Location'), 'redirect location matches');
+   $from .= '/';
+   is($res->code, 301, 'is permanent redirect');
+   is($to, $res->header('Location'), 'redirect location matches');
+   });
+
# redirect with trailing /
test_psgi($app, sub {
my ($cb) = @_;
--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 1/2] git-http-backend: move real close to GetlineBody

[Eric Wong]

This makes more sense as it keeps management of rpipe
nice and neat.
---
 lib/PublicInbox/GetlineBody.pm| 12 
 lib/PublicInbox/GitHTTPBackend.pm |  1 -
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/lib/PublicInbox/GetlineBody.pm b/lib/PublicInbox/GetlineBody.pm
index 4f8765b..5f32782 100644
--- a/lib/PublicInbox/GetlineBody.pm
+++ b/lib/PublicInbox/GetlineBody.pm
@@ -13,19 +13,23 @@ sub new {
bless { rpipe => $rpipe, end => $end, buf => $buf }, $class;
 }
 
+# close should always be called after getline returns undef,
+# but a client aborting a connection can ruin our day; so lets
+# hope our underlying PSGI server does not leak references, here.
 sub DESTROY { $_[0]->close }
 
 sub getline {
my ($self) = @_;
-   my $buf = delete $self->{buf};
+   my $buf = delete $self->{buf}; # initial buffer
defined $buf ? $buf : $self->{rpipe}->getline;
 }
 
 sub close {
my ($self) = @_;
-   delete $self->{rpipe};
-   my $end = delete $self->{end} or return;
-   $end->();
+   my $rpipe = delete $self->{rpipe};
+   close $rpipe if $rpipe;
+   my $end = delete $self->{end};
+   $end->() if $end;
 }
 
 1;
diff --git a/lib/PublicInbox/GitHTTPBackend.pm 
b/lib/PublicInbox/GitHTTPBackend.pm
index fd7afbc..1819444 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -186,7 +186,6 @@ sub serve_smart {
my $x = PublicInbox::Qspawn->new([qw(git http-backend)], \%env, \%rdr);
my ($fh, $rpipe);
my $end = sub {
-   close $rpipe if $rpipe && !$fh; # generic PSGI
if (my $err = $x->finish) {
err($env, "git http-backend ($git_dir): $err");
drop_client($env);
--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 2/2] git-http-backend: close pipe for generic PSGI on errors

[Eric Wong]

The generic PSGI code needs to avoid resource leaks if
smart cloning is disabled (due to resource contraints).
---
 lib/PublicInbox/GitHTTPBackend.pm | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/GitHTTPBackend.pm 
b/lib/PublicInbox/GitHTTPBackend.pm
index 1819444..58e75cd 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -210,7 +210,12 @@ sub serve_smart {
my $r = $rd_hdr->() or return;
$rd_hdr = undef;
if (scalar(@$r) == 3) { # error:
-   $async->close if $async;
+   if ($async) {
+   $async->close; # calls rpipe->close
+   } else {
+   $rpipe->close;
+   $end->();
+   }
return $res->($r);
}
if ($async) {
--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 4/3] httpd/async: do not needlessly weaken

[Eric Wong]

The restart_read callback has no chance of circular reference,
and weakening $self before we create it can cause $self to
be undefined inside the callback (seen during stress testing).

Fixes: 395406118cb2 ("httpd/async: prevent circular reference")
---
 lib/PublicInbox/HTTPD/Async.pm | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index b00e637..add07ce 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -24,14 +24,19 @@ sub new {
$self;
 }
 
+sub restart_read_cb ($) {
+   my ($self) = @_;
+   sub { $self->watch_read(1) }
+}
+
 sub async_pass {
my ($self, $io, $fh, $bref) = @_;
# In case the client HTTP connection ($io) dies, it
# will automatically close this ($self) object.
$io->{forward} = $self;
$fh->write($$bref);
+   my $restart_read = restart_read_cb($self);
weaken($self);
-   my $restart_read = sub { $self->watch_read(1) };
$self->{cb} = sub {
my $r = sysread($self->{sock}, $$bref, 8192);
if ($r) {
--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 1/3] httpd/async: prevent circular reference

[Eric Wong]

We must avoid circular references which can cause leaks in
long-running processes.  This callback is dangerous since
it may never be called to properly terminate everything.
---
 lib/PublicInbox/HTTPD/Async.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/PublicInbox/HTTPD/Async.pm b/lib/PublicInbox/HTTPD/Async.pm
index 47ba27d..b00e637 100644
--- a/lib/PublicInbox/HTTPD/Async.pm
+++ b/lib/PublicInbox/HTTPD/Async.pm
@@ -10,6 +10,7 @@ use strict;
 use warnings;
 use base qw(Danga::Socket);
 use fields qw(cb cleanup);
+use Scalar::Util qw(weaken);
 require PublicInbox::EvCleanup;
 
 sub new {
@@ -25,11 +26,12 @@ sub new {
 
 sub async_pass {
my ($self, $io, $fh, $bref) = @_;
-   my $restart_read = sub { $self->watch_read(1) };
# In case the client HTTP connection ($io) dies, it
# will automatically close this ($self) object.
$io->{forward} = $self;
$fh->write($$bref);
+   weaken($self);
+   my $restart_read = sub { $self->watch_read(1) };
$self->{cb} = sub {
my $r = sysread($self->{sock}, $$bref, 8192);
if ($r) {
--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 2/3] http: avoid circular reference for getline responses

[Eric Wong]

Lightly tested, this seems to work when mass-aborting
responses.  Will still need to automate the testing...
---
 lib/PublicInbox/HTTP.pm | 45 -
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 6aae5c8..0454f60 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -11,11 +11,12 @@ package PublicInbox::HTTP;
 use strict;
 use warnings;
 use base qw(Danga::Socket);
-use fields qw(httpd env rbuf input_left remote_addr remote_port forward);
+use fields qw(httpd env rbuf input_left remote_addr remote_port forward pull);
 use Fcntl qw(:seek);
 use Plack::HTTPParser qw(parse_http_request); # XS or pure Perl
 use HTTP::Status qw(status_message);
 use HTTP::Date qw(time2str);
+use Scalar::Util qw(weaken);
 use IO::File;
 use constant {
CHUNK_START => -1,   # [a-f0-9]+\r\n
@@ -255,6 +256,28 @@ sub response_done ($$) {
}
 }
 
+sub getline_response {
+   my ($self, $body, $write, $close) = @_;
+   $self->{forward} = $body;
+   weaken($self);
+   my $pull = $self->{pull} = sub {
+   local $/ = \8192;
+   my $forward = $self->{forward};
+   while ($forward && defined(my $buf = $forward->getline)) {
+   $write->($buf);
+   last if $self->{closed};
+   if ($self->{write_buf_size}) {
+   $self->write($self->{pull});
+   return;
+   }
+   }
+   $self->{forward} = $self->{pull} = undef;
+   $forward->close if $forward; # avoid recursion
+   $close->();
+   };
+   $pull->();
+}
+
 sub response_write {
my ($self, $env, $res) = @_;
my $alive = response_header_write($self, $env, $res);
@@ -266,21 +289,7 @@ sub response_write {
$write->($_) foreach @$body;
$close->();
} else {
-   my $pull;
-   $pull = sub {
-   local $/ = \8192;
-   while (defined(my $buf = $body->getline)) {
-   $write->($buf);
-   if ($self->{write_buf_size}) {
-   $self->write($pull);
-   return;
-   }
-   }
-   $pull = undef;
-   $body->close();
-   $close->();
-   };
-   $pull->();
+   getline_response($self, $body, $write, $close);
}
} else {
# this is returned to the calling application:
@@ -445,8 +454,10 @@ sub event_err { $_[0]->close }
 sub close {
my $self = shift;
my $forward = $self->{forward};
+   my $env = $self->{env};
+   delete $env->{'psgix.io'} if $env; # prevent circular referernces
+   $self->{pull} = $self->{forward} = $self->{env} = undef;
$forward->close if $forward;
-   $self->{forward} = $self->{env} = undef;
$self->SUPER::close(@_);
 }
 
--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 3/3] git-http-backend: fix aborts for generic PSGI clone

[Eric Wong]

We need to avoid circular references in the generic PSGI layer,
do it by abusing DESTROY.
---
 lib/PublicInbox/GetlineBody.pm| 31 +++
 lib/PublicInbox/GitHTTPBackend.pm | 13 -
 2 files changed, 35 insertions(+), 9 deletions(-)
 create mode 100644 lib/PublicInbox/GetlineBody.pm

diff --git a/lib/PublicInbox/GetlineBody.pm b/lib/PublicInbox/GetlineBody.pm
new file mode 100644
index 000..4f8765b
--- /dev/null
+++ b/lib/PublicInbox/GetlineBody.pm
@@ -0,0 +1,31 @@
+# Copyright (C) 2016 all contributors 
+# License: AGPL-3.0+ 
+
+# Wrap a pipe or file for PSGI streaming response bodies and calls the
+# end callback when the object goes out-of-scope.
+# This depends on rpipe being _blocking_ on getline.
+package PublicInbox::GetlineBody;
+use strict;
+use warnings;
+
+sub new {
+   my ($class, $rpipe, $end, $buf) = @_;
+   bless { rpipe => $rpipe, end => $end, buf => $buf }, $class;
+}
+
+sub DESTROY { $_[0]->close }
+
+sub getline {
+   my ($self) = @_;
+   my $buf = delete $self->{buf};
+   defined $buf ? $buf : $self->{rpipe}->getline;
+}
+
+sub close {
+   my ($self) = @_;
+   delete $self->{rpipe};
+   my $end = delete $self->{end} or return;
+   $end->();
+}
+
+1;
diff --git a/lib/PublicInbox/GitHTTPBackend.pm 
b/lib/PublicInbox/GitHTTPBackend.pm
index 9464cb4..fd7afbc 100644
--- a/lib/PublicInbox/GitHTTPBackend.pm
+++ b/lib/PublicInbox/GitHTTPBackend.pm
@@ -186,7 +186,7 @@ sub serve_smart {
my $x = PublicInbox::Qspawn->new([qw(git http-backend)], \%env, \%rdr);
my ($fh, $rpipe);
my $end = sub {
-   $rpipe = undef;
+   close $rpipe if $rpipe && !$fh; # generic PSGI
if (my $err = $x->finish) {
err($env, "git http-backend ($git_dir): $err");
drop_client($env);
@@ -201,7 +201,7 @@ sub serve_smart {
my $r = sysread($rpipe, $buf, 1024, length($buf));
return if !defined($r) && ($!{EINTR} || $!{EAGAIN});
return r(500, 'http-backend error') unless $r;
-   $r = parse_cgi_headers(\$buf) or return;
+   $r = parse_cgi_headers(\$buf) or return; # incomplete headers
$r->[0] == 403 ? serve_dumb($cgi, $git, $path) : $r;
};
my $res;
@@ -220,13 +220,8 @@ sub serve_smart {
}
 
# for synchronous PSGI servers
-   $r->[2] = Plack::Util::inline_object(
-   close => $end,
-   getline => sub {
-   my $ret = $buf;
-   $buf = undef;
-   defined $ret ? $ret : $rpipe->getline;
-   });
+   require PublicInbox::GetlineBody;
+   $r->[2] = PublicInbox::GetlineBody->new($rpipe, $end, $buf);
$res->($r);
};
sub {
--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/

[PATCH 0/3] http: another round EPIPE fixes

[Eric Wong]

Hopefully this is end of resource leaks on prematurely aborted
client connections.

Eric Wong (3):
  httpd/async: prevent circular reference
  http: avoid circular reference for getline responses
  git-http-backend: fix aborts for generic PSGI clone

 lib/PublicInbox/GetlineBody.pm| 31 +++
 lib/PublicInbox/GitHTTPBackend.pm | 13 ---
 lib/PublicInbox/HTTP.pm   | 45 ---
 lib/PublicInbox/HTTPD/Async.pm|  4 +++-
 4 files changed, 66 insertions(+), 27 deletions(-)

--
unsubscribe: meta+unsubscr...@public-inbox.org
archive: https://public-inbox.org/meta/