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

libathemecore/ptasks: handle status-prefixed channel messages #823

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 23 additions & 6 deletions libathemecore/ptasks.c
Expand Up @@ -567,17 +567,34 @@ handle_message(struct sourceinfo *si, char *target, bool is_notice, char *messag
if (si->su == NULL)
return;

/* if this is a channel, handle it differently. */
if (*target == '#')
// If the target is a channel, handle it differently
if ((p = strchr(target, '#')) != NULL)
{
handle_channel_message(si, target, is_notice, message);
return;
/* Account for the fact that the message may have a status prefix on it; for example, a message
* to @#channel should still be treated as a message to #channel rather than ignored. -- amdj
*/
bool is_status_prefixed = false;

for (unsigned int i = 0; p == (target + 1) && prefix_mode_list && prefix_mode_list[i].mode; i++)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i feel like the inclusion of static logic in a for condition is needlessly abstruce. this took me 5 minutes to understand

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there much at risk to just do

if ((p = strchr(target, '#')) != NULL)
{
    (void) handle_channel_message(si, p, is_notice, message);
    return;
}

?

it feels like having an octothorpe in a target name when it's not ultimately a channel is a protocol framing issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was proposed about a decade ago, but there was something wrong with it. I forget what it was now, though.

Honestly, it seems fine, we were probably bikeshedding about a non-practical issue.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there much at risk to just do

if ((p = strchr(target, '#')) != NULL)
{
    (void) handle_channel_message(si, p, is_notice, message);
    return;
}

Think of charybdis-family IRCds where you send a PRIVMSG to $#some.mask and #some.mask happens to be an extant channel.

{
if (target[0] == prefix_mode_list[i].mode)
{
is_status_prefixed = true;
break;
}
}

if (p == target || is_status_prefixed)
{
(void) handle_channel_message(si, p, is_notice, message);
return;
}
}

target_u = NULL;
si->service = NULL;
p = strchr(target, '@');
if (p != NULL)

if ((p = strchr(target, '@')) != NULL)
{
/* Make sure it's for us, not for a jupe -- jilles */
if (!irccasecmp(p + 1, me.name))
Expand Down