SrainApp / srain

Modern IRC client written in GTK
https://srain.silverrainz.me/
Other
302 stars 34 forks source link

Srain expects both 903 and 900 as responses to SASL login, but sometimes only 903 is given #371

Closed Techcable closed 1 year ago

Techcable commented 2 years ago

Apparently 903 is the logical response to SASL, while 900 is a separate "state change" event.

Requiring both 903 and 900 works fine when connecting to servers (Libera), but breaks on connection to Soju.

I was told by @progval that this is a bug in srain.

Relevant IRC logs (Aug 9th, 2022):

<Techcable> Here is a better log: https://gist.github.com/Techcable/873cd7609fbdb8a8e39663bc966bfa8b
<Techcable> There are some messages in soju.log, but they don't seem to match up with the time I sent the most recent request....
<Techcable> namely soju says registration complete for user "Techcable" and "failed to write message: use of closed network connection"
<Techcable> Based on ircdog, it appears srain is not sending "CAP END".
<Techcable> That looks like it's required/recommended from here: https://modern.ircdocs.horse/#connection-registrati                                                                                            on
<Techcable> Yes! It appears soju will refuse to complete registration until after its finished negotiating capibilities: https://git.sr.ht/~emersion/soju/tree/master/item/downstream.go#L823-825
<Techcable> I will maybe play more with this?
<val> ah yes
<val> I remember now
<val> Srain expects 903 and 900 as responses, which most servers do
<val> but Soju only returns 903 during handshake
<val> dammit, I told Soju's dev about this
<val> anyway, that's a bug in Srain
<val> 2022-03-18 10:37:13 emersion 903 indicates that the server has processed (and succeeded) the SASL auth
<val> 2022-03-18 10:37:34 emersion 900 on the other hand is more of a "state change" notification
<val> 2022-03-18 10:37:57 emersion 900 can happen without 903, e.g. if the user authenticated via NickServ
<val> 2022-03-18 10:38:39 emersion tl;dr after sending the final AUTHENTICATE command you should wait for 903 and other error numerics
<val> 2022-03-18 10:39:16 emersion 900 can typically be used to update a `bool loggedIn` or `string account` field in the client
<val> 2022-03-18 10:39:40 emersion (and you should reset that field on 901)
<val> (emersion is Soju's dev)
<val> could you open a ticket on Srain's bugtracker?
<val> (feel free to quote this)
<Techcable> absolutely :)
<Techcable> I may even write a patch :)
<Techcable> GUI is much nicer then hexchat (laugh)

Also this patch will probably not come from me anytime soon (I have school work....)

SilverRainZ commented 2 years ago

Thanks for your report. I am not active on IRC recently, so I would love to see you submit a patch,