unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/6] flesh out more -netd funcionality
@ 2022-08-01 21:24 Eric Wong
  2022-08-01 21:24 ` [PATCH 1/6] httpd: make internals slightly more generic Eric Wong
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Eric Wong @ 2022-08-01 21:24 UTC (permalink / raw)
  To: meta

These changes will allow public-inbox-netd to host multiple,
completely-unrelated .psgi apps within the same process via
psgi= as a per-listener option.  Having separate stdout/stderr
facsimiles is also supported via err= and out= keys (HTTP(S)
only has err= for $env->{'psgi.errors'}).

(public-inbox-{nntp,imap,pop3,http}d can actually do all that
-netd can do, too, the only difference is -netd has no default
port/protocol).

Further optimizations (PublicInbox::Config object sharing)
and reload improvements (TLS cert reload on SIGHUP) are on
the way...

Eric Wong (6):
  httpd: make internals slightly more generic
  daemon: support per-listener env, .psgi, out, err
  daemon: require absolute cert/key paths with --daemonize
  daemon: add diagnostics about inherited/bound listeners
  daemon: allow listening on well-known ports based on protocol
  daemon: share FDs for identical log paths

 Documentation/public-inbox-daemon.pod |  51 ++++++--
 Documentation/public-inbox-netd.pod   |  34 ++++--
 MANIFEST                              |   1 +
 lib/PublicInbox/Daemon.pm             | 168 +++++++++++++++++---------
 lib/PublicInbox/HTTP.pm               |  10 +-
 lib/PublicInbox/HTTPD.pm              |  60 +++++----
 lib/PublicInbox/IMAPD.pm              |   3 +-
 lib/PublicInbox/NNTPD.pm              |  25 ++--
 lib/PublicInbox/POP3D.pm              |  36 +++---
 t/alt.psgi                            |  17 +++
 t/httpd-corner.psgi                   |   8 +-
 t/httpd-corner.t                      |  39 +++++-
 12 files changed, 304 insertions(+), 148 deletions(-)
 create mode 100644 t/alt.psgi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] httpd: make internals slightly more generic
  2022-08-01 21:24 [PATCH 0/6] flesh out more -netd funcionality Eric Wong
@ 2022-08-01 21:24 ` Eric Wong
  2022-08-01 21:24 ` [PATCH 2/6] daemon: support per-listener env, .psgi, out, err Eric Wong
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2022-08-01 21:24 UTC (permalink / raw)
  To: meta

This brings the HTTP server closer to the IMAP/NNTP/POP3
implementations and eliminates package-wide globals in
PublicInbox::HTTPD.  The end goal is to be able to host
completely different PSGI applications on different listen
ports.
---
 lib/PublicInbox/Daemon.pm | 10 +++----
 lib/PublicInbox/HTTP.pm   | 10 +++----
 lib/PublicInbox/HTTPD.pm  | 55 ++++++++++++++++++++++-----------------
 3 files changed, 41 insertions(+), 34 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index bceae6e5..1af03cc4 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -81,11 +81,11 @@ sub load_mod ($) {
 	my $mod = $modc.'D';
 	eval "require $mod"; # IMAPD|HTTPD|NNTPD|POP3D
 	die $@ if $@;
-	my %xn = map { $_ => $mod->can($_) } qw(refresh post_accept);
-	$xn{tlsd} = $mod->new if $mod->can('refresh_groups'); #!HTTPD
-	my $tlsd = $xn{tlsd};
-	$xn{refresh} //= sub { $tlsd->refresh_groups(@_) };
-	$xn{post_accept} //= sub { $modc->new($_[0], $tlsd) };
+	my %xn;
+	my $tlsd = $xn{tlsd} = $mod->new;
+	$xn{refresh} = sub { $tlsd->refresh_groups(@_) };
+	$xn{post_accept} = $tlsd->can('post_accept_cb') ?
+			$tlsd->post_accept_cb : sub { $modc->new($_[0], $tlsd) };
 	$xn{af_default} = 'httpready' if $modc eq 'PublicInbox::HTTP';
 	\%xn;
 }
diff --git a/lib/PublicInbox/HTTP.pm b/lib/PublicInbox/HTTP.pm
index 76e978a2..669211e3 100644
--- a/lib/PublicInbox/HTTP.pm
+++ b/lib/PublicInbox/HTTP.pm
@@ -1,4 +1,4 @@
-# Copyright (C) 2016-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 #
 # Generic PSGI server for convenience.  It aims to provide
@@ -52,8 +52,8 @@ sub http_date () {
 }
 
 sub new ($$$) {
-	my ($class, $sock, $addr, $httpd) = @_;
-	my $self = bless { httpd => $httpd }, $class;
+	my ($class, $sock, $addr, $srv_env) = @_;
+	my $self = bless { srv_env => $srv_env }, $class;
 	my $ev = EPOLLIN;
 	my $wbuf;
 	if ($sock->can('accept_SSL') && !$sock->accept_SSL) {
@@ -78,7 +78,7 @@ sub event_step { # called by PublicInbox::DS
 	return read_input($self) if ref($self->{env});
 
 	my $rbuf = $self->{rbuf} // (\(my $x = ''));
-	my %env = %{$self->{httpd}->{env}}; # full hash copy
+	my %env = %{$self->{srv_env}}; # full hash copy
 	my $r;
 	while (($r = parse_http_request($$rbuf, \%env)) < 0) {
 		# We do not support Trailers in chunked requests, for
@@ -145,7 +145,7 @@ sub app_dispatch {
 	# note: NOT $self->{sock}, we want our close (+ PublicInbox::DS::close),
 	# to do proper cleanup:
 	$env->{'psgix.io'} = $self; # for ->close or async_pass
-	my $res = Plack::Util::run_app($self->{httpd}->{app}, $env);
+	my $res = Plack::Util::run_app($env->{'pi-httpd.app'}, $env);
 	eval {
 		if (ref($res) eq 'CODE') {
 			$res->(sub { response_write($self, $env, $_[0]) });
diff --git a/lib/PublicInbox/HTTPD.pm b/lib/PublicInbox/HTTPD.pm
index 715e4538..bcdbb9f9 100644
--- a/lib/PublicInbox/HTTPD.pm
+++ b/lib/PublicInbox/HTTPD.pm
@@ -13,12 +13,17 @@ use PublicInbox::HTTPD::Async;
 
 sub pi_httpd_async { PublicInbox::HTTPD::Async->new(@_) }
 
-sub new {
-	my ($class, $sock, $app, $client) = @_;
-	my $n = getsockname($sock) or die "not a socket: $sock $!\n";
-	my ($host, $port) = PublicInbox::Daemon::host_with_port($n);
+# we have a different env for ever listener socket for
+# SERVER_NAME, SERVER_PORT and psgi.url_scheme
+# envs: listener FD => PSGI env
+sub new { bless { envs => {} }, __PACKAGE__ }
 
-	my %env = (
+# this becomes {srv_env} in PublicInbox::HTTP
+sub env_for ($$$) {
+	my ($self, $srv, $client) = @_;
+	my $n = getsockname($srv) or die "not a socket: $srv $!\n";
+	my ($host, $port) = PublicInbox::Daemon::host_with_port($n);
+	{
 		SERVER_NAME => $host,
 		SERVER_PORT => $port,
 		SCRIPT_NAME => '',
@@ -40,26 +45,24 @@ sub new {
 		# this to limit git-http-backend(1) parallelism.
 		# We also check for the truthiness of this to
 		# detect when to use async paths for slow blobs
-		'pi-httpd.async' => \&pi_httpd_async
-	);
-	bless { app => $app, env => \%env }, $class;
+		'pi-httpd.async' => \&pi_httpd_async,
+		'pi-httpd.app' => $self->{app},
+	}
 }
 
-my %httpds; # per-listen-FD mapping for HTTPD->{env}->{SERVER_<NAME|PORT>}
-my $default_app; # ugh...
-
-sub refresh {
+sub refresh_groups {
+	my ($self) = @_;
+	my $app;
 	if (@main::ARGV) {
-		eval { $default_app = Plack::Util::load_psgi(@ARGV) };
-		if ($@) {
-			die $@,
-"$0 runs in /, command-line paths must be absolute\n";
-		}
+		eval { $app = Plack::Util::load_psgi(@ARGV) };
+		die $@, <<EOM if $@;
+$0 runs in /, command-line paths must be absolute
+EOM
 	} else {
 		require PublicInbox::WWW;
 		my $www = PublicInbox::WWW->new;
 		$www->preload;
-		$default_app = builder {
+		$app = builder {
 			eval { enable 'ReverseProxy' };
 			$@ and warn <<EOM;
 Plack::Middleware::ReverseProxy missing,
@@ -69,14 +72,18 @@ EOM
 			sub { $www->call(@_) };
 		};
 	}
-	%httpds = (); # invalidate cache
+	$_->{'pi-httpd.app'} = $app for values %{$self->{envs}};
+	$self->{app} = $app;
 }
 
-sub post_accept { # Listener->{post_accept}
-	my ($client, $addr, $srv) = @_; # $_[3] - tls_wrap (unused)
-	my $httpd = $httpds{fileno($srv)} //=
-				__PACKAGE__->new($srv, $default_app, $client);
-	PublicInbox::HTTP->new($client, $addr, $httpd),
+sub post_accept_cb { # for Listener->{post_accept}
+	my ($self) = @_;
+	sub {
+		my ($client, $addr, $srv) = @_; # $_[4] - tls_wrap (unused)
+		PublicInbox::HTTP->new($client, $addr,
+				$self->{envs}->{fileno($srv)} //=
+					env_for($self, $srv, $client));
+	}
 }
 
 1;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/6] daemon: support per-listener env, .psgi, out, err
  2022-08-01 21:24 [PATCH 0/6] flesh out more -netd funcionality Eric Wong
  2022-08-01 21:24 ` [PATCH 1/6] httpd: make internals slightly more generic Eric Wong
@ 2022-08-01 21:24 ` Eric Wong
  2022-08-01 21:24 ` [PATCH 3/6] daemon: require absolute cert/key paths with --daemonize Eric Wong
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2022-08-01 21:24 UTC (permalink / raw)
  To: meta

This allows memory savings by allowing multiple, completely
unrelated-PSGI apps to run within the same process as IMAP,
NNTP, and POP3.
---
 Documentation/public-inbox-daemon.pod |  51 +++++++++--
 Documentation/public-inbox-netd.pod   |  34 +++++--
 MANIFEST                              |   1 +
 lib/PublicInbox/Daemon.pm             | 124 ++++++++++++++++----------
 lib/PublicInbox/HTTPD.pm              |   9 +-
 lib/PublicInbox/IMAPD.pm              |   3 +-
 lib/PublicInbox/NNTPD.pm              |  25 +++---
 lib/PublicInbox/POP3D.pm              |  36 ++++----
 t/alt.psgi                            |  17 ++++
 t/httpd-corner.psgi                   |   8 +-
 t/httpd-corner.t                      |  39 +++++++-
 11 files changed, 239 insertions(+), 108 deletions(-)
 create mode 100644 t/alt.psgi

diff --git a/Documentation/public-inbox-daemon.pod b/Documentation/public-inbox-daemon.pod
index f77fc3a9..5d26ce56 100644
--- a/Documentation/public-inbox-daemon.pod
+++ b/Documentation/public-inbox-daemon.pod
@@ -4,16 +4,18 @@ public-inbox-daemon - common usage for public-inbox network daemons
 
 =head1 SYNOPSIS
 
+	public-inbox-netd
 	public-inbox-httpd
 	public-inbox-imapd
 	public-inbox-nntpd
+	public-inbox-pop3d
 
 =head1 DESCRIPTION
 
 This manual describes common options and behavior for
 public-inbox network daemons.  Network daemons for public-inbox
-provide read-only NNTP, IMAP and HTTP access to public-inboxes.  Write
-access to a public-inbox will never be required to run these.
+provide read-only IMAP, HTTP, NNTP and POP3 access to public-inboxes.
+Write access to a public-inbox will never be required to run these.
 
 These daemons are implemented with a common core using
 non-blocking sockets and optimized for fairness; even with
@@ -29,9 +31,9 @@ processes to take advantage of multiple CPUs.
 
 =over
 
-=item -l ADDRESS
+=item -l [PROTO://]ADDRESS[?opt1=val1,opt2=val2]
 
-=item --listen ADDRESS
+=item --listen [PROTO://]ADDRESS[?opt1=val1,opt2=val2]
 
 This takes an absolute path to a Unix socket or HOST:PORT
 to listen on.  For example, to listen to TCP connections on
@@ -42,8 +44,14 @@ like L<nginx(8)> to use.
 May be specified multiple times to allow listening on multiple
 sockets.
 
-This does not need to be specified at all if relying on
-L<systemd.socket(5)> or similar
+Unless per-listener options are used (required for
+L<public-inbox-netd(1)>), this does not need to be specified at
+all if relying on L<systemd.socket(5)> or similar,
+
+Per-listener options may be specified after C<?> as C<KEY=VALUE>
+pairs delimited by C<,>.  See L<public-inbox-netd(1)> for
+documentation on the C<cert=>, C<key=>, C<env.NAME=VALUE>,
+C<out=>, C<err=>, and C<psgi=> options available.
 
 Default: server-dependent unless socket activation is used with
 L<systemd(1)> or similar (see L<systemd.socket(5)>).
@@ -57,7 +65,9 @@ Using this is preferable to setting up the redirect externally
 (e.g. E<gt>E<gt>/path/to/log in shell) since it allows
 SIGUSR1 to be handled (see L<SIGNALS/SIGNALS> below).
 
-Default: /dev/null
+C<out=> may also be specified on a per-listener basis.
+
+Default: /dev/null with C<--daemonize>, inherited otherwise
 
 =item -2 PATH
 
@@ -65,6 +75,10 @@ Default: /dev/null
 
 Like C<--stdout>, but for the stderr descriptor (2).
 
+C<err=> may also be specified on a per-listener basis.
+
+Default: /dev/null with C<--daemonize>, inherited otherwise
+
 =item -W
 
 =item --worker-processes
@@ -82,6 +96,25 @@ the master on crashes.
 
 Default: 1
 
+=item --cert /path/to/cert
+
+The default TLS certificate for HTTPS, IMAPS, NNTPS, POP3S and/or STARTTLS
+support if the C<cert> option is not given with C<--listen>.
+
+Well-known TCP ports automatically get TLS or STARTTLS support
+If using systemd-compatible socket activation and a TCP listener
+on port well-known ports (563 is inherited, it is automatically
+NNTPS when this option is given.  When a listener on port 119 is
+inherited and this option is given, it automatically gets
+STARTTLS support.
+
+=item --key /path/to/key
+
+The default TLS certificate key for the default C<--cert> or
+per-listener C<cert=> option.  The private key may be
+concatenated into the path used by the cert, in which case this
+option is not needed.
+
 =back
 
 =head1 SIGNALS
@@ -183,11 +216,11 @@ L<http://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/meta/>
 
 =head1 COPYRIGHT
 
-Copyright 2013-2021 all contributors L<mailto:meta@public-inbox.org>
+Copyright all contributors L<mailto:meta@public-inbox.org>
 
 License: AGPL-3.0+ L<https://www.gnu.org/licenses/agpl-3.0.txt>
 
 =head1 SEE ALSO
 
 L<public-inbox-httpd(1)>, L<public-inbox-imapd(1)>,
-L<public-inbox-nntpd(1)>
+L<public-inbox-nntpd(1)>, L<public-inbox-pop3d(1)>, L<public-inbox-netd(1)>
diff --git a/Documentation/public-inbox-netd.pod b/Documentation/public-inbox-netd.pod
index dcf4d5b0..4dc27749 100644
--- a/Documentation/public-inbox-netd.pod
+++ b/Documentation/public-inbox-netd.pod
@@ -8,9 +8,10 @@ public-inbox-netd - read-only network daemon for sharing public-inboxes
 
 =head1 DESCRIPTION
 
-public-inbox-netd provides a read-only HTTP/IMAP/NNTP/POP3 daemon for
-public-inbox.  It uses options and environment variables common
-to all L<public-inbox-daemon(8)> implementations.
+public-inbox-netd provides a read-only multi-protocol
+(HTTP/IMAP/NNTP/POP3) daemon for public-inbox.  It uses options
+and environment variables common to all
+L<public-inbox-daemon(8)> implementations.
 
 The default configuration will never require write access
 to the directory where the public-inbox is stored, so it
@@ -28,21 +29,34 @@ See common options in L<public-inbox-daemon(8)/OPTIONS>.
 
 =item --listen PROTO://ADDRESS/?cert=/path/to/cert,key=/path/to/key
 
+=item -l http://ADDRESS/?env.PI_CONFIG=/path/to/cfg,psgi=/path/to/app.psgi
+
 In addition to the normal C<-l>/C<--listen> switch described in
 L<public-inbox-daemon(8)>, the protocol prefix (e.g. C<nntp://> or
 C<nntps://>) may be specified to force a given protocol.
 
+Environment variable overrides in effect during loading and
+reloading (SIGHUP) can be specified as C<env.NAME=VALUE> for
+all protocols.
+
+HTTP(S) listeners may also specify C<psgi=> to use a different
+C<.psgi> file for each listener.
+
+C<err=/path/to/errors.log> may be used to isolate error/debug output
+for a particular listener away from C<--stderr>.
+
+Non-HTTP(S) listeners may also specify C<out=> for logging to
+C<stdout>.  HTTP(S) users are encouraged to configure
+L<Plack::Middleware::AccessLog> or
+L<Plack::Middleware::AccessLog::Timed>, instead.
+
 =item --cert /path/to/cert
 
-The default TLS certificate for optional TLS support
-if the C<cert> option is not given with C<--listen>.
+See L<public-inbox-daemon(1)>.
 
 =item --key /path/to/key
 
-The default private TLS certificate key for optional TLS support
-if the C<key> option is not given with C<--listen>.  The private
-key may be concatenated into the path used by C<--cert>, in which case this
-option is not needed.
+See L<public-inbox-daemon(1)>.
 
 =back
 
@@ -57,6 +71,8 @@ L<public-inbox-config(5)>.
 
 =item publicinbox.nntpserver
 
+=item publicinbox.pop3state
+
 =back
 
 See L<public-inbox-config(5)> for documentation on them.
diff --git a/MANIFEST b/MANIFEST
index dac6875e..e5880cc3 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -390,6 +390,7 @@ scripts/xhdr-num2mid
 t/.gitconfig
 t/address.t
 t/admin.t
+t/alt.psgi
 t/altid.t
 t/altid_v2.t
 t/cgi.t
diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 1af03cc4..0392d15f 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -10,6 +10,7 @@ use v5.10.1;
 use Getopt::Long qw(:config gnu_getopt no_ignore_case auto_abbrev);
 use IO::Handle; # ->autoflush
 use IO::Socket;
+use File::Spec;
 use POSIX qw(WNOHANG :signal_h);
 use Socket qw(IPPROTO_TCP SOL_SOCKET);
 STDOUT->autoflush(1);
@@ -27,7 +28,7 @@ my ($set_user, $oldset);
 my (@cfg_listen, $stdout, $stderr, $group, $user, $pid_file, $daemonize);
 my $worker_processes = 1;
 my @listeners;
-my %pids;
+my (%pids, %logs);
 my %tls_opt; # scheme://sockname => args for IO::Socket::SSL->start_SSL
 my $reexec_pid;
 my ($uid, $gid);
@@ -35,24 +36,31 @@ my ($default_cert, $default_key);
 my %KNOWN_TLS = (443 => 'https', 563 => 'nntps', 993 => 'imaps', 995 =>'pop3s');
 my %KNOWN_STARTTLS = (110 => 'pop3', 119 => 'nntp', 143 => 'imap');
 
-sub accept_tls_opt ($) {
-	my ($opt_str) = @_;
-	# opt_str: opt1=val1,opt2=val2 (opt may repeat for multi-value)
-	require PublicInbox::TLS;
+sub listener_opt ($) {
+	my ($str) = @_; # opt1=val1,opt2=val2 (opt may repeat for multi-value)
 	my $o = {};
 	# allow ',' as delimiter since '&' is shell-unfriendly
-	foreach (split(/[,&]/, $opt_str)) {
+	for (split(/[,&]/, $str)) {
 		my ($k, $v) = split(/=/, $_, 2);
-		push @{$o->{$k} ||= []}, $v;
+		push @{$o->{$k}}, $v;
 	}
 
 	# key may be a part of cert.  At least
 	# p5-io-socket-ssl/example/ssl_server.pl has this fallback:
-	$o->{cert} //= [ $default_cert ];
+	$o->{cert} //= [ $default_cert ] if defined($default_cert);
 	$o->{key} //= defined($default_key) ? [ $default_key ] : $o->{cert};
+	$o;
+}
+
+sub accept_tls_opt ($) {
+	my ($opt) = @_;
+	my $o = ref($opt) eq 'HASH' ? $opt : listener_opt($opt);
+	return if !defined($o->{cert});
+	require PublicInbox::TLS;
 	my %ctx_opt = (SSL_server => 1);
 	# parse out hostname:/path/to/ mappings:
-	foreach my $k (qw(cert key)) {
+	for my $k (qw(cert key)) {
+		$o->{$k} // next;
 		my $x = $ctx_opt{'SSL_'.$k.'_file'} = {};
 		foreach my $path (@{$o->{$k}}) {
 			my $host = '';
@@ -75,18 +83,61 @@ sub accept_tls_opt ($) {
 	{ SSL_server => 1, SSL_startHandshake => 0, SSL_reuse_ctx => $ctx };
 }
 
-sub load_mod ($) {
-	my ($scheme) = @_;
+sub check_absolute ($$) {
+	my ($var, $val) = @_;
+	die <<EOM if index($val // '/', '/') != 0;
+$var must be an absolute path when using --daemonize: $val
+EOM
+}
+
+sub do_chown ($) {
+	$uid // return;
+	my ($path) = @_;
+	chown($uid, $gid, $path) or warn "chown $path: $!\n";
+}
+
+sub open_log_path ($$) { # my ($fh, $path) = @_; # $_[0] is modified
+	open $_[0], '>>', $_[1] or die "open(>> $_[1]): $!";
+	$_[0]->autoflush(1);
+	do_chown($_[1]);
+}
+
+sub load_mod ($;$) {
+	my ($scheme, $opt) = @_;
 	my $modc = "PublicInbox::\U$scheme";
 	my $mod = $modc.'D';
 	eval "require $mod"; # IMAPD|HTTPD|NNTPD|POP3D
 	die $@ if $@;
 	my %xn;
 	my $tlsd = $xn{tlsd} = $mod->new;
-	$xn{refresh} = sub { $tlsd->refresh_groups(@_) };
+	my %env = map {
+		substr($_, length('env.')) => $opt->{$_}->[-1];
+	} grep(/\Aenv\./, keys %$opt);
+	$xn{refresh} = sub {
+		my ($sig) = @_;
+		local @ENV{keys %env} = values %env;
+		$tlsd->refresh_groups($sig);
+	};
 	$xn{post_accept} = $tlsd->can('post_accept_cb') ?
 			$tlsd->post_accept_cb : sub { $modc->new($_[0], $tlsd) };
-	$xn{af_default} = 'httpready' if $modc eq 'PublicInbox::HTTP';
+	my @paths = qw(out err);
+	if ($modc eq 'PublicInbox::HTTP') {
+		@paths = qw(err);
+		$xn{af_default} = 'httpready';
+		if (my $p = $opt->{psgi}) {
+			die "multiple psgi= options specified\n" if @$p > 1;
+			check_absolute('psgi=', $p->[0]) if $daemonize;
+			$tlsd->{psgi} = $p->[0];
+		}
+	}
+	for my $f (@paths) {
+		my $p = $opt->{$f} or next;
+		die "multiple $f= options specified\n" if @$p > 1;
+		check_absolute("$f=", $p->[0]) if $daemonize;
+		$p = File::Spec->canonpath($p->[0]);
+		open_log_path(my $fh, $p);
+		$tlsd->{$f} = $logs{$p} = $fh;
+	}
 	\%xn;
 }
 
@@ -125,6 +176,7 @@ EOF
 	GetOptions(%opt) or die $help;
 	if ($show_help) { print $help; exit 0 };
 
+	$_ = File::Spec->canonpath($_ // next) for ($stdout, $stderr);
 	if (defined $pid_file && $pid_file =~ /\.oldbin\z/) {
 		die "--pid-file cannot end with '.oldbin'\n";
 	}
@@ -151,15 +203,17 @@ EOF
 			my $s = $KNOWN_TLS{$1} // $KNOWN_STARTTLS{$1};
 			$scheme = $s if defined $s;
 		}
+		my $opt; # non-TLS options
 		if ($l =~ s!/?\?(.+)\z!!) {
-			$tls_opt{"$scheme://$l"} = accept_tls_opt($1);
+			$opt = listener_opt($1);
+			$tls_opt{"$scheme://$l"} = accept_tls_opt($opt);
 		} elsif (defined($default_cert)) {
 			$tls_opt{"$scheme://$l"} = accept_tls_opt('');
 		} elsif ($scheme =~ /\A(?:https|imaps|nntps|pop3s)\z/) {
 			die "$orig specified w/o cert=\n";
 		}
 		$scheme =~ /\A(http|imap|nntp|pop3)/ and
-			$xnetd->{$l} = load_mod($1);
+			$xnetd->{$l} = load_mod($1, $opt);
 
 		next if $listener_names->{$l}; # already inherited
 		my (%o, $sock_pkg);
@@ -212,18 +266,12 @@ EOF
 			$tls_opt{''} ||= accept_tls_opt('');
 		}
 	}
-
+	my @d;
+	while (my ($k, $v) = each %tls_opt) { push(@d, $k) if !defined($v) }
+	delete @tls_opt{@d};
 	die "No listeners bound\n" unless @listeners;
 }
 
-sub check_absolute ($$) {
-	my ($var, $val) = @_;
-	if (defined $val && index($val, '/') != 0) {
-		die
-"--$var must be an absolute path when using --daemonize: $val\n";
-	}
-}
-
 sub daemonize () {
 	if ($daemonize) {
 		require Cwd;
@@ -232,9 +280,9 @@ sub daemonize () {
 			next unless -e $arg;
 			$ARGV[$i] = Cwd::abs_path($arg);
 		}
-		check_absolute('stdout', $stdout);
-		check_absolute('stderr', $stderr);
-		check_absolute('pid-file', $pid_file);
+		check_absolute('--stdout', $stdout);
+		check_absolute('--stderr', $stderr);
+		check_absolute('--pid-file', $pid_file);
 
 		chdir '/' or die "chdir failed: $!";
 	}
@@ -317,18 +365,9 @@ sub worker_quit { # $_[0] = signal name or number (unused)
 }
 
 sub reopen_logs {
-	if ($stdout) {
-		open STDOUT, '>>', $stdout or
-			warn "failed to redirect stdout to $stdout: $!\n";
-		STDOUT->autoflush(1);
-		do_chown($stdout);
-	}
-	if ($stderr) {
-		open STDERR, '>>', $stderr or
-			warn "failed to redirect stderr to $stderr: $!\n";
-		STDERR->autoflush(1);
-		do_chown($stderr);
-	}
+	$logs{$stdout} //= \*STDOUT if defined $stdout;
+	$logs{$stderr} //= \*STDERR if defined $stderr;
+	while (my ($p, $fh) = each %logs) { open_log_path($fh, $p) }
 }
 
 sub sockname ($) {
@@ -688,13 +727,6 @@ sub run {
 	# ->DESTROY runs when $for_destroy goes out-of-scope
 }
 
-sub do_chown ($) {
-	my ($path) = @_;
-	if (defined $uid and !chown($uid, $gid, $path)) {
-		warn "could not chown $path: $!\n";
-	}
-}
-
 sub write_pid ($) {
 	my ($path) = @_;
 	Net::Server::Daemonize::create_pid_file($path);
diff --git a/lib/PublicInbox/HTTPD.pm b/lib/PublicInbox/HTTPD.pm
index bcdbb9f9..e531ee70 100644
--- a/lib/PublicInbox/HTTPD.pm
+++ b/lib/PublicInbox/HTTPD.pm
@@ -16,7 +16,7 @@ sub pi_httpd_async { PublicInbox::HTTPD::Async->new(@_) }
 # we have a different env for ever listener socket for
 # SERVER_NAME, SERVER_PORT and psgi.url_scheme
 # envs: listener FD => PSGI env
-sub new { bless { envs => {} }, __PACKAGE__ }
+sub new { bless { envs => {}, err => \*STDERR }, __PACKAGE__ }
 
 # this becomes {srv_env} in PublicInbox::HTTP
 sub env_for ($$$) {
@@ -28,7 +28,7 @@ sub env_for ($$$) {
 		SERVER_PORT => $port,
 		SCRIPT_NAME => '',
 		'psgi.version' => [ 1, 1 ],
-		'psgi.errors' => \*STDERR,
+		'psgi.errors' => $self->{err},
 		'psgi.url_scheme' => $client->can('accept_SSL') ?
 					'https' : 'http',
 		'psgi.nonblocking' => Plack::Util::TRUE,
@@ -53,8 +53,9 @@ sub env_for ($$$) {
 sub refresh_groups {
 	my ($self) = @_;
 	my $app;
-	if (@main::ARGV) {
-		eval { $app = Plack::Util::load_psgi(@ARGV) };
+	$self->{psgi} //= $main::ARGV[0] if @main::ARGV;
+	if ($self->{psgi}) {
+		eval { $app = Plack::Util::load_psgi($self->{psgi}) };
 		die $@, <<EOM if $@;
 $0 runs in /, command-line paths must be absolute
 EOM
diff --git a/lib/PublicInbox/IMAPD.pm b/lib/PublicInbox/IMAPD.pm
index b24097a2..9a5bdcfe 100644
--- a/lib/PublicInbox/IMAPD.pm
+++ b/lib/PublicInbox/IMAPD.pm
@@ -1,8 +1,7 @@
 # Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
-# represents an IMAPD (currently a singleton),
-# see script/public-inbox-imapd for how it is used
+# represents an IMAPD, see script/public-inbox-imapd for how it is used
 package PublicInbox::IMAPD;
 use strict;
 use v5.10.1;
diff --git a/lib/PublicInbox/NNTPD.pm b/lib/PublicInbox/NNTPD.pm
index f31d4381..9e232ef6 100644
--- a/lib/PublicInbox/NNTPD.pm
+++ b/lib/PublicInbox/NNTPD.pm
@@ -13,21 +13,10 @@ use PublicInbox::NNTP;
 
 sub new {
 	my ($class) = @_;
-	my $pi_cfg = PublicInbox::Config->new;
-	my $name = $pi_cfg->{'publicinbox.nntpserver'};
-	if (!defined($name) or $name eq '') {
-		$name = hostname;
-	} elsif (ref($name) eq 'ARRAY') {
-		$name = $name->[0];
-	}
-
 	bless {
-		groups => {},
 		err => \*STDERR,
 		out => \*STDOUT,
-		pi_cfg => $pi_cfg,
-		servername => $name,
-		greet => \"201 $name ready - post via email\r\n",
+		# pi_cfg => $pi_cfg,
 		# accept_tls => { SSL_server => 1, ..., SSL_reuse_ctx => ... }
 		# idler => PublicInbox::InboxIdle
 	}, $class;
@@ -35,7 +24,17 @@ sub new {
 
 sub refresh_groups {
 	my ($self, $sig) = @_;
-	my $pi_cfg = $sig ? PublicInbox::Config->new : $self->{pi_cfg};
+	my $pi_cfg = PublicInbox::Config->new;
+	my $name = $pi_cfg->{'publicinbox.nntpserver'};
+	if (!defined($name) or $name eq '') {
+		$name = hostname;
+	} elsif (ref($name) eq 'ARRAY') {
+		$name = $name->[0];
+	}
+	if ($name ne ($self->{servername} // '')) {
+		$self->{servername} = $name;
+		$self->{greet} = \"201 $name ready - post via email\r\n";
+	}
 	my $groups = $pi_cfg->{-by_newsgroup}; # filled during each_inbox
 	my $cache = eval { $pi_cfg->ALL->misc->nntpd_cache_load } // {};
 	$pi_cfg->each_inbox(sub {
diff --git a/lib/PublicInbox/POP3D.pm b/lib/PublicInbox/POP3D.pm
index 0609627e..5cfe9613 100644
--- a/lib/PublicInbox/POP3D.pm
+++ b/lib/PublicInbox/POP3D.pm
@@ -1,7 +1,7 @@
 # Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 
-# represents an POP3D (currently a singleton)
+# represents an POP3D
 package PublicInbox::POP3D;
 use v5.12;
 use parent qw(PublicInbox::Lock);
@@ -37,20 +37,12 @@ if ($^O eq 'linux' || $^O eq 'freebsd') {
 	die "File::FcntlLock required for POP3 on $^O: $@\n";
 
 sub new {
-	my ($cls, $pi_cfg) = @_;
-	$pi_cfg //= PublicInbox::Config->new;
-	my $d = $pi_cfg->{'publicinbox.pop3state'} //
-		die "publicinbox.pop3state undefined\n";
-	-d $d or do {
-		require File::Path;
-		File::Path::make_path($d, { mode => 0700 });
-		PublicInbox::Syscall::nodatacow_dir($d);
-	};
+	my ($cls) = @_;
 	bless {
 		err => \*STDERR,
 		out => \*STDOUT,
-		pi_cfg => $pi_cfg,
-		lock_path => "$d/db.lock", # PublicInbox::Lock to protect SQLite
+		# pi_cfg => PublicInbox::Config
+		# lock_path => ...
 		# interprocess lock is the $pop3state/txn.locks file
 		# txn_locks => {}, # intraworker locks
 		# accept_tls => { SSL_server => 1, ..., SSL_reuse_ctx => ... }
@@ -61,16 +53,22 @@ sub refresh_groups { # PublicInbox::Daemon callback
 	my ($self, $sig) = @_;
 	# TODO share pi_cfg with nntpd/imapd inside -netd
 	my $new = PublicInbox::Config->new;
-	my $old = $self->{pi_cfg};
-	my $s = 'publicinbox.pop3state';
-	$new->{$s} //= $old->{$s};
-	if ($new->{$s} ne $old->{$s}) {
-		warn <<EOM;
+	my $d = $new->{'publicinbox.pop3state'} //
+		die "publicinbox.pop3state undefined ($new->{-f})\n";
+	-d $d or do {
+		require File::Path;
+		File::Path::make_path($d, { mode => 0700 });
+		PublicInbox::Syscall::nodatacow_dir($d);
+	};
+	$self->{lock_path} //= "$d/db.lock";
+	if (my $old = $self->{pi_cfg}) {
+		my $s = 'publicinbox.pop3state';
+		$new->{$s} //= $old->{$s};
+		return warn <<EOM if $new->{$s} ne $old->{$s};
 $s changed: `$old->{$s}' => `$new->{$s}', config reload ignored
 EOM
-	} else {
-		$self->{pi_cfg} = $new;
 	}
+	$self->{pi_cfg} = $new;
 }
 
 # persistent tables
diff --git a/t/alt.psgi b/t/alt.psgi
new file mode 100644
index 00000000..c7f42979
--- /dev/null
+++ b/t/alt.psgi
@@ -0,0 +1,17 @@
+# Copyright (C) all contributors <meta@public-inbox.org>
+# License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
+use v5.12;
+use warnings;
+use Plack::Builder;
+my $pi_config = $ENV{PI_CONFIG} // 'unset'; # capture ASAP
+my $app = sub {
+	my ($env) = @_;
+	$env->{'psgi.errors'}->print("ALT\n");
+	[ 200, ['Content-Type', 'text/plain'], [ $pi_config ] ]
+};
+
+builder {
+	enable 'ContentLength';
+	enable 'Head';
+	$app;
+}
diff --git a/t/httpd-corner.psgi b/t/httpd-corner.psgi
index e9a3a6b7..10cf8350 100644
--- a/t/httpd-corner.psgi
+++ b/t/httpd-corner.psgi
@@ -1,11 +1,12 @@
-# Copyright (C) 2016-2021 all contributors <meta@public-inbox.org>
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 # corner case tests for the generic PSGI server
 # Usage: plackup [OPTIONS] /path/to/this/file
-use strict;
+use v5.12;
 use warnings;
 use Plack::Builder;
 require Digest::SHA;
+my $pi_config = $ENV{PI_CONFIG} // 'unset'; # capture ASAP
 my $app = sub {
 	my ($env) = @_;
 	my $path = $env->{PATH_INFO};
@@ -114,6 +115,9 @@ my $app = sub {
 	} elsif ($path eq '/url_scheme') {
 		$code = 200;
 		push @$body, $env->{'psgi.url_scheme'}
+	} elsif ($path eq '/PI_CONFIG') {
+		$code = 200;
+		push @$body, $pi_config; # show value at ->refresh_groups
 	}
 	[ $code, $h, $body ]
 };
diff --git a/t/httpd-corner.t b/t/httpd-corner.t
index 0a613a9e..973cc55d 100644
--- a/t/httpd-corner.t
+++ b/t/httpd-corner.t
@@ -1,4 +1,5 @@
-# Copyright (C) 2016-2021 all contributors <meta@public-inbox.org>
+#!perl -w
+# Copyright (C) all contributors <meta@public-inbox.org>
 # License: AGPL-3.0+ <https://www.gnu.org/licenses/agpl-3.0.txt>
 # note: our HTTP server should be standalone and capable of running
 # generic PSGI/Plack apps.
@@ -19,7 +20,7 @@ ok(defined mkfifo($fifo, 0777), 'created FIFO');
 my $err = "$tmpdir/stderr.log";
 my $out = "$tmpdir/stdout.log";
 my $psgi = "./t/httpd-corner.psgi";
-my $sock = tcp_server() or die;
+my $sock = tcp_server();
 my @zmods = qw(PublicInbox::GzipFilter IO::Uncompress::Gunzip);
 
 # Make sure we don't clobber socket options set by systemd or similar
@@ -53,14 +54,40 @@ sub unix_server ($) {
 
 my $upath = "$tmpdir/s";
 my $unix = unix_server($upath);
+my $alt = tcp_server();
 my $td;
 my $spawn_httpd = sub {
 	my (@args) = @_;
-	my $cmd = [ '-httpd', @args, "--stdout=$out", "--stderr=$err", $psgi ];
-	$td = start_script($cmd, undef, { 3 => $sock, 4 => $unix });
+	my $x = tcp_host_port($alt);
+	my $cmd = [ '-httpd', @args, "--stdout=$out", "--stderr=$err", $psgi,
+		'-l', "http://$x/?psgi=t/alt.psgi,env.PI_CONFIG=/path/to/alt".
+			",err=$tmpdir/alt.err" ];
+	my $env = { PI_CONFIG => '/dev/null' };
+	$td = start_script($cmd, $env, { 3 => $sock, 4 => $unix, 5 => $alt });
 };
 
 $spawn_httpd->();
+{
+	my $conn = conn_for($alt, 'alt PSGI path');
+	$conn->write("GET / HTTP/1.0\r\n\r\n");
+	$conn->read(my $buf, 4096);
+	like($buf, qr!^/path/to/alt\z!sm,
+		'alt.psgi loaded on alt socket with correct env');
+
+	$conn = conn_for($sock, 'default PSGI path');
+	$conn->write("GET /PI_CONFIG HTTP/1.0\r\n\r\n");
+	$conn->read($buf, 4096);
+	like($buf, qr!^/dev/null\z!sm,
+		'default PSGI on original socket');
+	my $log = capture("$tmpdir/alt.err");
+	ok(grep(/ALT/, @$log), 'alt psgi.errors written to');
+	$log = capture($err);
+	ok(!grep(/ALT/, @$log), 'STDERR not written to');
+	is(unlink($err, "$tmpdir/alt.err"), 2, 'unlinked stderr and alt.err');
+
+	$td->kill('USR1'); # trigger reopen_logs
+}
+
 if ('test worker death') {
 	my $conn = conn_for($sock, 'killed worker');
 	$conn->write("GET /pid HTTP/1.1\r\nHost:example.com\r\n\r\n");
@@ -82,6 +109,10 @@ if ('test worker death') {
 	like($body, qr/\A[0-9]+\z/, '/pid response');
 	isnt($body, $pid, 'respawned worker');
 }
+{ # check on prior USR1 signal
+	ok(-e $err, 'stderr recreated after USR1');
+	ok(-e "$tmpdir/alt.err", 'alt.err recreated after USR1');
+}
 {
 	my $conn = conn_for($sock, 'Header spaces bogus');
 	$conn->write("GET /empty HTTP/1.1\r\nSpaced-Out : 3\r\n\r\n");

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/6] daemon: require absolute cert/key paths with --daemonize
  2022-08-01 21:24 [PATCH 0/6] flesh out more -netd funcionality Eric Wong
  2022-08-01 21:24 ` [PATCH 1/6] httpd: make internals slightly more generic Eric Wong
  2022-08-01 21:24 ` [PATCH 2/6] daemon: support per-listener env, .psgi, out, err Eric Wong
@ 2022-08-01 21:24 ` Eric Wong
  2022-08-01 21:24 ` [PATCH 4/6] daemon: add diagnostics about inherited/bound listeners Eric Wong
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2022-08-01 21:24 UTC (permalink / raw)
  To: meta

This is preparation for supporting loading new certs on SIGHUP.
---
 lib/PublicInbox/Daemon.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 0392d15f..87a359e7 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -66,6 +66,7 @@ sub accept_tls_opt ($) {
 			my $host = '';
 			$path =~ s/\A([^:]+):// and $host = $1;
 			$x->{$host} = $path;
+			check_absolute($k, $path) if $daemonize;
 		}
 	}
 	my $ctx = IO::Socket::SSL::SSL_Context->new(%ctx_opt) or
@@ -283,6 +284,8 @@ sub daemonize () {
 		check_absolute('--stdout', $stdout);
 		check_absolute('--stderr', $stderr);
 		check_absolute('--pid-file', $pid_file);
+		check_absolute('--cert', $default_cert);
+		check_absolute('--key', $default_key);
 
 		chdir '/' or die "chdir failed: $!";
 	}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/6] daemon: add diagnostics about inherited/bound listeners
  2022-08-01 21:24 [PATCH 0/6] flesh out more -netd funcionality Eric Wong
                   ` (2 preceding siblings ...)
  2022-08-01 21:24 ` [PATCH 3/6] daemon: require absolute cert/key paths with --daemonize Eric Wong
@ 2022-08-01 21:24 ` Eric Wong
  2022-08-01 21:24 ` [PATCH 5/6] daemon: allow listening on well-known ports based on protocol Eric Wong
  2022-08-01 21:24 ` [PATCH 6/6] daemon: share FDs for identical log paths Eric Wong
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2022-08-01 21:24 UTC (permalink / raw)
  To: meta

These are helpful for diagnosing configuration problems,
as well as a bug (to be fixed in the following commit).
---
 lib/PublicInbox/Daemon.pm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 87a359e7..3264bb6c 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -129,6 +129,7 @@ sub load_mod ($;$) {
 			die "multiple psgi= options specified\n" if @$p > 1;
 			check_absolute('psgi=', $p->[0]) if $daemonize;
 			$tlsd->{psgi} = $p->[0];
+			warn "# $scheme://$addr psgi=$p->[0]\n";
 		}
 	}
 	for my $f (@paths) {
@@ -138,6 +139,7 @@ sub load_mod ($;$) {
 		$p = File::Spec->canonpath($p->[0]);
 		open_log_path(my $fh, $p);
 		$tlsd->{$f} = $logs{$p} = $fh;
+		warn "# $scheme://$addr $f=$p\n";
 	}
 	\%xn;
 }
@@ -247,8 +249,10 @@ EOF
 		warn "error binding $l: $! ($@)\n" unless $s;
 		umask $prev;
 		if ($s) {
-			$listener_names->{sockname($s)} = $s;
 			$s->blocking(0);
+			my $k = sockname($s);
+			warn "# bound $scheme://$k\n";
+			$listener_names->{$k} = $s;
 			push @listeners, $s;
 		}
 	}
@@ -434,11 +438,12 @@ sub inherit ($) {
 		if (my $k = sockname($s)) {
 			my $prev_was_blocking = $s->blocking(0);
 			warn <<"" if $prev_was_blocking;
-Inherited socket (fd=$fd) is blocking, making it non-blocking.
+Inherited socket ($k fd=$fd) is blocking, making it non-blocking.
 Set 'NonBlocking = true' in the systemd.service unit to avoid stalled
 processes when multiple service instances start.
 
 			$listener_names->{$k} = $s;
+			warn "# inherited $k fd=$fd\n";
 			push @rv, $s;
 		} else {
 			warn "failed to inherit fd=$fd (LISTEN_FDS=$fds)";

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 5/6] daemon: allow listening on well-known ports based on protocol
  2022-08-01 21:24 [PATCH 0/6] flesh out more -netd funcionality Eric Wong
                   ` (3 preceding siblings ...)
  2022-08-01 21:24 ` [PATCH 4/6] daemon: add diagnostics about inherited/bound listeners Eric Wong
@ 2022-08-01 21:24 ` Eric Wong
  2022-08-01 21:24 ` [PATCH 6/6] daemon: share FDs for identical log paths Eric Wong
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2022-08-01 21:24 UTC (permalink / raw)
  To: meta

This allows admins to use "-l nntp://0.0.0.0/" to bind on port 119
without specifying ":119" on the CLI.
---
 lib/PublicInbox/Daemon.pm | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index 3264bb6c..ead5afc0 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -35,6 +35,9 @@ my ($uid, $gid);
 my ($default_cert, $default_key);
 my %KNOWN_TLS = (443 => 'https', 563 => 'nntps', 993 => 'imaps', 995 =>'pop3s');
 my %KNOWN_STARTTLS = (110 => 'pop3', 119 => 'nntp', 143 => 'imap');
+my %SCHEME2PORT = map { $KNOWN_TLS{$_} => $_ + 0 } keys %KNOWN_TLS;
+for (keys %KNOWN_STARTTLS) { $SCHEME2PORT{$KNOWN_STARTTLS{$_}} = $_ + 0 }
+$SCHEME2PORT{http} = 80;
 
 sub listener_opt ($) {
 	my ($str) = @_; # opt1=val1,opt2=val2 (opt may repeat for multi-value)
@@ -103,9 +106,10 @@ sub open_log_path ($$) { # my ($fh, $path) = @_; # $_[0] is modified
 	do_chown($_[1]);
 }
 
-sub load_mod ($;$) {
-	my ($scheme, $opt) = @_;
+sub load_mod ($;$$) {
+	my ($scheme, $opt, $addr) = @_;
 	my $modc = "PublicInbox::\U$scheme";
+	$modc =~ s/S\z//;
 	my $mod = $modc.'D';
 	eval "require $mod"; # IMAPD|HTTPD|NNTPD|POP3D
 	die $@ if $@;
@@ -200,11 +204,17 @@ EOF
 	foreach my $l (@cfg_listen) {
 		my $orig = $l;
 		my $scheme = '';
-		if ($l =~ s!\A([^:]+)://!!) {
-			$scheme = $1;
-		} elsif ($l =~ /\A(?:\[[^\]]+\]|[^:]+):([0-9])+/) {
-			my $s = $KNOWN_TLS{$1} // $KNOWN_STARTTLS{$1};
-			$scheme = $s if defined $s;
+		my $port;
+		if ($l =~ s!\A([^:]+)://!!) { $scheme = $1 }
+		if ($l =~ /\A(?:\[[^\]]+\]|[^:]+):([0-9]+)/) {
+			$port = $1 + 0;
+			my $s = $KNOWN_TLS{$port} // $KNOWN_STARTTLS{$port};
+			$scheme //= $s if defined $s;
+		} elsif (index($l, '/') != 0) { # unix socket
+			$port //= $SCHEME2PORT{$scheme} if $scheme;
+			$port // die "no port in listen=$l\n";
+			$l =~ s!\A([^/]+)!$1:$port! or
+				die "unable to add port=$port to $l\n";
 		}
 		my $opt; # non-TLS options
 		if ($l =~ s!/?\?(.+)\z!!) {
@@ -215,8 +225,8 @@ EOF
 		} elsif ($scheme =~ /\A(?:https|imaps|nntps|pop3s)\z/) {
 			die "$orig specified w/o cert=\n";
 		}
-		$scheme =~ /\A(http|imap|nntp|pop3)/ and
-			$xnetd->{$l} = load_mod($1, $opt);
+		$scheme =~ /\A(?:http|imap|nntp|pop3)/ and
+			$xnetd->{$l} = load_mod($scheme, $opt, $l);
 
 		next if $listener_names->{$l}; # already inherited
 		my (%o, $sock_pkg);
@@ -263,7 +273,7 @@ EOF
 	for my $sockname (@inherited_names) {
 		$sockname =~ /:([0-9]+)\z/ or next;
 		if (my $scheme = $KNOWN_TLS{$1}) {
-			$xnetd->{$sockname} = load_mod(substr($scheme, 0, -1));
+			$xnetd->{$sockname} = load_mod($scheme);
 			$tls_opt{"$scheme://$sockname"} ||= accept_tls_opt('');
 		} elsif (($scheme = $KNOWN_STARTTLS{$1})) {
 			$xnetd->{$sockname} = load_mod($scheme);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 6/6] daemon: share FDs for identical log paths
  2022-08-01 21:24 [PATCH 0/6] flesh out more -netd funcionality Eric Wong
                   ` (4 preceding siblings ...)
  2022-08-01 21:24 ` [PATCH 5/6] daemon: allow listening on well-known ports based on protocol Eric Wong
@ 2022-08-01 21:24 ` Eric Wong
  5 siblings, 0 replies; 7+ messages in thread
From: Eric Wong @ 2022-08-01 21:24 UTC (permalink / raw)
  To: meta

We rely on the %logs hash for SIGUSR1 log reopening.  Without this sharing,
some FDs would be hidden inside its respective {HTTP,IMAP,POP3}D
object and not reopened on USR2
---
 lib/PublicInbox/Daemon.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/Daemon.pm b/lib/PublicInbox/Daemon.pm
index ead5afc0..20b07b83 100644
--- a/lib/PublicInbox/Daemon.pm
+++ b/lib/PublicInbox/Daemon.pm
@@ -104,6 +104,7 @@ sub open_log_path ($$) { # my ($fh, $path) = @_; # $_[0] is modified
 	open $_[0], '>>', $_[1] or die "open(>> $_[1]): $!";
 	$_[0]->autoflush(1);
 	do_chown($_[1]);
+	$_[0];
 }
 
 sub load_mod ($;$$) {
@@ -141,8 +142,7 @@ sub load_mod ($;$$) {
 		die "multiple $f= options specified\n" if @$p > 1;
 		check_absolute("$f=", $p->[0]) if $daemonize;
 		$p = File::Spec->canonpath($p->[0]);
-		open_log_path(my $fh, $p);
-		$tlsd->{$f} = $logs{$p} = $fh;
+		$tlsd->{$f} = $logs{$p} //= open_log_path(my $fh, $p);
 		warn "# $scheme://$addr $f=$p\n";
 	}
 	\%xn;

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-01 21:24 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-01 21:24 [PATCH 0/6] flesh out more -netd funcionality Eric Wong
2022-08-01 21:24 ` [PATCH 1/6] httpd: make internals slightly more generic Eric Wong
2022-08-01 21:24 ` [PATCH 2/6] daemon: support per-listener env, .psgi, out, err Eric Wong
2022-08-01 21:24 ` [PATCH 3/6] daemon: require absolute cert/key paths with --daemonize Eric Wong
2022-08-01 21:24 ` [PATCH 4/6] daemon: add diagnostics about inherited/bound listeners Eric Wong
2022-08-01 21:24 ` [PATCH 5/6] daemon: allow listening on well-known ports based on protocol Eric Wong
2022-08-01 21:24 ` [PATCH 6/6] daemon: share FDs for identical log paths Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).