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.

Sign up for GitHub

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

Jump to bottom

Document PRIVMSG server mask #134

Merged
merged 1 commit into from
Nov 28, 2021
Merged

Document PRIVMSG server mask #134

merged 1 commit into from
Nov 28, 2021

Conversation

emersion
Copy link
Contributor

@emersion emersion commented Oct 8, 2021

No description provided.

@jwheare
Copy link
Contributor

jwheare commented Oct 8, 2021

See also ircdocs/wooooms#5

@emersion
Copy link
Contributor Author

cc @progval: any chance to get this reviewed?

@emersion
Copy link
Contributor Author

emersion commented Nov 2, 2021

cc @progval: does this look good to you?

@progval
Copy link
Member

progval commented Nov 2, 2021

Sorry, I just need to test the existing impls first

@jobe1986
Copy link

jobe1986 commented Nov 2, 2021 •
edited
Loading

Note: there are some implementations that further restrict the format of the server mask specified, for example I have come accross an implementation where "$" was not allowed and a format such as "$.tld" was required instead

@progval
Copy link
Member

progval commented Nov 6, 2021 •
edited
Loading

I just checked support with various servers:

Support $* and $*.tld but not $$* or $$*.tld: Unreal, InspIRCd
Support $*.tld/$$*.tld but not $*/$$* (RPL_NOSUCHNICK): ngircd
Supports only $$*.tld: plexus4 (replies with ERR_NOTOPLEVEL if $$*, and with NOTICE if $*.tld). Additionally, plexus4 requires netadmin status (which is set by services, I believe), not just oper.
Don't support any of them (reply with RPL_NOSUCHNICK): Bahamut, Chary/Solanum, Ergo, irc2, ircu2

So the behavior described in this PR only applies to Unreal and Insp. It could be amended to describe Unreal+Insp+ngirc+plexus4, but is it worth it?

@progval
Copy link
Member

progval commented Nov 6, 2021 •
edited
Loading

Oh also: servers that do support them send messages to all users on the server but the sender.

And some servers rewrite the target (Insp always replaces it with $*, another one I forgot replaces it with a nick)

progval added a commit to progval/irctest that referenced this pull request Nov 6, 2021
@progval
Add tests for PRIVMSG to a server mask
7b27344 
ircdocs/modern-irc#134

A bunch of tests are failing, we need to work this out in the Modern PR

Note: it needs the following patch to plexus4 to be relevant:

```diff
diff --git a/modules/core/m_message.c b/modules/core/m_message.c
index adf0821..7568f20 100644
--- a/modules/core/m_message.c
+++ b/modules/core/m_message.c
@@ -575,7 +575,7 @@ handle_special(int p_or_n, const char *command, struct Client *client_p,
       return;
     }

-    if (MyClient(source_p) && !HasUMode(source_p, UMODE_NETADMIN) && !HasFlag(source_p, FLAGS_SERVICE) && strcmp(nick + 1, me.name))
+    if (false)
     {
       sendto_one(source_p, form_str(ERR_NOPRIVILEGES), me.name, source_p->name);
       return;
```

(I'm too lazy to figure out how to become a netadmin)
@progval progval mentioned this pull request Nov 6, 2021
Closed
@emersion
Copy link
Contributor Author

emersion commented Nov 6, 2021

My main motivation is to add some notes in the spec to tell clients to handle these messages properly, ie. put them in the server buffer if applicable and don't open a new query buffer. Maybe we should only spec the receiving end of this: receiving a message starting with $ means it's a server-wide broadcast, but leave it unspecified how to send them?

@progval
Copy link
Member

progval commented Nov 6, 2021

Sounds good. You could mention in passing what it's for.

ie. put them in the server buffer if applicable and don't open a new query buffer

Just mention it's a possibility, this should be up to the implementation

@emersion emersion force-pushed the privmsg-server-mask branch from c2f3119 to 2f7bada Compare November 7, 2021 10:24
@emersion
Copy link
Contributor Author

emersion commented Nov 7, 2021

Done!

@emersion emersion force-pushed the privmsg-server-mask branch from 2f7bada to 1208cfd Compare November 7, 2021 10:47
@progval
Copy link
Member

progval commented Nov 7, 2021

👍

@progval progval added the feedback wanted We need to make sure this is correct label Nov 7, 2021
@progval progval merged commit 192795d into ircdocs:master Nov 28, 2021
@emersion emersion deleted the privmsg-server-mask branch November 28, 2021 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
feedback wanted We need to make sure this is correct
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@emersion @jwheare @progval @jobe1986