slopnode/bot
Watch
2
0
Fork
You've already forked bot
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Tolerate UnrealIRCd's trailing space in the CAP ACK #8

Merged
drawblank merged 1 commit from fix/cap-ack-trailing-space into main 2026-08-20 11:18:14 +00:00
Conversation 0 Commits 1 Files changed 2 +228
drawblank commented 2026-08-20 11:17:50 +00:00
Owner
Copy link

The actual root cause of the registration stall, found in the DEBUG capture:

FROM SERVER: :hertz.slopnode.net CAP zuckerberg ACK :sasl 
arguments: ['ACK', 'sasl ']

UnrealIRCd puts a trailing space inside the final parameter, so the parsed argument is 'sasl '. The library's step tests:

if event.arguments[0] == "ACK" and 'sasl' in event.arguments:

'sasl' in ['ACK', 'sasl '] is False. Neither the ACK nor the NAK branch runs, _sasl_step stays parked on _sasl_cap_req, AUTHENTICATE is never sent, and registration stalls until the server times the connection out. The log shows precisely that — nothing at all after the PONG.

Its sibling _sasl_cap_ls splits the same field on whitespace, which is why CAP REQ goes out fine. Only this one step is intolerant, so splitting the capability list the same way is the entire fix.

It's installed on the connection instance because _sasl_cap_ls advances with self._sasl_step = self._sasl_cap_req — the instance attribute wins, and there's a test pinning that assumption.

Replaying the server's captured lines verbatim

BEFORE (what your server saw)
  -> CAP REQ sasl
  -> PONG 7CBCF5B5
  registration completes? False

AFTER  (with the fix)
  -> CAP REQ sasl
  -> PONG 7CBCF5B5
  -> AUTHENTICATE PLAIN
  -> AUTHENTICATE AHp1Y2tlcmJlc...  <- credentials
  -> CAP END
  registration completes? True

A bug in my own patch, caught by the test

My first version advanced to _sasl_auth_sent instead of _sasl_auth_plain. That sends AUTHENTICATE PLAIN and then waits forever for a 903 that cannot arrive, because the credentials are never sent — a different hang at a later point.

A test asserting only "AUTHENTICATE PLAIN was sent" would have passed and shipped it. The end-to-end test that drives the whole exchange to CAP END failed instead.

Verification

420 tests, up from 407. They cover both ACK spellings, NAK, malformed and non-CAP events, and pin the library's unstripped membership test — so when upstream fixes it, CI tells us the patch can be dropped rather than leaving it to rot.

The actual root cause of the registration stall, found in the DEBUG capture: ``` FROM SERVER: :hertz.slopnode.net CAP zuckerberg ACK :sasl arguments: ['ACK', 'sasl '] ``` UnrealIRCd puts a **trailing space** inside the final parameter, so the parsed argument is `'sasl '`. The library's step tests: ```python if event.arguments[0] == "ACK" and 'sasl' in event.arguments: ``` `'sasl' in ['ACK', 'sasl ']` is `False`. Neither the ACK nor the NAK branch runs, `_sasl_step` stays parked on `_sasl_cap_req`, **AUTHENTICATE is never sent**, and registration stalls until the server times the connection out. The log shows precisely that — nothing at all after the PONG. Its sibling `_sasl_cap_ls` splits the same field on whitespace, which is why `CAP REQ` goes out fine. Only this one step is intolerant, so splitting the capability list the same way is the entire fix. It's installed on the connection instance because `_sasl_cap_ls` advances with `self._sasl_step = self._sasl_cap_req` — the instance attribute wins, and there's a test pinning that assumption. ## Replaying the server's captured lines verbatim ``` BEFORE (what your server saw) -> CAP REQ sasl -> PONG 7CBCF5B5 registration completes? False AFTER (with the fix) -> CAP REQ sasl -> PONG 7CBCF5B5 -> AUTHENTICATE PLAIN -> AUTHENTICATE AHp1Y2tlcmJlc... <- credentials -> CAP END registration completes? True ``` ## A bug in my own patch, caught by the test My first version advanced to `_sasl_auth_sent` instead of `_sasl_auth_plain`. That sends `AUTHENTICATE PLAIN` and then waits forever for a `903` that cannot arrive, because the credentials are never sent — a *different* hang at a later point. A test asserting only "AUTHENTICATE PLAIN was sent" would have passed and shipped it. The end-to-end test that drives the whole exchange to `CAP END` failed instead. ## Verification 420 tests, up from 407. They cover both ACK spellings, NAK, malformed and non-CAP events, and pin the library's unstripped membership test — so when upstream fixes it, CI tells us the patch can be dropped rather than leaving it to rot.
Root cause of the stall, from the DEBUG capture:

    FROM SERVER: :hertz.slopnode.net CAP zuckerberg ACK :sasl
    arguments: ['ACK', 'sasl ']

UnrealIRCd puts a trailing space inside the final parameter, so the parsed
argument is 'sasl ' rather than 'sasl'. The library's step tests

    if event.arguments[0] == "ACK" and 'sasl' in event.arguments:

which is False against ['ACK', 'sasl ']. Neither the ACK nor the NAK branch
runs, _sasl_step stays parked on _sasl_cap_req, AUTHENTICATE is never sent,
and registration stalls until the server times the connection out. The log
shows exactly that: nothing at all after the PONG.

Its sibling _sasl_cap_ls splits the same field on whitespace, which is why
CAP REQ went out fine. Only this one step is intolerant, so the fix is to
split the capability list the same way. Installed on the connection instance,
because _sasl_cap_ls advances with `self._sasl_step = self._sasl_cap_req` and
therefore picks up the instance attribute.

Replaying the server's captured lines verbatim, trailing space included:

    before:  CAP REQ sasl -> PONG -> (nothing)
    after:   CAP REQ sasl -> PONG -> AUTHENTICATE PLAIN
             -> AUTHENTICATE <credentials> -> CAP END

My first version advanced to _sasl_auth_sent instead of _sasl_auth_plain,
which sends AUTHENTICATE PLAIN and then waits forever for a 903 that cannot
arrive because the credentials were never sent. The end-to-end test caught
it; a test that only checked "AUTHENTICATE PLAIN was sent" would not have.

Tests: 420, up from 407. They pin the library's unstripped membership test so
that when upstream fixes it, CI says the patch can be dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
slopnode/bot!8
No description provided.