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

clarify that servers SHOULD send RPL_ENDOFWHOIS #243

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

slingamn
Copy link
Contributor

@slingamn slingamn commented Oct 8, 2024

From discussion on Libera-Chat/sable#101

cc @progval

@slingamn slingamn changed the title clarify that servers SHOULD send RPL_WHOIS clarify that servers SHOULD send RPL_ENDOFWHOIS Oct 8, 2024
@progval
Copy link
Member

progval commented Oct 9, 2024

@spb pointed out that RPL_ENDOFWHOIS is not sent after ERR_NEEDMOREPARAMS

@tommyrot
Copy link
Contributor

tommyrot commented Oct 9, 2024

RPL_ENDOFWHOIS is not sent after ERR_NEEDMOREPARAMS

Not after ERR_NONICKNAMEGIVEN either.

@SadieCat
Copy link
Contributor

SadieCat commented Oct 9, 2024

At least in our implementation we only send ERR_ENDOFWHOIS after a well formed WHOIS request. So you'll only get it after one ERR_NOSUCHNICK or one or more RPL_WHOIS* numerics. I don't think we need to worry too much about clients sending broken responses imo.

@slingamn
Copy link
Contributor Author

Thanks, I updated this to clarify that the SHOULD only applies when the client's message is well-formed.

@progval
Copy link
Member

progval commented Oct 15, 2024

Can/shoukd we also keep it a MUST when there is no <target> (ie. local whois), or should we leave this implementation-specific?

@slingamn
Copy link
Contributor Author

I don't see a compelling reason to change it right now (we would presumably have to do a more detailed investigation of server behaviors, also it's not clear to me that a hard guarantee here would be helpful to clients --- as in the debate over labeled-response, clients should probably implement a timeout regardless?)

I would support opening a follow-up thread to discuss this.

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

Successfully merging this pull request may close these issues.

4 participants