sorcix / sIRC

sIRC is a general purpose IRC library for Java, to be used as a foundation for IRC clients or bots. Using an event-driven architecture for recieving messages and a easy way of sending messages, sIRC simplifies the task of working with IRC servers.
https://vic.demuzere.be/projects/sirc/
Other
50 stars 18 forks source link

Error in ban command #5

Open JamoBox opened 11 years ago

JamoBox commented 11 years ago

Line 91 in Channel.java appears to try and ban a user by their ident, however it is getting their nick instead of their username. This can cause problems when people have different nicks from their usernames.

Line 91:

 this.setMode("+b *!" + user.getNick() + "@*");

When banning code>foobar!Username@somehost.com</code in this way it will not effect the user "foobar" as it does not match his ident.

To correct this, you should either use the getUserName() method instead of the getNick() method, or change the ban string so it bans the user's nick instead:

this.setMode("+b " + user.getNick() + "!*@*");
sorcix commented 10 years ago

You're right, I'll update this, thanks!

sorcix commented 10 years ago

Some User objects contain only a nickname, so using getUserName() is not an option. If more user details are available, banning on hostname will work. I don't think it's necessary to check if getUserName() is available.

JamoBox commented 10 years ago

Some User objects contain only a nickname - Couldn't data such as usernames be populated by a simple whois to the server about the client? Only considering this as nick bans are generally very weak and bypassed by a simple nick change.

sorcix commented 10 years ago

In that case we need to delay the actual ban command and wait for whois response, but that would work.

Another way of doing this is improving the user list in channels. Currently sIRC keeps track of user modes for every user inside a channel, and we have full identification for users that joined after we did. However, the userlist sent when we join the channel only mentions nicknames. We could update that list when the user says something (as messages contain identification).

sorcix commented 10 years ago

And currently the ban method doesn't even check if there is information about the user in the userlist. That definitely has to change.

JamoBox commented 10 years ago

I think it would be a better idea pro-actively looking for user data separately rather than waiting for a whois response in the ban method. When a ban is issued it should be carried out as fast as possible, therefore waiting for the whois response inside the ban method itself seems a bad idea.

I'm not sure of the efficiency of this, but I know some IRC clients will populate user information by sending a whois to the server with a comma-separated list of all names on the channel it just joined (keeping in mind the number of users in the channel should be considered before doing this). In the information the IRC server will reply will be the RPL_WHOISUSER response which will have the user's username in.

The format of the RPL_WHOISUSER looks like this : "<nick> <user> <host> * :<real name>".

I'm not sure if there is a way to just extract this information to save doing a full whois, but the above should be worth considering.

src: https://www.rfc-editor.org/rfc/rfc1459.txt