PRIVMSG nick@server is broken

[aaronmdjones]

solanum/modules/core/m_message.c

Lines 948 to 992 in 52f1947

	if((server = strchr(nick, '@')) != NULL)
	{
	if((target_p = find_server(source_p, server + 1)) == NULL)
	{
	sendto_one_numeric(source_p, ERR_NOSUCHSERVER,
	form_str(ERR_NOSUCHSERVER), server + 1);
	return;
	}
	if(!IsOper(source_p))
	{
	if(strchr(nick, '%') || (strncmp(nick, "opers", 5) == 0))
	{
	sendto_one_numeric(source_p, ERR_NOSUCHNICK,
	form_str(ERR_NOSUCHNICK), nick);
	return;
	}
	}
	/* somewhere else.. */
	if(!IsMe(target_p))
	{
	sendto_one(target_p, ":%s %s %s :%s",
	get_id(source_p, target_p), cmdname[msgtype], nick, text);
	return;
	}
	/* Check if someones msg'ing [email protected] */
	if(strncmp(nick, "opers@", 6) == 0)
	{
	sendto_realops_snomask(SNO_GENERAL, L_ALL, "To opers: From: %s: %s",
	source_p->name, text);
	return;
	}
	/* This was not very useful except for bypassing certain
	* restrictions. Note that we still allow sending to
	* remote servers this way, for messaging pseudoservers
	* securely whether they have a service{} block or not.
	* -- jilles
	*/
	sendto_one_numeric(source_p, ERR_NOSUCHNICK,
	form_str(ERR_NOSUCHNICK), nick);
	return;
	}

Line 971 uses the whole value of nick (i.e. the @ is not replaced with \0), so on the destination server, it will hit line 989.

[aaronmdjones]

This should be adjusted to confirm that the nick is on the server in question, and then strip the @. This will also avoid handle_special getting called unnecessarily on every server in the chain.

[aaronmdjones]

Alternatively, if we only want to support this for services, which I think is what the comment at the bottom is trying to get at, we should make the code reflect that reality: check if the server is u:lined, check if the nick is on it, and unconditionally reply with ERR_NOSUCHNICK otherwise on the sending client's server. Again there's no real need to call this on every server in the chain.

[edk0]

The comment there seems like an entirely reasonable rationale for the current behaviour; we don't support it as a destination but do relay it for the benefit of pseudoservers that do. I don't think there's any particular need to assume everything with a service block supports user@server. We could make the error message in this case indicate that user@server isn't supported for normal users.