Eric Wong [Wed, 2 Jul 2025 20:43:21 +0000 (20:43 +0000)]
nntpd|imapd|pop3d: wait on writability, first
While instances of the first write(2) failing with EAGAIN
immediately after being returned by accept(2) is highly
unlikely, it still seems like a theoretical possibility. So out
of an abundance of caution, we'll just wait on writability to
reduce branches in our code.
Eric Wong [Sat, 28 Jun 2025 11:20:13 +0000 (11:20 +0000)]
ds: long_response: reduce {long_cb} size
Invoking the `fileno' op should be trivial in cost and we can
reduce the amount of potentially long-lived storage for long
responses by invoking it with every long_step call.
Eric Wong [Sat, 28 Jun 2025 11:20:10 +0000 (11:20 +0000)]
ds: support enqueued CODE refs with arguments
While we inherited support for enqueuing CODE refs from
Danga::Socket, it makes sense to support enqueuing CODE
refs with extra arguments specific to that CODE ref[1]
instead of relying on anonymous CODE refs to capture its
args..
Supporting args with CODE refs allows our ->flush_write
implementation to be more generic and no longer specific to
tmpio write buffering. This change may eventually allow us
to chain multiple long_response calls together and eliminate
the {long_cb} field.
[1] we prefer CODE refs point to named subs with arguments
passed to it to save RAM as opposed to constantly allocating
new anonymous subs to capture local variables.
Eric Wong [Mon, 23 Jun 2025 18:51:36 +0000 (18:51 +0000)]
ds: flush_write: check {sock} after calling CODE
A user callback may call ->close and invalidate {sock} or even
replace it, so we must revalidate the {sock} field and break
break out of the loop ASAP to ensure we don't attempt further
operations on an invalid socket.
This really ought to fix the
`BUG: ep_mod GLOB=GLOB(...): no such file or directory' (ENOENT)
errors on EPOLL_CTL_MOD when a client shuts down TLS on us.
Eric Wong [Fri, 20 Jun 2025 20:39:40 +0000 (20:39 +0000)]
inbox: introduce lock_file for inbox_idle
We may support idling on inbox-ish objects such as
PublicInbox::ExtSearch or similar (ActivityPub) objects in the
future, so taking version-specific knowledge out of InboxIdle
make sense for future changes.
Eric Wong [Tue, 17 Jun 2025 00:13:34 +0000 (00:13 +0000)]
ds: shutdn_tls_step clobbers {wbuf} early
Instead of prepending to {wbuf} and preserving tmpio objects
and/or other coderefs, clobber it early since we don't keep
sockets after shutting down TLS. This ought to fix the `BUG:
ep_mod GLOB=GLOB(...): no such file or directory' (ENOENT)
errors on EPOLL_CTL_MOD when a client shuts down the connection
while serving large HTTPS responses.
Eric Wong [Tue, 17 Jun 2025 00:13:33 +0000 (00:13 +0000)]
tls: shorten warning $SSL_ERROR prefix
Use our usual `W:' warning prefix here since the stringified
form of the $SSL_ERROR magic variable will be something like:
"SSL read error error:xxx:SSL routines:SSL_shutdown:shutdown while in init"
and we don't need to be mentioning `SSL' ourselves.
Eric Wong [Tue, 17 Jun 2025 00:13:32 +0000 (00:13 +0000)]
ds: confess on epoll_ctl failure
ENOENT on epoll_ctl(2) is always a sign of buggy code and it
makes sense to dump the backtrace by default, here. I've only
noticed this failure on HTTPS connections.
Eric Wong [Sat, 14 Jun 2025 09:26:35 +0000 (09:26 +0000)]
daemon: introduce register_log public API
Having this public API would make it easier to configure
AccessLog and AccessLog::Timed middleware in multiple
areas while allowing SIGUSR1 to reopen them:
my $log_pfx = $ENV{LOGGER_PFX} // '/var/log/xyz/';
my $access_log = sub {
my ($pfx) = @_;
my $fh = PublicInbox::Daemon::register_log
$log_pfx.$pfx.'.access.log';
enable 'AccessLog::Timed',
logger => sub { syswrite $fh, $_[0] };
};
builder {
mount 'http://foo.example.com/' => builder {
$access_log->('foo');
$foo_app;
};
mount 'http://bar.example.com/' => builder {
$access_log->('bar');
$bar_app;
};
}
I'm not particularly happy about introducing a
public-inbox-specific API for .psgi file writers; I can't think
of another portable and reliable way to reopen all log files
used by a PSGI application upon SIGUSR1.
Eric Wong [Mon, 16 Jun 2025 20:05:13 +0000 (20:05 +0000)]
daemon: reopen log files atomically
Ensuring there is no window for a Perl IO (file handle) to be
invalid is useful in case some code ever gets used in a
multithreaded environment. The FD stability would also be less
confusing for users using `lsof(8)' or similar to track FDs.
Eric Wong [Sat, 14 Jun 2025 09:26:33 +0000 (09:26 +0000)]
daemon: allow `out' directive for HTTP, too
While we currently don't output the the `out' directive for HTTP
as we do for protocols which lack PSGI support; the special case
for HTTP doesn't make sense as (in theory) a PSGI middleware
could someday exist to write to that destination.
Eric Wong [Sat, 14 Jun 2025 09:26:32 +0000 (09:26 +0000)]
http: response_write handles HEAD responses
Since we special-case MSG_MORE for HEAD responses already,
our HTTP server code now strips response bodies from HEAD
requests unconditionally, allowing .psgi files to omit the
`enable "Head"' directive.
Proper placement of `Head' (Plack::Middleware::Head) in the
middleware stack can be confusing and tedious since it needs to
be after things like AccessLog::Timed which need to wrap the
response body with an object which responds to ->close.
Furthermore, removal of the response body in HEAD requests is
required in all relevant HTTP RFCs (2616, 7231 and 9110) and
doing HEAD response handling in the server itself avoids the
possibility of misconfiguration by the .psgi file maintainer.
The main reason in this rewrite was the desire to easily
configure per-map AccessLog::Timed destination log files.
Correct configuration without this patch would require putting
an `enable "Head"' directive inside every Plack::Builder::map
block after `enable "AccessLog::Timed"'.
In other words, the .psgi file having a single `enable "Head"'
encompassing everything:
...would be more tedious with many `map' directives. With this
change, all `enable "Head"' directives can simply be omitted for
public-inbox-{netd,httpd} users.
Eric Wong [Tue, 10 Jun 2025 05:18:10 +0000 (05:18 +0000)]
www_static: use Plack::Component->to_app
Requiring users to create anonymous subs is tedious and
Plack::Component is already a part of Plack (and installed
by all PSGI users) and already used by many middlewares.
Eric Wong [Tue, 10 Jun 2025 05:18:09 +0000 (05:18 +0000)]
www_static: support default_type parameter
Sysadmins may wish to override the default
`application/octet-stream' for files with no suffix or unknown
suffixes. One use case is serving the
https://public-inbox.org/README itself with the `text/plain'
Content-Type.
Plack::MIME->set_fallback can be an alternative to this
parameter, but its effect is interpreter-wide and server admins
may wish to set the default type on a per-route basis.
We'll attempt to stringify the failing IO handle in case
Perl can output more info about the object. Furthermore,
we can disable checks for `print' failures preceding ->flush
since they're redundant and a waste of code.
Eric Wong [Fri, 6 Jun 2025 10:24:27 +0000 (10:24 +0000)]
test_common: fix xap_helper build reuse
The change in commit c45a5498 to avoid copying introduced a
major test performance regression since the extra XFLAGS and
xap_modversion files (required to detect changes for rebuilding)
weren't copied. Instead, just symlink the entire
architecture-specific directory instead of individual files.
I didn't notice the slowdown earlier since I only tested on a
busy system where my worktree is on an overloaded HDD.
Fixes: c45a5498 (test_common: symlink xap_helper instead of excessive copying, 2025-06-01)
Eric Wong [Thu, 5 Jun 2025 06:53:36 +0000 (06:53 +0000)]
http: don't ->close after ->accept_SSL failure
->accept_SSL failures leaves the socket ref as a GLOB (not
IO::Handle) and unable to respond to the ->close method.
Calling close in any form isn't actually necessary at all,
so just let refcounting destroy the socket.
Followup-to: e85d3280 (ds: don't try ->close after ->accept_SSL failure, 2023-11-02)
Eric Wong [Wed, 4 Jun 2025 11:34:58 +0000 (11:34 +0000)]
http: respect `Connection: close' in HTTP/1.1 requests
HTTP/1.1 clients may wish to explicitly terminate the connection
upon reading the response. While clients may specify HTTP/1.0
to force a disconnect, HTTP/1.0 lacked chunked encoding which
meant streaming responses of indeterminate length could be
silently truncated at the protocol level and undetectable in
cases of uncompressed plain-text.
Eric Wong [Wed, 4 Jun 2025 11:34:57 +0000 (11:34 +0000)]
ds: do_read: return 0 on EOF
It's conceivable that future users of this internal function
will want to distinguish between a client which failed due to
some network or HW error or a normal disconnect.
James Bottomley [Thu, 29 May 2025 15:23:21 +0000 (11:23 -0400)]
fix git permission denied in certain configs
The problem is the which() sub in Spawn.pm only checks if the path
fragment is executable, but this may be true of a directory as well.
In particular, a lot of people have a git directory in their $HOME
(where they store their git repos) and if $PATH contains '.' then
SpawnPP.pm will try to execute this directory leading to this failure
on a lot of lei commands:
exec ./git var GIT_PAGER failed: Permission denied at /usr/share/perl5/PublicInbox/SpawnPP.pm line 62.
fork_exec git var GIT_PAGER failed: Permission denied
As you can see what's happened is it's tried to execute my ./git
directory, which obviously fails. The fix is simple, skip executable
files which are also directories when doing $PATH based lookup
[ew: use `_' bareword to avoid 2nd stat, spelling fix in commit message]
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Eric Wong [Thu, 29 May 2025 00:20:31 +0000 (00:20 +0000)]
extmsg: improve handling of obfuscated Message-ID URLs
Some email clients (or 3rd-party archival sites) apparently
mistake Message-IDs as email addresses and replace the last part
of the unique (non-domain) prefix before the `@' with `...'.
The `...' causes Xapian query parser to attempt to interpret
the Message-ID portion as a range. So attempt to replace
eliminated portions (`...') with the wildcard operator (`*')
and hope it doesn't overload Xapian.
We'll also use wildcards more liberally for long word-like
sequences to improve success rate and improve diagnostics for
failed queries while we're at it.
Eric Wong [Thu, 29 May 2025 00:20:30 +0000 (00:20 +0000)]
xap_helper: mset: ignore non-ultimate exceptions
For requests consisting of multiple queries, ignore Xapian
exceptions for all but the last query. When sending multiple
queries to the `mset' xap_helper command, we only care about the
result of the first successful query where success is defined as
a non-empty mset response. Thus an exception is considered an
empty mset and we can try other queries from the same request.
Eric Wong [Thu, 29 May 2025 00:20:29 +0000 (00:20 +0000)]
xhc_mset: improve error message formatting
We don't need extra linefeeds before the `)' in the stderr
output, and the context from croak() is bogus anyways since we
get error messages from an external process.
Eric Wong [Sun, 1 Jun 2025 10:14:47 +0000 (10:14 +0000)]
test_common: symlink xap_helper instead of excessive copying
Copying the entire directory is racy in the face of parallel
tests and caused an occasional test failure due to attempts (and
failures) in copying short-lived temporary files. A symlink
for just the executable avoids attempted copies temporary files
is less expensive for the intended file, as well.
Eric Wong [Mon, 12 May 2025 20:45:01 +0000 (20:45 +0000)]
www: extmsg: dedupe cross-posted Message-IDs
Having redundant links to the same cross-posted message is
unlikely to be of help, especially when using an /all/ extindex.
So just grab the first matching inbox + Message-ID combo and
ignore subsequent URLs for Message-IDs which exist in multiple
inboxes/extindices.
Eric Wong [Mon, 12 May 2025 20:45:00 +0000 (20:45 +0000)]
www: extmsg: async partial Message-ID search
While partial Message-ID matches tend to be relatively fast,
there's still some potential for pathological matches taking
a long time and blocking our event loop. So throw them over
to the external xap_helper process to ensure our main event
loop can handle other HTTP/NNTP/POP3/IMAP requests while
potentially waiting on Xapian disk seeks.
Eric Wong [Mon, 12 May 2025 20:44:59 +0000 (20:44 +0000)]
xap_helper: mset supports multiple requests
By supporting multiple queries in one IPC call, we can reduce
IPC traffic for search endpoints which make multiple search
requests but only use the result of the first. This will be
used for partial Message-ID matching for handling truncated
URLs.
Eric Wong [Mon, 12 May 2025 20:44:58 +0000 (20:44 +0000)]
www: extmsg: rely on event loop for partial matches
Preparation for (optionally) using xap_helper to asynchronously
look up Message-ID matches. This change does not materially
change behavior, but will make subsequent changes easier to
digest.
Eric Wong [Fri, 9 May 2025 23:11:57 +0000 (23:11 +0000)]
sha: avoid string eval for loading Net::SSLeay
We can rely on require + import and use glob assigments to give
anonymous subs a place in the symbol table. This ought to make
it easier to do compile-only syntax checking and hopefully
easier for a hypothetical Perl implementation to optimize.
Eric Wong [Wed, 7 May 2025 00:16:33 +0000 (00:16 +0000)]
httpd: psgix.io is the underlying IO object
PSGI::Extensions(3pm) states psgix.io should be "the raw IO
socket", so we shall conform to that specification. For our
codebase to continue taking advantage of
public-inbox-daemon-specific features, we now expose the
PublicInbox::HTTP object via $env->{'pi-httpd.client'}. The
`pi-httpd.async' boolean field is now gone, since we can just
test for the presence of `pi-httpd.app' or `pi-httpd.client'
to avoid occupying another hash table entry.
Eric Wong [Thu, 1 May 2025 23:18:10 +0000 (23:18 +0000)]
syscall: fix fstatfs(2) use for x32
x32 syscalls all seem to have 0x40000000 added to them, so the
low x86-64 number seems obviously wrong. Tested on an x32 btrfs
chroot where I also realized the statfs.f_type field is 8 bytes
since x32 kernels are 64-bit.
Eric Wong [Thu, 1 May 2025 19:35:55 +0000 (19:35 +0000)]
solver: accept extra trailing space in filenames of diffs
Some crazy MUAs add trailing whitespace which throws off our
strict patch parsing. So fall back to stripping trailing
whitespace and retrying to match the filename from `git
ls-files'. Stripping trailing whitespace matches our liberal
acceptance of trailing whitespace when using `git apply', too.
For debug messages, switch to `B::cstring' for unconditionally
quoting pathnames since our `git_quote' sub matches the
problematic git(1) behavior in not quoting actual pathnames with
trailing whitespace. `B' is the Perl compiler backend
module, which (AFAIK) is in every Perl 5 installation,
so its use shouldn't cause burdens to our existing users.
Eric Wong [Tue, 29 Apr 2025 20:47:46 +0000 (20:47 +0000)]
spawn: avoid string eval for optional BSD::Resource
BSD::Resource has had a get_rlimits() sub for a while to return
a hashref to non-Inline::C users. So favor using that after
`require' rather than a string eval which can't be checked for
syntax-only errors.
Eric Wong [Tue, 29 Apr 2025 20:47:45 +0000 (20:47 +0000)]
repo_atom: add comments around string eval usage
Readers unfamiliar with the code may see a code injection
vulnerability here, so try to reassure them that we're using
`--perl'-escaped output from git-for-each-ref(1) that ought
to be safe to run through `eval'.
Eric Wong [Tue, 29 Apr 2025 20:47:43 +0000 (20:47 +0000)]
treewide: use autodie::open where possible
Outside of development scripts to run on cfarm machines, we can
rely on autodie being available to reduce noise in our code and
improve error message consistency.
Eric Wong [Tue, 29 Apr 2025 20:47:42 +0000 (20:47 +0000)]
clone: don't dup(2) stdout FD
Avoiding close(2) on stdout doesn't seem to be worth it anyways,
as lei code shouldn't be closing FDs unless they fail (e.g.
SIGPIPE), in which case keeping a duplicate FD isn't doing any
good, anyways.
Eric Wong [Tue, 29 Apr 2025 17:16:51 +0000 (17:16 +0000)]
dskqxs: use OnDestroy explicitly for $fork_gen
We need to ensure OnDestroy is always loaded before using it.
While DSKQXS itslf is only loaded by DS (which loads OnDestroy),
the t/ds-kqxs.t test doesn't load DS (unless using `make check-run').
Eric Wong [Tue, 29 Apr 2025 17:16:50 +0000 (17:16 +0000)]
treewide: replace signalfd with epoll_pwait
epoll_pwait requires less code and fewer FDs on our end to
support than signalfd. Linux <=2.6.18 users will now be stuck
with select/poll, but I doubt there's any <=2.6.18 users
nowadays running a current public-inbox. On the plus side,
Linux 2.6.19..2.6.21 users now get faster and more reliable
signal wakeups since they lacked signalfd support, but again,
there's probably no public-inbox users running that small window
of old Linux releases, either.
*BSD users will also benefit from not having to deal with a
redundant kqueue FD to handle signals quickly and reliably.
Eric Wong [Tue, 29 Apr 2025 17:16:49 +0000 (17:16 +0000)]
dskqxs: ignore all EV_DISABLE failures
It's impossible to check for the same errors on non-registered
FDs with the poll(2) and select(2) backends while our epoll
backend has historically ignored failed EPOLL_CTL_DEL ops. So
just ignore it for now since synchronous use of XhcMset in lei
can trigger the unnecessary EV_DISABLE.
Eric Wong [Tue, 29 Apr 2025 17:16:48 +0000 (17:16 +0000)]
lei: deal with spurious wakeups
Apparently spurious wakeups remain a problem even for local
sockets. Oddly, I only noticed this when switching lei to
use epoll_pwait on a slow machine.
Eric Wong [Tue, 29 Apr 2025 17:16:47 +0000 (17:16 +0000)]
input_stream: only rely on events for pipes+sockets
If using kevent(2) in the event loop, kevent actually accepts
EVFILT_READ for regular files instead of triggering EPERM
as epoll_ctl(2) does. This kevent-specific behavior doesn't
work with our expected use with static files which don't see
modifications.
I noticed this on FreeBSD when trying to make lei use kevent
for everything instead of select(2) for sockets and
kevent for EVFILT_SIGNAL only.
Eric Wong [Tue, 29 Apr 2025 17:16:45 +0000 (17:16 +0000)]
t/httpd-corner: improve EPIPE/SIGPIPE handling
We'll ignore SIGPIPE as we do in other places so we can get
EPIPE on failed `print' calls. Furthermore, it's possible we'll
hit EPIPE before we see an 400 HTTP response code when making
bad requests. I've noticed this problem on slower single CPU
machines.
Eric Wong [Fri, 11 Apr 2025 18:35:51 +0000 (18:35 +0000)]
linkify: give <a> tags to gemini:// URLs
While I still think serving the Gemini text and protocol is
unnecessary bloat on our part, there are web browsers with
gemini:// support, so we might as well support it.
Eric Wong [Fri, 11 Apr 2025 01:38:34 +0000 (01:38 +0000)]
http: send `100 Continue' responses for uploads
curl(1) defaults to sending a `Expect: 100-continue' header and
waits 1 second for a `100' response when using the `-T' switch
to upload via PUT. We'll unconditionally respond with a `100'
response to save curl users 1 second. Expecting curl users to
to disable the Expect: header via `-HExpect:' on every invocation
seems unreasonable, so our behavior should be tailored to curl
default behavior as much as possible.
We can't delegate the decision to send 100 or not to the
underlying Plack app (e.g. PublicInbox::WWW) since PSGI specs
don't provide a convenient callback interface for dealing with
trickled env->{'psgi.input'} reads. Of course, having to
synchronously wait on env->{'psgi.input'}->read would be
unacceptable since that would prevent the server process from
servicing other concurrent clients.
Eric Wong [Tue, 8 Apr 2025 20:49:58 +0000 (20:49 +0000)]
daemon: support backlog= listen parameter
For -netd instances using multiple listeners, it's easiest for me
to configure all ListenStream directives in a single
systemd.socket(5) unit. Unfortunately, this seems to force all
listeners to use the same Backlog= value, and juggling
multiple systemd.socket(5) units for a single systemd service
seems tedious.
So support setting the backlog= parameter on the public-inbox-*d
command-line on a per-listener basis; allowing us to override
the backlog value of inherited listeners and also to set the
backlog for listeners we bind ourselves. This makes it possible
for non-systemd users to easily configure per-listener backlogs,
as well.
Eric Wong [Tue, 8 Apr 2025 20:49:57 +0000 (20:49 +0000)]
daemon: use unpack_sockaddr_* for clarity
unpack_sockaddr_un and unpack_sockaddr_in have been around since
Perl 5.002 in the mid-1990s, so be explicit and use them instead
of relying on wantarray caller context. We'll also omit the
$host check for ($port == 0) since zero is not a valid TCP port.
Eric Wong [Sat, 5 Apr 2025 17:44:13 +0000 (17:44 +0000)]
http: refuse to deal with >4GB chunks in uploads
The `hex' perlop will return an NV (typically 64-bit double) on
UV (unsigned int) overflow and warns on larger values. While
64-bit integer builds of 32-bit perl (e.g. Debian i386) can
handle 64-bit numbers, there are builds of perl which still use
32-bit integers nowadays (e.g. OpenBSD 7.x i386).
It's unlikely we'll ever see chunks even close to 4GB, so just
cap it at 8 hex characters and drop clients which send larger
amounts.
Eric Wong [Thu, 3 Apr 2025 08:46:19 +0000 (08:46 +0000)]
t/httpd-https: test SSL session reuse
Reusing SSL sessions is one avenue to improve performance on
high-latency networks. For now, we can support the built-in
session cache of OpenSSL. For multi-process setups, using
using Cache::FastMmap will likely be the way to go...
Eric Wong [Thu, 3 Apr 2025 08:46:17 +0000 (08:46 +0000)]
ds: close: always call SSL_close on TLS sockets
Instead of relying on the explicit ->shutdn API, just
use the overloaded ->close so $env->{'psgix.io'} callers
can use it directly. This makes it easier to ensure
proper shutdown so SSL session reuse can be a possiblity.
Eric Wong [Wed, 2 Apr 2025 19:00:15 +0000 (19:00 +0000)]
http: support trailers for chunked requests
While HTTP/1.1 trailers are not widely supported by other
software at the moment, they may be someday. Trailers are
useful for things such as calculating checksums and signatures
on streaming uploads of unseekable data. So add support for
them in case they're needed someday, perhaps for allow
uploading mail or ActivityPub messages over HTTP.
Eric Wong [Wed, 2 Apr 2025 19:00:14 +0000 (19:00 +0000)]
http: fix and test Trailer: rejection
We need to check for the existence of Trailers after successful
parsing. I actually intend to support HTTP trailers, and I
noticed this while working on adding support for them.
Eric Wong [Wed, 2 Apr 2025 19:00:13 +0000 (19:00 +0000)]
t/httpd-corner: modernize test
We'll rely on autodie more. We'll also declare internal
subroutines as `my' scalars to ensure they can't be accessed by
other tests when we reuse test processes in `check-run'. We'll
also set avoid setting TCP_NODELAY for cases where it's not
necessary to save a handfull of cycles.
More corner case tests will be added soon to ensure we can
handle HTTPS termination for Varnish (or other caches).
Eric Wong [Fri, 28 Mar 2025 10:34:10 +0000 (10:34 +0000)]
daemon: allow per-listener servername= and serverport=
Being able to specify per-listener servername= with the
`--listen' CLI arg is helpful for running an Tor .onion NNTP
server with the same config file (and thus
publicinbox.nntpserver) as a public-facing NNTP endpoint.
For HTTP, servername= and serverport= allow overriding the
SERVER_NAME and SERVER_PORT PSGI environment variables,
respectively. These are useful for generating full URLs
for clients which don't send the HTTP `Host:' header.
These currently have no effect aside from wasting memory for
POP3 and IMAP listeners. serverport= isn't used by NNTP,
either.
Eric Wong [Fri, 28 Mar 2025 10:34:09 +0000 (10:34 +0000)]
www: omit SERVER_PORT for standard port redirects
For HTTP requests without a `Host:' header, we can omit
the server port component of the URL if using port 443
for `https' or 80 for plain `http'. We'll also consistently
use SCRIPT_NAME in all of our tests since Plack requires
it.
Eric Wong [Thu, 27 Mar 2025 23:20:46 +0000 (23:20 +0000)]
nntp: avoid repeated rand() calls
We only need to generate the secret salt once, so initialize
it early to avoid potentially expensive `rand' ops in repeated
calls to wildmat2re. We'll also stringify it early to hopefully
improve CoW sharing and reduce fragmentation.
Eric Wong [Thu, 27 Mar 2025 23:20:45 +0000 (23:20 +0000)]
nntp: avoid modifying $_[0] in RE conversions
I've been getting occasional t/nntp.t warnings about
uninitialized variables which I'm not able to reproduce
reliably. My current theory is that modifying $_[0] may get
wonky when it's happening across several layers of the call
stack, so stop doing it since it's unlikely to yield any
real world speedups and only made the code more difficult
to understand, here.
Eric Wong [Thu, 27 Mar 2025 23:20:44 +0000 (23:20 +0000)]
nntp: avoid uninitialized vars from bogus LIST args
We can only use the `list_' prefix in NNTP.pm for subcommands
which we intend to dispatch from client requests. So prefix
the list_*_i subs with a `_' to prevent `args_ok' from firing
on a non-prototyped subroutine.
I noticed this problem in the previous change to reduce code
duplication between LIST subcommands.
Eric Wong [Thu, 27 Mar 2025 23:20:42 +0000 (23:20 +0000)]
tests: get rid of most Data::Dumper usage
`explain' from Test::More already uses Data::Dumper, so
it's unnecessary to set it up and call it ourselves.
This saves a bunch of wonky code in t/nntp.t, as well.
Eric Wong [Wed, 26 Mar 2025 20:13:03 +0000 (20:13 +0000)]
content_digest_dbg: more compact display w/ git_quote
Make WWW /$MSGID/d/ endpoints and `lei mail-diff' output
less verbose by getting rid of Data::Dumper in favor of
a lightly-modified git_quote output. The extra [] pairs
from Data::Dumper were unnecessary noise and the quoting
used by git_quote should be more familiar to git users.
Unfortunately, git's escaping of NUL bytes to `\000' is
a bit noisy, so we'll undo that by substituting to `\0'.
Eric Wong [Wed, 26 Mar 2025 03:35:02 +0000 (03:35 +0000)]
search: improve xap_helper mset error reporting
For user errors generating bad queries, dumping exception
messages from Xapian::QueryParser can be helpful. So
unconditionally pass the stderr FD as a regular file to
xap_helper processes for both lei and WWW search users.
Eric Wong [Wed, 26 Mar 2025 03:35:01 +0000 (03:35 +0000)]
treewide: use git_quote for diagnostic dumps
git_quote output is probably less alien to most users than
the Perl-specific Data::Dumper, so use it for any messages
which hit stdout/stderr or syslog.
Eric Wong [Wed, 26 Mar 2025 03:34:59 +0000 (03:34 +0000)]
solver: use git_quote on filenames for diagnostics
git_quote output makes more sense than relying on \Q..\E from
Perl or even Data::Dumper since these filenames are from git
output and more users are likely familiar with git-style quoting
than anything Perl-specific.
Eric Wong [Fri, 21 Mar 2025 22:22:30 +0000 (22:22 +0000)]
limiter: refactor to reduce code duplication
PlackLimiter, Qspawn, and ViewVCS all have roughly the same
code around our base Limiter package, so put everything around
a new Limiter->may_start subroutine. PlackLimiter loses some
stats as a result but that's logged anyways and I doubt the
customizable error message was worth the effort.
We now have ckhup and 499 (client disconnect) handling for all
PSGI uses of Limiter, as well.
t/qspawn.t changes were required since the original ->finalize
logic now relies on on_destroy; but none of the existing PSGI
code using Qspawn required changes.
Eric Wong [Thu, 20 Mar 2025 00:05:35 +0000 (00:05 +0000)]
plack_limiter: PSGI middleware to limit concurrency
While processing several concurrent requests within the same
worker process is helpful to exploit parallelism in git blob
lookups and smooth out delays; excessive parallelism is harmful
since it allows too much memory to be allocated at once for zlib
buffers and such.
While PublicInbox::WWW already uses the limiter for certain
expensive endpoints (e.g. /s/ and anything using Qspawn); some
long-running endpoints with many inexpensive steps (e.g. /T/,
/t/, /d/, *.atom, *.mbox.gz, etc.) can end up using a large
amount of memory for gzip buffers despite being fair to other
responses and being able to stream >500 messages/sec on 2010-era
hardware.
So give sysadmins an option to balance between smoothing out
delays in blob retrieval and memory usage required to compress
and spew out chunks of potentially large multi-email responses.
Eric Wong [Tue, 18 Mar 2025 08:30:27 +0000 (08:30 +0000)]
lg2: disable strict hash verification
Unlike git(1), libgit2 verifies the SHA-(1|256) of objects it
reads by default. This verification results in a large (nearly
100% w/ SHA1DC) performance penalty for us. Since our libgit2
code only reads (and never writes objects), just follow git(1)
and skip verification for normal reads.
This brings our libgit2-based Gcf2 batch loop performance closer
to that of the `git cat-file --batch-command' as shown in the
new xt/lg2_cmp.t developer test. However, Gcf2Client still uses
a more verbose (but more flexible) input format and the Perl
gcf2_loop still incurs normal Perl method dispatch overheads.
Eric Wong [Fri, 14 Mar 2025 09:22:04 +0000 (09:22 +0000)]
daemon: define %TLS_ONLY hash
Defining TLS-only protocols only once makes it easier to support
new protocols in the future since we can rely on only updating
this new hash instead of having to update regexps in other
places.
Eric Wong [Thu, 13 Mar 2025 20:35:04 +0000 (20:35 +0000)]
www: disable legacy encoded Message-IDs for non-v1
For a while in the very early days of the v1 format, we
supported SHA-1 checksums of the Message-ID in the URL. This
never affected v2 inboxes nor extindex, and those URLs would've
been broken if run through public-inbox-convert, anyways.
Eric Wong [Thu, 6 Mar 2025 20:34:42 +0000 (20:34 +0000)]
XapHelper.pm: fix QP_FLAGS initialization
We can't use $PublicInbox::Search::QP_FLAGS until after
load_xapian is called. Failure to set the correct query parser
flags was causing failures in
`PI_NO_CXX=1 TEST_DAEMON_XH=-X0 prove -bw t/imapd.t'
since phrase parsing was broken with the Perl bindings
XapHelper implementation.
Now, the `check-xh0' and `check-xh1' targets both pass with
PI_NO_CXX=1 set to disable the C++ xap_helper.
Fixes: fa6a7919 (xap_helper: enable FLAG_PURE_NOT in external process, 2025-02-23)
Eric Wong [Wed, 5 Mar 2025 23:26:47 +0000 (23:26 +0000)]
search: unoverload {relevance} in options
Take the lead from xap_helper and split out handling of {asc}
and {sort_col} fields while keeping the {relevance} field as a
boolean. As with xap_helper, {sort_col} < 0 means
Xapian::BoolWeight will be used for docid ordering.
Eric Wong [Wed, 5 Mar 2025 23:26:46 +0000 (23:26 +0000)]
search: use common do_enquire for MiscSearch
We can consistently use a {sort_col} search option as in
xap_helper to allow do_enquire to be more generic across
searches. This lets us avoid the need for a specialized
implementation in MiscSearch.
Eric Wong [Wed, 5 Mar 2025 07:18:36 +0000 (07:18 +0000)]
lei: use C++ xap_helper if available
The C++ version of xap_helper allows `thread:' subqueries,
non-fragile parsing of git approxidate with `d:', `dt:' and
`rt:' prefixes, and possibly more features in the future not
available in the SWIG or XS Xapian bindings.
There's no point in lei supporting the Perl version of XapHelper
since lei isn't expected to deal with abusive clients making
expensive queries, so lei will only use the C++ version.
We spawn xap_helper in every lei worker process to guarantee
resource availability without having to resort to preforking.
While pre-forking and on-demand thread||process creation was
considered, I decided having a lingering xap_helper process
probably wasn't worth the startup time improvement and instead
prefer to minimize idle memory use.
The whole implementation is a bit strange since lei support was
an after-thought for xap_helper and we wrap the async_mset API
to make it synchronous once again to reduce the code impact for
code shared with public-facing daemons.
Finally, test_lei in TestCommon gets tweaked to avoid repeatedly
rebuilding in a new directory with every test_lei use in our test
suite.
Eric Wong [Wed, 5 Mar 2025 07:18:35 +0000 (07:18 +0000)]
xap_helper.h: share ThreadFieldProcessor across DBs
Unlike notmuch, we don't actually use the QueryParser from
initialization and can stop storing it in the class. Instead,
we rely on the global `cur_srch' at runtime for both which has
both the Xapian::Databaase and Xapian::QueryParser objects. So
stop carrying a copy of the ThreadFieldProcessor object for
every DB connection we have since some public non-extindex-users
may have many DBs and we can probably save some memory this way.
Eric Wong [Wed, 5 Mar 2025 07:18:34 +0000 (07:18 +0000)]
search: avoid `git rev-parse' if using C++ xap_helper
The C++ xap_helper can work with the Xapian query parser API to
run run git_date_parse(), so there's no need to do janky string
substitutions as we do for the Perl bindings.
Eric Wong [Wed, 5 Mar 2025 07:18:33 +0000 (07:18 +0000)]
xap_helper: use libgit2 git_date_parse in C++ impl
Using proper Xapian RangeProcessor and FieldProcessor subclasses
via the Xapian API means our `d:', `dt:' and `rt:' prefixes can
use git `approxidate' interpretation rules inside quoted
subqueries with the `thread:' prefix.
The only incompatibility is the `rt:' field which no longer
takes seconds with <5 characters (e.g. `rt:0..' as was in
t/xap_helper.t), but that'd be broken anyways in real-world use
which is run through `git rev-parse --since='.
Eric Wong [Wed, 5 Mar 2025 07:18:32 +0000 (07:18 +0000)]
search: make `d:' search prefix consistently date-only
While `d:' previously treated YYYYMMDD and YYYY-MM-DD as
low-precision by assuming 00:00:00 for the HH:MM:SS portion, it
would not do so for dates passed to git-rev-parse (e.g.
"last.year") since the HH:MM:SS of the current time would be
used by git. So stop remapping `d:' in queries to `dt:' and
continue using `d:' in the YYYYMMDD column.
This does break things like `d:2.hours.ago..' by causing too
many (or too few) results to be returned due to lack of
precision, but I expect small time ranges of less than one
day to be of limited use.
`dt:' remains a higher-precision field for searching on both
date and time from the Date: header, while `d:' is now always
date-only.
Eric Wong [Wed, 5 Mar 2025 00:45:36 +0000 (00:45 +0000)]
listener: don't set listen backlog on inherited sockets
By using the listen(2) backlog as-is when inheriting (from
systemd or similar), we can give the sysadmin more control on
controlling overload on a per-listener basis. For systemd
users, this means the `Backlog=' parameter in systemd.socket(5)
can be respected and configured to give certain sockets a
smaller backlog (perhaps combined with with per-listener
`multi-accept' parameter on sockets with the standard (huge)
backlog).
For sockets we create, continue to use INT_MAX and let the
kernel clamp it to whatever system-wide limit there is
(e.g. `net.core.somaxconn' sysctl on Linux).