diff --git a/REVIEW.md b/REVIEW.md index dbcda30..4eae429 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -77,8 +77,18 @@ except C-09 (a man page); since nc-oc-ds4.1f-free was out of credits, nc-gpt-sol and nc-oc-gpt61t verified them and cross-checked each other's findings. Those reviews found three more problems in the first versions of #353 and #355, fixed in the same pull requests (in their -rows). #363 only gave a test more time (a TLS test that failed under -`-race` on a busy one-core host); it has no row. +rows). On 2026-11-11 the user asked why #363 had given a TLS test more +time on a busy host, and every pull request that had changed a test was +checked for a test weakened to pass on a busy machine: #363 and #226 +(raised bounds, a dropped check), #83, #170 and #274 had left tests +passing without testing what they guard, and two more tests waited a +fixed time that a slow machine could pass without testing anything. Each +got its check back or now times the event itself, and was shown to fail +with the defect it guards (the fix reverted, or a scratch mutant): rows +#369 and #371 to #376. FreeBSD CI found #367, #368 and the short send fixed in #377. The +user redesigned the wait after failed logins (#370), which replaced #147 +and removed the dummy hash work of #209, #253 and #310. nc-oc-gpt61t +verified them. #365 only updated nntp-testkit; it has no row. Test review items that only added a test or removed duplicate or slow test code, without a defect behind them, have no row: M-08 (#221), L-01 to L-03 (#233), L-10 (#238), G-01 (#239), G-02 (#251), G-03 (#243, #247, #249, @@ -215,7 +225,7 @@ table. | The SIGCHLD handler logged ` returned ` from inside the handler: `Logger` isn't async-signal-safe (realloc, `localtime_r`, ofstream, `syslog`). A session ending with an error while the master logged (the reload line, "accept failed") deadlocked the master in a futex for good: no more clients, a live process for systemd. The handler now only reaps, counts and records (pid, status) in a fixed array, keeps `errno`; the accept loop logs | 5 | review (area 5) | #113 | | The detached daemon (no `-n`) closed fds 0-2 instead of pointing them at `/dev/null`: the listening socket became fd 0, the pid file fd 1, and in a session the client's socket fd 2 (stderr: glibc's abort messages, ASan reports went to the client). `setugid` reported a missing Username/Groupname only on that closed stderr, so the daemon died at once with nothing in the log; it logs through `slog` now too | 5 | review (area 5) | #113 | | Started as root without `Username` (or with only a `Groupname`), the daemon and every session (client input, PAM, upstream TLS) kept running as root silently; `setugid` now logs a warning | 5 | review (area 5) | #113 | -| `TestSpoolDiskFullDaemon` failed now and then, mostly in the `-race` pass on a busy machine: client 1 waits while client 2 works, which took longer than the default ClientTimeout (60 s), so NewsCache ended client 1's session as idle; and a reply deadline of 60 s was too short for 20000 overview records. Test timing only, not a NewsCache bug | 15 | e2e tests (Fedora 44, `go test -race`) | #83 | +| `TestSpoolDiskFullDaemon` failed now and then, mostly in the `-race` pass on a busy machine: client 1 waits while client 2 works, which took longer than the default ClientTimeout (60 s), so NewsCache ended client 1's session as idle; and a reply deadline of 60 s was too short for 20000 overview records. Test timing only, not a NewsCache bug. #373 found that the bounds this gave (180 s per reply, ClientTimeout 600) let a lock held for nearly three minutes pass (see its row) | 15 | e2e tests (Fedora 44, `go test -race`) | #83, #373 | | With `Options starttls`, a 480 to MODE READER (sent before STARTTLS) made `issue()` log in at once (the re-login of #68): User and Password went out in plain text, which anyone in the middle could force by injecting the 480; this undid #16. Also, a server that wants the login before MODE READER got a second login (502) and the connect failed every time | 7 | review (area 7) | #85 | | NEXT and LAST crashed (SIGSEGV) at the first gap in a group that isn't cached (`Options not-cached`): `RNewsgroup::getover` returns NULL for a number without a record, and `ns_lastnext` read `o[0]` | 2 | review (area 2) | #86 | | POST: an article whose input ended before the final "." (the client disconnected in the middle) was posted upstream cut off: `readtext` stops at the end of the input without an error, and `ns_post` didn't check the stream | 2 | review (areas 1 and 7, seen elsewhere) | #87 | @@ -280,7 +290,7 @@ table. | `--with-uniquename` never defined `WITH_UNIQUE_PACKAGE_NAME`: configure.ac tested `$with_uniquenames` (never set) against `no`. Now `--with-uniquename` (any value but `no`) defines it, so newscacheclean and updatenews log under NewsCache's name; `newscache -o` spells it `--with-uniquename` | 14 | review (area 14) | #143 | | The md5auto option was declared as `AC_ARG_WITH(md5_auto)`: configure knew only `--with-md5_auto`, while the help text, README and `newscache -o` say `--with-md5auto` and the test read `$with_md5auto`; `--without-md5auto` was "unrecognized" and `MD5_AUTO` stayed defined, while `--with-md5_auto` (any value) left it undefined, i.e. turned autodetection off. Now `--with-md5auto` as documented (any value but `no` keeps it on), and the help text says that without it every stored hash is checked as MD5 | 14 | review (area 14) | #144 | | PAM got no `PAM_RHOST`: `check_pam_user_pass` called `pam_start` and `pam_authenticate` without the client's address, so host-based PAM rules (pam_access, pam_succeed_if rhost) saw no host and didn't apply. Now `pam_set_item(PAM_RHOST, client address)` comes before `pam_authenticate` (a failure there refuses the login). `configure --with-pam=` (empty service name, `pam_start("")` fails every login) is an error | 3 | review (security pass, F4) | #145 | -| A failed login slept `AuthFailureDelay` (default 15, up to 3600) times the number of failures in the connection, so each guessing connection held a process and its memory for 90 s by default and up to 6 hours. The counter is per connection, so it didn't stop parallel guessing either. Per the user, the delay is now at most 1 s, the same after each failure (default 1; larger configured values count as 1, 0 still allowed); the close after the third failure stays | 3 | review (security pass, F5) | #147 | +| A failed login slept `AuthFailureDelay` (default 15, up to 3600) times the number of failures in the connection, so each guessing connection held a process and its memory for 90 s by default and up to 6 hours. The counter is per connection, so it didn't stop parallel guessing either. Per the user, the delay is now at most 1 s, the same after each failure (default 1; larger configured values count as 1, 0 still allowed); the close after the third failure stays. #370 replaced this: 1, 2 and 4 s after the first, second and third failure, and `AuthMinLength` (see its row) | 3 | review (security pass, F5) | #147, #370 | | MD5 auth-file hashes were wrong on big-endian hosts: md5lib reverses its words only with `HIGHFIRST`, which nothing defined, so every digest was wrong and no MD5 password matched. Now configure runs `AC_C_BIGENDIAN` and md5.cc defines `HIGHFIRST` from `WORDS_BIGENDIAN`. Unit check cMD5 (RFC 1321 vectors, md5crypt); run under qemu-s390x: main fails every vector, the fix passes | 3 | review (area 3) | #148 | | `LimitGroupSize` was applied to upstream GROUP replies but not to LIST ACTIVE entries stored in the active database; after an active-list refresh, LIST ACTIVE, cached GROUP results, and `updatenews -g` could see/use the full range until a GROUP refresh. The limit is now applied while each server's active list is read | 7, 9, 10 | real-upstream report; e2e test | #151 | | `TestPamRhost` logged in as a user that doesn't exist: before Linux-PAM 1.4 (openSUSE Leap 15.6 has 1.3.0) `pam_succeed_if` looks the user up first and refuses an unknown one, so the 127.0.0.1 case got 481. And the PAM tests ran only as root, to write their service file; they now need only write access to `/etc/pam.d` | 15 | full machine run (openSUSE 15.6) | #155 | @@ -289,7 +299,7 @@ table. | TLS connections to upstream servers retried a read or write interrupted by SIGTERM or the alarm (`GNUTLS_E_INTERRUPTED` without the alarm-indicator check of the plain path), and GnuTLS's own pull-timeout function retried `poll()` after EINTR: a session waiting for an upstream TLS server ended only when the server answered | 7 | review of #160 (nc-oc-ds4.1f-free), test by nc-oc-gpt | #160 | | `ListenTo` with an IPv6 address such as `[::1]` failed on a host without a global IPv6 address (a container): `getaddrinfo` was called with `AI_ADDRCONFIG`, which ignores loopback addresses. The listen address is numeric and is now used as given | 5 | CI of #160 (Debian 13 container lane) | #160 | | The listening socket's backlog was 4: a burst of clients overflowed it and FreeBSD reset the extra connections. Now 128; `MaxConnections` still limits the sessions | 5 | CI of #160 (FreeBSD lane) | #160 | -| "No such user" and "wrong password" could be told apart by the time the answer takes: for file and unix authentication an unknown user (or a locked or empty password field) returned before any hash was computed, so a client could probe for user names. Now `dummy_password_check()` hashes the client's password and discards the result in those cases, with a hash taken from the same auth file if there is one, so the reply takes as long as for a wrong password. The test compares the three cases in rotation, so a load change on the host can't skew it (#253) | 3 | review (area 3) | #209, #253 | +| "No such user" and "wrong password" could be told apart by the time the answer takes: for file and unix authentication an unknown user (or a locked or empty password field) returned before any hash was computed, so a client could probe for user names. Now `dummy_password_check()` hashes the client's password and discards the result in those cases, with a hash taken from the same auth file if there is one, so the reply takes as long as for a wrong password. The test compares the three cases in rotation, so a load change on the host can't skew it (#253). Removed in #370 by the user's decision, with `TestAuthUnknownUserTiming`: bots guessing logins cost no hash work, the time of a 481 may tell whether a user name exists, and strong user names and passwords (`AuthMinLength`) are the protection | 3 | review (area 3) | #209, #253; removed in #370 | | `src/crypt.cc` didn't compile and nothing built it: removed. The man page and the sample auth file recommended htpasswd, whose default `$apr1$` hashes libxcrypt can't check; they now name the formats that work: raw MD5 (unsalted, weak), `$6$` (only if the system's `crypt(3)` has it), `$apr1$` is not supported, and say what `--without-md5auto` and the plaintext build option do | 3, 0 | review (area 3) | #181 | | The last case of the remap fix #105: when `lock(UnLock)` went back to an outer lock and `make_current` failed there (the file grew between the inner and the outer lock, or no address space was left), the outer lock stayed held with the mapping reset to NULL: the session read nothing ("no such article") and other sessions waited for the lock. The outer locks are now given up as for a corrupt file, and the next access maps the file again. The unlocked write methods (`sset`, `sdel`, `sprepend`, `sappend`, `sremove`, `sclear`) wrote through the NULL mapping after such a failure (SIGSEGV, in `NVHash::sadd` even into the file header); they throw an `Error` now (#207) | 9 | review (area 9, integration of #105) | #182, #207 | | SIGHUP reload: the startup-only settings changed silently without effect. Now every reload logs one notice per setting (Username, Groupname, PidFile, LogDirectory in a file-log build, ServerType, ListenTo, TLSListenTo, CachePort, Hostname, NiceServer) whose configured value differs from the startup value, until a restart or until the file is reverted. A new Hostname was read but never applied; it and NiceServer are restart-only now (new sessions used the reloaded NiceServer while the master kept the old one). That the running user must be able to read the config and the TLS files is in the man page | 5, 6 | review (area 6, L7); user decisions of 2026-10-29 | #188 | @@ -339,7 +349,7 @@ table. | `TestMessageIDStaleEntry` claimed that newscacheclean prunes a stale history entry, but asked for the ID before running the cleaner (`spool_locate` calls `History::remove` itself) and did nothing without newscacheclean. A separate test creates a stale and a live entry, checks both with the test-only `cHistory --lookup`, runs the cleaner before any reader, and requires the stale record gone and the other unchanged | 15 | test review M-05 | #194 | | Log assertions (about 40 tests) ran only in the two Ubuntu file-log lanes, and no test looked at the default syslog output. The ARM64 lane builds `--without-syslog`, and `TestSyslogMessages` reads a private `/dev/log` in a user namespace: a connect line, a failed login without the password, the startup error for an occupied port | 15 | test review M-06 | #224 | | `TestSpoolDiskFull` and `TestSpoolDiskFullDaemon` only logged whether the tmpfs filled (8 blocks were still free in one case), so they could pass without a full disk. Both require `statfs` to report zero available blocks; the inetd wrapper passes the child's signal status through, so a SIGBUS can't be hidden | 15 | test review M-07 | #196 | -| Tests that asserted too little or too much: stale comments and sleeps before a log read or a negative check, replaced by waits (L-04 to L-08, #226); capability order, the space of an empty HDR value, HELP and long Message-IDs pinned or tolerated against RFC 3977 (L-09, #231); `freeAddr` port race, readiness before the greeting (L-11, #236); a source tree path baked into the test binary, little-endian `.db` layout (L-12, L-14, #234); C++ unit checks that passed with a missing fixture or compared `ARTNUM_LIMIT + 1 > ARTNUM_LIMIT`, and fixtures written to the build directory (L-13, #241); a `ServerHello` parser that rejected an empty session ID and assumed one TLS record (#202) | 15 | test review (L-04 to L-14); Terra's cross-review | #226, #231, #236, #234, #241, #202 | +| Tests that asserted too little or too much: stale comments and sleeps before a log read or a negative check, replaced by waits (L-04 to L-08, #226); capability order, the space of an empty HDR value, HELP and long Message-IDs pinned or tolerated against RFC 3977 (L-09, #231); `freeAddr` port race, readiness before the greeting (L-11, #236); a source tree path baked into the test binary, little-endian `.db` layout (L-12, L-14, #234); C++ unit checks that passed with a missing fixture or compared `ARTNUM_LIMIT + 1 > ARTNUM_LIMIT`, and fixtures written to the build directory (L-13, #241); a `ServerHello` parser that rejected an empty session ID and assumed one TLS record (#202). #226 also dropped `TestDate`'s check that the 501 names DATE's syntax (back in #371) and raised the stalled-TLS-writer test's ClientTimeout from 3 to 6 (back to 3 in #369) | 15 | test review (L-04 to L-14); Terra's cross-review | #226, #231, #236, #234, #241, #202 | | An inetd session (`newscache -i`) that got SIGINT or SIGTERM between two commands waited for the client's next line: the flags were set after the loop checked `Xsignal`, and `cin.getline` then blocked (8 of 1200 test cases on Linux, 1 of 50 on illumos). The session reads through `InetdInputBuffer` now: the signals stay blocked while it checks `alarmed`, and `pselect` unblocks them only for the wait; reads inside a command (a POST's article) give up after ClientTimeout + 10 s, as in the daemon. A compiled-in test hook raises SIGINT in the old window | 1, 5 | `TestAdditionalSignalBehavior` on illumos (#247), diagnosed by nc-gpt-sol | #254 | | Checked, not a defect: locking audit (2026-11-03/04). Every write to a group database, the active list and the history (NVcontainer, NVArray, NVHash, NVListBase) runs under the exclusive lock; the remaining shared-to-exclusive upgrades check again after the upgrade (`corrupt()` by inode under the directory lock, `storechunk`/`prefetchchunk`, `setarticle`'s range check); an unreadable `.art` is unlinked only under the shared lock, when no writer runs; `.art.tmp` is written only under the group's exclusive lock; the post spool is serialized by its two ObjLocks; the fetch and refresh lock files are opened once per process, and no second descriptor of a locked file is closed while the lock is held; history and CServer open their files only after the fork; the signal handlers make only async-signal-safe calls, and the sessions table is read with SIGCHLD blocked; `NVNewsgroup::readoverdb` (exclusive lock across an upstream read) is unreachable since #277; a `.db` or `.history` that newscacheclean or `corrupt()` unlinks stays in use by the sessions that have it open (the cache only; the space is freed when they end) | 5, 9, 11 | Opus auditor | not a defect | | Checked, not a defect: after #276 moved the active list's fetch out of the database lock, several sessions with a stale list could have sent LIST ACTIVE at once. They don't: the validity is checked again once the refresh lock is held, as it was under the database lock before (`TestActiveRefreshOnePerWindow` guards it). Two narrow races of #277 stay as they are: a number missing from a LISTGROUP reply but stored by another session meanwhile is removed (needs a news server farm whose members lag), and a record that `dropbad` removes between the snapshot and the store leaves the chunk marked synced (the next read fetches it again). After #287: when history points at an old number and the copy at a newer number was stored by an older version without a history entry, freeing the old number leaves no location until the next read of the newer copy adds it. `TestUpstreamStalledActive` failed once on a 1 GB pool container at load 20 and didn't reproduce in 29 runs (20 under load, 9 pinned to one busy core), before or after #276 (most likely paging) | 9 | nc-oc-ds4.1f-free, verifier of #277, #283 and #287 | not a defect | @@ -356,7 +366,7 @@ table. | The post spool's lock (`ObjLock`) gave up when a signal interrupted its blocking wait: after SIGTERM a post the upstream server didn't take could fail to reach `.artSpool`, and the client got an error. The blocking calls wait on after EINTR; the lock is held only while one file is stored or removed | 9 | verifier of #270 | #271 | | A daemon session (not inetd) stopped with SIGTERM or SIGINT while a command ran sent neither that command's reply nor the goodbye: the signal set `alarmed`, and the flush at the top of the command loop failed on it, so a client whose post had gone to the spool got no 240 and could post again. Every e2e signal test ran in inetd mode, whose stdio output ignores `alarmed`. The reply and the 400 now share two seconds, armed once, before that flush; the goodbye's `alarm(2)` comes before `alarmed = 0`, so the pending one-second alarm can't set it again in between | 1, 5 | nc-oc-ds4.1f-free's review of #269 (the order), the verifier of #272 (the daemon reply) | #272 | | Eight `NVcontainer` error messages said `NVConatiner::` | 9 | nc-oc-ds4.1f-free's review of #270 | #273 | -| XOVER, XHDR and LISTGROUP held the group's shared lock while they wrote to the client: a client that stopped reading kept it until its send timed out (`ClientTimeout`, 600 s in the sample config), and every session that had to store an article or overview record for the group waited that long. Each chunk (64 records or 64 KB; 4096 numbers for LISTGROUP) is copied under the lock and written with it released. A damaged record in the first chunk is now found before anything went out, so the command is retried on the repaired database; `TestCorruptDBMidListCloses` keeps the close-after-part-of-a-list path covered | 9 | nc-oc-ds4.1f-free, checking the dependency of #270 | #274 | +| XOVER, XHDR and LISTGROUP held the group's shared lock while they wrote to the client: a client that stopped reading kept it until its send timed out (`ClientTimeout`, 600 s in the sample config), and every session that had to store an article or overview record for the group waited that long. Each chunk (64 records or 64 KB; 4096 numbers for LISTGROUP) is copied under the lock and written with it released. A damaged record in the first chunk is now found before anything went out, so the command is retried on the repaired database; `TestCorruptDBMidListCloses` keeps the close-after-part-of-a-list path covered. `TestCorruptDBFuzz` lost its mid-list case with this, and `TestClientStalledReader` passed even when the stalled session never ended; #374 and #377 made them test it again | 9 | nc-oc-ds4.1f-free, checking the dependency of #270 | #274 | | An article fetch from the news server held the group's shared lock for the whole fetch (up to `ReadTimeout`, longer with retries), so writers to the group waited for the news server. The stored copy is looked up under the lock, the article fetched without it, and stored under the exclusive lock, which re-checks the range | 9 | nc-oc-ds4.1f-free | #275 | | A LIST ACTIVE (or a GROUP that needed a refresh) held the active database's exclusive lock while it fetched the list from every news server, so every other session's GROUP and LIST waited up to 5 minutes per server. The list is parsed into memory without the lock and stored under it; a separate refresh lock (`.active.refresh`) keeps one fetch at a time, a session with a list serves it meanwhile, and a session or updatenews waiting for it ends on SIGTERM. A fetch stopped by a signal no longer sets the "server failed, retry later" mark, which made other sessions serve an old or empty list for a minute; a fetch that hits the reply limit or deadline leaves the stored list as it was (a reply cut off midway still stores what was read) | 7, 9, 12 | nc-oc-ds4.1f-free; verifier of #276 (SIGTERM wait, retry mark, an exception leaking the lock); review (updatenews fetching a fresh list on every run, in the branch's first version) | #276 | | Synchronizing a chunk of a group's overview (XOVER, OVER, XHDR, HDR) held the group's exclusive lock while it asked the news server for GROUP, LISTGROUP and OVER, and `updatenews -g` held it around LISTGROUP and every ARTICLE: every reader of the group, even of a cached article, waited for the news server. The news server is asked without the lock and the records are stored under a short one that re-checks the range and the sync state; a one-byte fcntl lock per chunk in `.fetch.lock` keeps one fetch per chunk. `-l` now reserves the chunk instead of locking the group. updatenews -g/-o stop their chunk loops after SIGTERM | 9, 12 | nc-oc-ds4.1f-free (confirmed by test), verifier of #277 (the loops) | #277 | @@ -371,11 +381,11 @@ table. | newscacheclean read a group's range before scanning its directory and unlinked by path at the end of the run: a big article stored during the scan (above the old range) was removed even below `SpoolSize`, and so was one stored again under the same name after the scan. The database is opened before the scan (a damaged one is moved to `.db.bad` first and found), the range read after it, and a file is unlinked only if it still has the scanned device and inode | 9, 11 | Opus auditor | #292 | | `updatenews -g` marked a chunk synced for numbers it hadn't fetched: in a group of more than 10 million numbers the part of the first chunk below the prefetched range, and numbers the index had moved past during the prefetch. The chunk is marked only from where it was fetched. Its test found that `RServer(MPList*)`, a constructor production doesn't use, left the connection pointer and the login state uninitialised (a crash on FreeBSD, where that memory wasn't zero); it initialises every member now | 7, 9, 12 | Opus auditor; the constructor by FreeBSD CI | #293 | | A chunk's sync entry could vouch for numbers the spool never got: a fetch outside the lock whose interval the group's range had left (no overlap) was still marked synced, a partial overlap kept the mark for the records it dropped, and a range shrunk from below and grown back (no race) trusted the old entry. `storechunk` returns early without overlap, and `resize_index` clears the old first chunk's entry when the index grows down from a first number that isn't the chunk's start | 9 | nc-oc-gpt61t; the partial and no-race cases by the cross-review | #294 | -| Flaky tests: `TestSigchldStress` read the config while the test rewrote it and a SIGHUP of the burst was still pending (#170); the stalled-TLS-writer test read the log once before the line was written (#174) and counted the readiness session as a second child (#244); `TestUpdatenewsSigpipe` expected one GROUP per group (#187, product cause in #200); `TestAuthUnknownUserTiming` took its samples in blocks (#253); `TestGroupUpstreamFault403` took its dead upstream server's port from `freeAddr`, which frees it again, and its own fake upstream got the same port (#258) | 15 | CI, full machine runs | #170, #174, #244, #187, #253, #258 | +| Flaky tests: `TestSigchldStress` read the config while the test rewrote it and a SIGHUP of the burst was still pending (#170; its repeated SIGHUP let a lost reread pass, fixed in #372); the stalled-TLS-writer test read the log once before the line was written (#174) and counted the readiness session as a second child (#244); `TestUpdatenewsSigpipe` expected one GROUP per group (#187, product cause in #200); `TestAuthUnknownUserTiming` took its samples in blocks (#253); `TestGroupUpstreamFault403` took its dead upstream server's port from `freeAddr`, which frees it again, and its own fake upstream got the same port (#258) | 15 | CI, full machine runs | #170, #174, #244, #187, #253, #258 | | CI: `TestUpstreamActiveReplyLimits` read the file log in a syslog build and failed there (#240); `TestUpdatenewsPrefetchFileNotRegular` used `syscall.Mkfifo`, which doesn't exist for illumos, so the optional matrix failed on every run since #214 (#229); a corrupt `dist` artifact download failed four matrix jobs with `digest-mismatch` (#246); a failed e2e run printed only the last 100 log lines, not the failed test (#242); `cCorruptDB` hung on GNU/Hurd, where fcntl locks belong to the open file and not to the process (#257); the unit checks printed six `-Wwrite-strings` warnings (#259); the FreeBSD and distro-container jobs showed only the last 60 lines of a failed run, so a test that failed early wasn't named (#278); that container job ran `make check` under a shell as PID 1, which doesn't reap orphaned processes (`--init`, #279); the SHA-pinned actions and the e2e Go module get weekly Dependabot pull requests (#280) | 15 | CI, pool runs, the Hurd VM, `make distcheck`, diablo-nntp's CI | #240, #229, #246, #242, #257, #259, #278, #279, #280 | | With `Options starttls`, a reply shorter than a status code (to MODE READER, STARTTLS or LIST OVERVIEW.FMT; anyone in the middle can send one) or a refused login made `issue()`/`authenticate()` throw inside `connect()`, and the half set-up stream stayed: `is_connected()` was true, the next command went out on it in plain text without STARTTLS or the login, and a 480 to it sent User and Password in plain text. A `StreamGuard` deletes the stream when `connect()` is left before its set-up is done | 7 | full review 2026-11-04 | #296 | | A POST with a second `Newsgroups` header went upstream with it: the PostTo check and the choice of servers read only the first, so a client allowed only `local.*` could post anywhere the upstream accepts. A header RFC 5536 3.1/3.2 (and RFC 5322 3.6) allows once that appears twice gets 441 and is logged; `Keywords` may repeat | 2 | full review 2026-11-04 | #300 | -| With `--enable-plainpass`, every login against a plain-text auth-file field first ran `crypt()` on it, which logged "crypt() can't use the stored hash" at level Error where the field can't be a salt. A plain field is compared without `crypt()`, after the same dummy hash work an unknown user gets, so the timing doesn't tell the two apart | 3 | full review 2026-11-04 | #310 | +| With `--enable-plainpass`, every login against a plain-text auth-file field first ran `crypt()` on it, which logged "crypt() can't use the stored hash" at level Error where the field can't be a salt. A plain field is compared without `crypt()`, after the same dummy hash work an unknown user gets, so the timing doesn't tell the two apart (the dummy hash work went in #370) | 3 | full review 2026-11-04 | #310 | | Generated Message-IDs always ended in `$1@`: `TOBASE36` divides its argument in place, and the static counter was passed to it, so two posts without a Message-ID in one session in the same second (to several servers) got the same ID and the second was taken for a duplicate (STAT 223) and dropped while the client got 240 | 7 | full review 2026-11-04 | #299 | | The article after POST had no overall deadline: the command's alarm ends with the command line, and only the per-read timeout applied, which every byte restarts, so a client sending a byte now and then held a session (and a `MaxConnections` slot) for ever. The whole article must arrive within `ClientTimeout` after the 340, else 400 and the session ends | 2 | full review 2026-11-04 | #301 | | Client wildmats weren't RFC 3977 4.2 wildmats: `LIST ACTIVE comp.*,alt.*` and `LIST NEWSGROUPS` with a list matched nothing (single-pattern `wildmat()`), and `LIST ACTIVE.TIMES *.test` parsed its argument as an access rule list, listing every group the client may see. `wildmat_list()` (comma list, `!`, last match wins) serves all three; `NewsgroupFilter`'s copy constructor keeps the wildmat. XPAT keeps its RFC 2980 patterns joined by spaces, as INN does | 10 | full review 2026-11-04 | #302 | @@ -428,6 +438,17 @@ table. | An active-list stamp in the future (the clock was put back) kept the list fresh until the clock got there, and a refresh couldn't lower it: new groups stayed invisible (C-08; #328 fixed this per group). A stamp still to come is stale, also the overview's per-chunk stamps | 9 | Opus review 2026-11-09 | #360 | | newscache.conf(5) called a server's Read "wildmat patterns": routing takes names that may end in `*` and start with `!`, the longest match wins, a `*` anywhere matches the rest and `?` and `[...]` are literal (C-09). Man page and manual say so | 0, 6 | Opus review 2026-11-09 | #357 | | With `Options compress`, a valid reply was discarded (403) when more than 64 KiB of compressed data that inflates to nothing (empty deflate blocks) came before it (G-01). The limit is a time now: ReadTimeout without any content | 7 | GLM review | #361 | +| `TestMaxConnectionsPerHost` failed on FreeBSD, where the test doesn't wait for its process: the harness's readiness connection was still ending when the test connected, and its helper retried only MaxConnections' "400 too many users", not the per-host refusal. Each test names the one refusal it retries. Test only | 15 | FreeBSD CI | #367 | +| `TestNewscachecleanEvictionOrder` failed on ZFS (FreeBSD CI): it measured the files' allocated blocks before ZFS had written them and set SpoolSize from the smaller numbers, so newscacheclean removed every group. It syncs and measures until the numbers are stable. Test only | 15 | FreeBSD CI | #368 | +| The stalled-TLS-writer test timed the test's own fake news server, not the session: the reply came from the fake upstream inside the test process, which under `-race` took the session's end from 7.4 to 9.6 s also on an idle host, and #226 and #363 raised its bounds for that (ClientTimeout 3 to 6, then the end bound from 11 to 14 s), blamed on a busy host. It fills the cache first and times the session from the stalled reply: ClientTimeout 3, the session ends after about 4 s, bounds 6 s for the end and 2 s for the second client's wait; with 763126c reverted it fails at 13 s | 15 | the user's question about #363 | #369 | +| Failed logins (redesign by the user, after #366 was declined): a failed AUTHINFO PASS waits 1, 2 and 4 s, counted per connection, and the third closes the connection; a successful login doesn't wait. `AuthFailureDelay` 0 turns the wait off, any other value on. New `AuthMinLength` (default 10 bytes, 0 off) refuses a shorter user name or password with 481 after the wait, without a lookup or PAM call. No dummy hash work (#209, #253, #310 removed): bots guessing logins cost no CPU for it, and the time of a 481 may tell whether a user name exists; the manual tells admins to use strong names and passwords | 3 | user decision 2026-11-11 | #370 | +| `TestDate` no longer checked that the 501 for "DATE x" names DATE's syntax (the reply once named GROUP's): #226 dropped the check. It is back | 15 | test review 2026-11-11 | #371 | +| `TestSigchldStress` sent SIGHUP again and dialled every 200 ms for up to 5 s (#170), so a lost or late reread passed: a daemon that noticed SIGHUP only with the next connection passed. One SIGHUP; with a log file the reread must be logged within 1 s (9 to 46 ms measured) and the first session after that line must get its greeting; with syslog, whose lines the test can't read, a session must get the 200 greeting within 1 s of the SIGHUP | 15 | test review 2026-11-11 | #372 | +| `TestSpoolDiskFullDaemon` allowed 180 s per reply and ClientTimeout 600 (#83), so a lock held for nearly three minutes passed. The slow part was the second client's last XOVER fetching 10000 records again from the test's own fake server (20 s under `-race`). The group ends in a chunk of 11 records that can't be kept either, so nothing slow is left in the measured part: every reply on the full disk within 5 s (at most 0.14 s measured), default ClientTimeout | 15 | test review 2026-11-11 | #373 | +| `TestCorruptDBFuzz` no longer reached a damaged record in the middle of a list: since #274 its 21 records were found damaged before anything was sent, and the check had become a log line. The group has 201 records, more than the 64 formatted at a time, so damage past the first chunk closes the session after part of a list (75 of the 150 damaged sessions), and the check fails again when none does | 15 | test review 2026-11-11 | #374 | +| `TestXoverStalledClientDoesNotHoldGroupLock` stored one article 700 ms after the first client's XOVER: on a slow machine the first session hadn't reached its records yet, and the test passed without testing anything (a scratch build holding the lock across the write, one second late, passed). The second client stores one article after the other until the first session has ended, which happens only when its send times out, so the stores cover the whole stall on every system with /proc or pgrep; that build fails at about 3 s | 15 | test review 2026-11-11 | #375 | +| `TestActiveRefreshBlocksGroup` sent the waiting session's GROUP 300 ms after the refresh's request: when the refresh began later, the GROUP ran first and the test passed without testing anything (a build holding the lock across the fetch, half a second late, passed). The GROUP starts once the news server has the refresh's LIST ACTIVE | 15 | test review 2026-11-11 | #376 | +| A client that stopped reading in the middle of a reply could keep its session for two ClientTimeouts (FreeBSD: 6.0 s with ClientTimeout 3): when the send timeout runs out after `send()` has taken part of the data, Linux and FreeBSD return that part instead of an error, and `sockstreambuf::sync()` sent the rest with a second full wait. After a short send on a socket with a send timeout, the rest goes out only if the socket takes more within what is left of that timeout; otherwise the write fails like a failed send. This applies to upstream connections (ReadTimeout) too. `cReplyBudget` checks it (2.07 s before, 1.0 s after); `TestClientStalledReader` fills the cache first and wants the session gone within 1.5 ClientTimeouts of the request again (it had only checked that another client was served) | 7 | FreeBSD CI, on the first version of the test | #377 | ## Open points