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

Incorrect multi-prefix handling on WHOIS #431

Closed
JustAnotherArchivist opened this issue Jun 11, 2024 · 4 comments
Closed

Incorrect multi-prefix handling on WHOIS #431

JustAnotherArchivist opened this issue Jun 11, 2024 · 4 comments

Comments

@JustAnotherArchivist
Copy link
Contributor

Prior to #430, Solanum was always returning all prefixes, even if a client had not enabled the multi-prefix capability. However, it turned out that the fix there was incorrect due to remote WHOIS not having access to client capabilities, so 746ced2 disabled it altogether and always only returns the highest prefix.

The desired behaviour, of course, is to send the highest prefix to clients without multi-prefix and all prefixes to those with.

@jillest
Copy link
Contributor

jillest commented Jun 12, 2024

Keep in mind what the purpose of the multi-prefix capability is: not breaking old clients while still allowing @+ combinations as much as possible. Sending @+ in WHOIS was implemented long before the multi-prefix capability. Apparently, old ircd developers had found that sending @+ in WHOIS broke few or no clients, while sending @+ in NAMES and WHO did break some clients. This does not seem entirely surprising to me since NAMES and WHO replies are often processed automatically while WHOIS replies tend to be merely displayed.

Since WHOIS has sent @+ for so long, making it follow multi-prefix will not make any meaningful old clients work. Any actively developed clients will support multi-prefix, so the proposed change will not help them.

Also note that WHO will send @+ if WHOX flags are used. That feature was likewise originally implemented (in ircu) before multi-prefix existed.

I think this issue should be closed without any code change.

@dwfreed
Copy link
Member

dwfreed commented Jun 12, 2024

I think this issue should be closed without any code change.

It's a bit late for that. The reversion, instead of going back to the old behavior, changed it so WHOIS never uses multiple prefixes, regardless of capabilities or anything else.

@JustAnotherArchivist
Copy link
Contributor Author

#432 reverted this now and claims that sending all prefixes even if the client hasn't enabled them has 'no known impact on clients that exist today'. I definitely ran into spec compliance walls just like this (albeit not this exact one) when I implemented my own IRC client (bot) from scratch based on the RFC and modern.ircdocs.horse, neither of which allow/document multiple prefixes in RPL_WHOISCHANNELS. I don't think willfully violating the spec is a good idea. But if this is intentional behaviour across independent ircds, it should be documented on modern.ircdocs.horse.

@dwfreed
Copy link
Member

dwfreed commented Jun 13, 2024

Please keep in mind that the RFCs are horribly outdated, and often aren't useful for much more than a general idea on how the protocol works. All ircds do many things that don't comply with the RFCs, which is why ircdocs was created. But ircdocs is also a work in progress, so just because it's not documented there doesn't mean it doesn't happen. I opened ircdocs/modern-irc#239 to request that it be updated to reflect that WHOIS may always be multi-prefix. Notably this also happens in oftc-hybrid, so this behavior is not without precedent.

@dwfreed dwfreed closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants