Repository navigation
A pile of IRC protocol fixes - #2
Merged
Merged
Conversation
Every CAP subcommand got "CAP * LS", including CAP END. Some clients answer an unsolicited LS with another CAP END, so the two loop forever. Reply to LS/LIST, NAK any REQ, and stay silent on END. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The NICK message only went to channel members, so a user in no channels never learned their new nick. Their client then ignored its own JOIN and showed the NAMES list without the channel. Also allow case-only nick changes: the user collided with themselves, and the channel map update deleted the key it had just written. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
User is written from its own connection goroutine, the TNC goroutine and other users' goroutines, but bufio.Writer is not safe for that. NAMES and USERHOST were also written in pieces, so a concurrent line could splice into the middle of them. Lock User.Write and write each line in one call. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Parse required exactly three fields split on '!' and '@'. A real name that was empty or contained '@' or '!' made receivers drop every transmission from that user. Split on the first '!' and '@' instead, and have ID() replace '!'/'@' and use '*' for an empty host so stations that have not updated can still parse it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
userPurge compared LastSeen, which stays zero for a local user who has not spoken, so idle local users were removed from channels within an hour while their client still showed them joined. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A radio message whose nick matched a local user was attributed to the local user: it was hidden from them and retransmitted, which could loop between two stations using the same nick. Leave the radio user untracked in that case, and compare users by pointer in send() and partChannel(). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Channel.Nick read ch.Users under the channel lock while writers hold the server lock, which can crash with a concurrent map access. The partedChannels append in PART also ran without a lock. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
partedChannels holds channel keys, but send() compared them against the target as typed, so a parted user still got messages for mixed-case channels. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Write errors to the TNC were ignored, so a failed transmission looked like a message that did nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FREQUENCY, SETHARDWARE and -sethardware blocked forever waiting for a reply that most TNCs, direwolf included, never send. That froze the client, or the whole server at startup. Wait 2s instead. Also: SETHARDWARE no longer sends its own command name in the frame, and both commands reject missing arguments. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The announce went out when the TCP connection opened, before NICK and USER, so it carried no callsign. That is unidentified traffic, and receivers dropped it anyway. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
QUIT went through send() once per channel, producing ":id QUIT #chan :reason" (clients read #chan as the reason) and transmitting each copy over the air, where receivers discard QUIT. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Channel members got an unsolicited 332 numeric instead of a TOPIC message, which clients don't treat as a change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"JOIN #chan key" was refused as missing parameters. Channels without a leading # were created, but messages to them were routed as private messages and went nowhere. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Owner
|
@arodland: Just read over everything. Looks god to me. Out of curiosity where are you and what band are you using hamirc on? I am yet to use hamirc with another human. |
Contributor
Author
|
I'm using it on 20m and 40m with github.com/arodland/Data2G (experimental), though I expect it also works with VARA's KISS TNC. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi,
I'm not sure if you appreciate this big pile of patches, and I'm not sure if you're okay with them being Claude-generated, but I think that hamirc is cool and I want to be able to use it with a modem that I'm working on, and these fixes made it so that I can actually get on air with it. If you would like to see them split out or done differently, let me know in comments.
The first two commits are the most important:
The remainder are a mix of relatively minor protocol-correctness issues, and fixes for relatively rare (but still worth fixing) races.