]> git.ipfire.org Git - thirdparty/public-inbox.git/log
thirdparty/public-inbox.git
4 days agonntpd|imapd|pop3d: wait on writability, first master
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.

7 days agods: long_response: reduce {long_cb} size
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.

7 days agoimap: fetch_compile: more meaningful variable names
Eric Wong [Sat, 28 Jun 2025 11:20:12 +0000 (11:20 +0000)] 
imap: fetch_compile: more meaningful variable names

Match the variable names used by callers to refuse confusion
rather than an ambiguously-named array and meaningless offsets.

7 days agods: long_response: fix bad comment
Eric Wong [Sat, 28 Jun 2025 11:20:11 +0000 (11:20 +0000)] 
ds: long_response: fix bad comment

As far as I can tell, {long} never existed as a field and
it's always meant the callback passed for ->long_response.

7 days agods: support enqueued CODE refs with arguments
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.

13 days agods: flush_write: check {sock} after calling CODE
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.

Followup-to: f7aaea70 (ds: shutdn_tls_step clobbers {wbuf} early, 2025-06-17)
13 days agoinbox: introduce lock_file for inbox_idle
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.

2 weeks agods: avoid ->method dispatch for `print'
Eric Wong [Tue, 17 Jun 2025 00:13:35 +0000 (00:13 +0000)] 
ds: avoid ->method dispatch for `print'

`->method' dispatches have always been slower than directly
invoking Perl ops without `->', so favor the latter.

2 weeks agods: shutdn_tls_step clobbers {wbuf} early
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.

2 weeks agotls: shorten warning $SSL_ERROR prefix
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.

2 weeks agods: confess on epoll_ctl failure
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.

3 weeks agodaemon: introduce register_log public API
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.

3 weeks agodaemon: reopen log files atomically
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.

3 weeks agodaemon: allow `out' directive for HTTP, too
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.

3 weeks agohttp: response_write handles HEAD responses
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:

builder {
enable 'Head';
map 'http://example.com/' => builder {
enable 'AccessLog::Timed', @a_params;
};
map 'http://example.net/' => builder {
enable 'AccessLog::Timed', @b_params;
};
};

...would not log HEAD requests properly, and the correct alternative:

builder {
map 'http://example.com/' => builder {
enable 'AccessLog::Timed', @a_params;
enable 'Head';
};
map 'http://example.net/' => {
enable 'AccessLog::Timed', @b_params;
enable 'Head';
};
};

...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.

3 weeks agotests: disable ContentLength middleware in *.psgi
Eric Wong [Sat, 14 Jun 2025 09:26:31 +0000 (09:26 +0000)] 
tests: disable ContentLength middleware in *.psgi

Our HTTP server code already calculates Content-Length in order to
support persistent connections, so the middleware is redundant.

3 weeks agowww_static: use Plack::Component->to_app
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.

3 weeks agowww_static: support default_type parameter
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.

3 weeks agotreewide: consistent IO::Handle->flush error handling
Eric Wong [Sat, 7 Jun 2025 19:48:37 +0000 (19:48 +0000)] 
treewide: consistent IO::Handle->flush error handling

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.

3 weeks agolei_saved_search: show errno on ->flush failure
Eric Wong [Sat, 7 Jun 2025 19:48:36 +0000 (19:48 +0000)] 
lei_saved_search: show errno on ->flush failure

->flush failures need to describe the nature of the
failure, not just the failing handle.

4 weeks agotest_common: fix xap_helper build reuse
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)
4 weeks agohttp: don't ->close after ->accept_SSL failure
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)
4 weeks agohttp: respect `Connection: close' in HTTP/1.1 requests
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.

4 weeks agods: do_read: return 0 on EOF
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.

4 weeks agofix git permission denied in certain configs
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>
5 weeks agoextmsg: improve handling of obfuscated Message-ID URLs
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.

5 weeks agoxap_helper: mset: ignore non-ultimate exceptions
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.

5 weeks agoxhc_mset: improve error message formatting
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.

5 weeks agotest_common: symlink xap_helper instead of excessive copying
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.

7 weeks agowww: extmsg: dedupe cross-posted Message-IDs
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.

7 weeks agowww: extmsg: async partial Message-ID search
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.

7 weeks agoxap_helper: mset supports multiple requests
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.

7 weeks agowww: extmsg: rely on event loop for partial matches
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.

8 weeks agosha: avoid string eval for loading Net::SSLeay
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.

8 weeks agorename PlackLimiter to PsgiLimiter
Eric Wong [Wed, 7 May 2025 00:16:34 +0000 (00:16 +0000)] 
rename PlackLimiter to PsgiLimiter

`PSGI' is probably a appropriate name since it's the name
of the interface, and `Plack' is merely the implementation
of it.

8 weeks agohttpd: psgix.io is the underlying IO object
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.

2 months agosyscall: fix fstatfs(2) use for x32
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.

2 months agot/netd: skip Linux-only test in some cases
Eric Wong [Fri, 2 May 2025 11:11:11 +0000 (11:11 +0000)] 
t/netd: skip Linux-only test in some cases

Apparently, some ss(8) on some older Linux kernels don't seem to
report Send-Q properly for Unix stream socket listeners.

2 months agosolver: accept extra trailing space in filenames of diffs
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.

c.f. <AM9PR04MB860415AC2AE7973F9F0A4AA095E72@AM9PR04MB8604.eurprd04.prod.outlook.com>

2 months agospawn: avoid string eval for optional BSD::Resource
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.

2 months agorepo_atom: add comments around string eval usage
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'.

2 months agospawn: avoid string eval for optional Inline::C
Eric Wong [Tue, 29 Apr 2025 20:47:44 +0000 (20:47 +0000)] 
spawn: avoid string eval for optional Inline::C

String eval prevents error detection via `perl -c' syntax
checks, so avoid it by trading `use' in favor of `require' +
`->import'.

2 months agotreewide: use autodie::open where possible
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.

2 months agoclone: don't dup(2) stdout FD
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.

2 months agodskqxs: use OnDestroy explicitly for $fork_gen
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').

2 months agotreewide: replace signalfd with epoll_pwait
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.

2 months agodskqxs: ignore all EV_DISABLE failures
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.

2 months agolei: deal with spurious wakeups
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.

2 months agoinput_stream: only rely on events for pipes+sockets
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.

2 months agorename InputPipe to InputStream
Eric Wong [Tue, 29 Apr 2025 17:16:46 +0000 (17:16 +0000)] 
rename InputPipe to InputStream

Calling it a "stream" is more accurate since it can be a stream
socket, FIFO, or even a regular file; not just a pipe.

2 months agot/httpd-corner: improve EPIPE/SIGPIPE handling
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.

2 months agotreewide: remove unnecessary Sigfd use and requires
Eric Wong [Tue, 29 Apr 2025 17:16:44 +0000 (17:16 +0000)] 
treewide: remove unnecessary Sigfd use and requires

signalfd is an implementation detail and its use may be
replaced in future changes.

2 months agolinkify: give <a> tags to gemini:// URLs
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.

2 months agohttp: send `100 Continue' responses for uploads
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.

2 months agodaemon: support backlog= listen parameter
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.

2 months agodaemon: use unpack_sockaddr_* for clarity
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.

2 months agohttp: refuse to deal with >4GB chunks in uploads
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.

3 months agot/httpd-https: test SSL session reuse
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...

3 months agot/httpd-https: fix extra CRLF in request
Eric Wong [Thu, 3 Apr 2025 08:46:18 +0000 (08:46 +0000)] 
t/httpd-https: fix extra CRLF in request

I only noticed this via manual inspection since the test doesn't
reuse the socket afterwards.

3 months agods: close: always call SSL_close on TLS sockets
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.

3 months agodoc: TODO and release notes updates
Eric Wong [Wed, 2 Apr 2025 20:17:54 +0000 (20:17 +0000)] 
doc: TODO and release notes updates

3 months agodoc: mknews: fix uninitialized variable
Eric Wong [Wed, 2 Apr 2025 19:09:49 +0000 (19:09 +0000)] 
doc: mknews: fix uninitialized variable

SCRIPT_NAME is required by Plack, so ensure this mock env
has it, as well.

Fixes: 1b130400 (www: omit SERVER_PORT for standard port redirects, 2025-03-28)
3 months agohttp: support trailers for chunked requests
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.

3 months agohttp: fix and test Trailer: rejection
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.

3 months agot/httpd-corner: modernize test
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).

3 months agohttp: use autodie::open
Eric Wong [Wed, 2 Apr 2025 19:00:12 +0000 (19:00 +0000)] 
http: use autodie::open

A small step towards making fatal messages more consistent.

3 months agodaemon: allow per-listener servername= and serverport=
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.

3 months agowww: omit SERVER_PORT for standard port redirects
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.

3 months agolei ls-mail-source: propagate errors to caller
Eric Wong [Thu, 27 Mar 2025 23:20:47 +0000 (23:20 +0000)] 
lei ls-mail-source: propagate errors to caller

LeiInput already handles exceptions via `die', so just die
rather than using lei->err() to report to stderr without
affecting the exit code.

I noticed this problem while making some changes to our NNTP
implementation.

3 months agonntp: avoid repeated rand() calls
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.

3 months agonntp: avoid modifying $_[0] in RE conversions
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.

3 months agonntp: avoid uninitialized vars from bogus LIST args
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.

3 months agonntp: share common LIST code to reduce duplication
Eric Wong [Thu, 27 Mar 2025 23:20:43 +0000 (23:20 +0000)] 
nntp: share common LIST code to reduce duplication

Deduplicating code here will make a future change to wildmat2re
easier-to-review.

3 months agotests: get rid of most Data::Dumper usage
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.

3 months agot/lg2_cfg: fix RE for multi-line preservation
Eric Wong [Sun, 30 Mar 2025 17:57:12 +0000 (17:57 +0000)] 
t/lg2_cfg: fix RE for multi-line preservation

I forgot to run the test on a machine with libgit2 1.8+ before
pushing :x

3 months agouse git(1) for configs if libgit2 <1.8
Eric Wong [Fri, 28 Mar 2025 03:32:09 +0000 (03:32 +0000)] 
use git(1) for configs if libgit2 <1.8

libgit2 multiline support appeared broken before 1.8.  I noticed
the following commits in libgit2 seemed relevant to us, at least:

dff05bc30 (Multiline config values not preserved on saving, 2021-11-25)
de9a76b92 (config: properly handle multiline quotes, 2023-12-14)

3 months agocontent_digest_dbg: more compact display w/ git_quote
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'.

3 months agosearch: improve xap_helper mset error reporting
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.

3 months agotreewide: use git_quote for diagnostic dumps
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.

3 months agowww: hoist out sanitize_local_paths sub for solver
Eric Wong [Wed, 26 Mar 2025 03:35:00 +0000 (03:35 +0000)] 
www: hoist out sanitize_local_paths sub for solver

SearchView and ViewVCS both benefit from local path sanitation
in diagnostic messages of the WWW UI.

3 months agosolver: use git_quote on filenames for diagnostics
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.

3 months agotest_common: fix require_git skip message
Eric Wong [Sat, 22 Mar 2025 20:55:43 +0000 (20:55 +0000)] 
test_common: fix require_git skip message

We need to use `sprintf("%vd")' to format the required version
v-string for human consumption.

3 months agolimiter: refactor to reduce code duplication
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.

3 months agoqspawn: rename {psgi_env} => {env}
Eric Wong [Fri, 21 Mar 2025 22:22:29 +0000 (22:22 +0000)] 
qspawn: rename {psgi_env} => {env}

There's no need to make a distinction here, and
it will help us make the limiter code more flexible
and reusable in next commits.

3 months agoplack_limiter: PSGI middleware to limit concurrency
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.

3 months agolg2: disable strict hash verification
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.

3 months agowww_static: path_info_raw: support non-HTTP(S) schemes
Eric Wong [Fri, 14 Mar 2025 09:22:05 +0000 (09:22 +0000)] 
www_static: path_info_raw: support non-HTTP(S) schemes

We may add support for gemini:// which supports CGI-like
protocols like PSGI, so relaxing the HTTP(S) URL scheme
requirement seems to make sense.

3 months agodaemon: define %TLS_ONLY hash
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.

3 months agowww: disable legacy encoded Message-IDs for non-v1
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.

4 months agoXapHelper.pm: fix QP_FLAGS initialization
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)
4 months agoxap_helper: drop qp_extra_done flag and conditions
Eric Wong [Thu, 6 Mar 2025 20:34:41 +0000 (20:34 +0000)] 
xap_helper: drop qp_extra_done flag and conditions

As with -d (directories), the -Q (extra query prefixes) flag is
already part of the cache key so there's no need to initialize
it lazily.

While we're at it, drop a `map' op from the cache key generation
for the Perl implementation.

4 months agosearch: unoverload {relevance} in options
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.

4 months agosearch: use common do_enquire for MiscSearch
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.

4 months agolei: use C++ xap_helper if available
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.

4 months agoxap_helper.h: share ThreadFieldProcessor across DBs
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.

4 months agosearch: avoid `git rev-parse' if using C++ xap_helper
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.

4 months agoxap_helper: use libgit2 git_date_parse in C++ impl
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='.

4 months agosearch: make `d:' search prefix consistently date-only
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.

4 months agolistener: don't set listen backlog on inherited sockets
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).

4 months agot/xap_helper: bail on build failure if TEST_XH_CXX_ONLY
Eric Wong [Thu, 27 Feb 2025 00:08:19 +0000 (00:08 +0000)] 
t/xap_helper: bail on build failure if TEST_XH_CXX_ONLY

We shouldn't transparently fall back to the Perl bindings test
on build failures if the tester wants to explicitly test the C++
implementation.