Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Techcable opened this issue Aug 18, 2022 · 1 comment · Fixed by #373
Closed

Comments

@Techcable
Copy link

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants