cinchrb / cinch

The IRC Bot Building Framework
http://www.rubydoc.info/gems/cinch
MIT License
1k stars 180 forks source link

Mask#nick does not get the right value #209

Closed elifoster closed 8 years ago

elifoster commented 8 years ago

Looking at the source, regex really should not be used for this. The reason is that, first of all, users on webchat's nicknames automatically get turned into "*" according to the method. Along with that, the value that the regexp gets for the nick is actually the username, not the nickname. And even then, it's not the full username most of the time. For example, it would get "LittleHel" rather than "LittleHelper".

My proposed solution would be to somehow, instead of attempting to get the nickname through the hostmask (which is wrong), get a User object by the hostmask. Then you don't even need any of these methods or attr_readers because you can just get them correctly from the User object that you get.

dominikh commented 8 years ago

Instead of your interpretation of the code, could you provide one of the hostmasks you're seeing for which Mask#nick is not working correctly? Thanks.

elifoster commented 8 years ago

I wrote up some code quickly to easily get you relevant information, using the unban event's Ban object. Here is an IRC log with useless stuff stripped.

[13:33:23] LittleHelper [~LittleHel@2601:1c2:e01:3f07:842d:d46f:6d0d:b7f7] has joined #FTB-Wiki-Dev
[13:33:26] <@SatanicSanta> !unban LittleHelper
[13:33:27] <@LittleHelper> Nick: *
[13:33:27] <@LittleHelper> Mask: *!*@2601:1c2:e01:3f07:842d:d46f:6d0d:b7f7
[13:33:41] <@SatanicSanta> !unban xbony2|afk
[13:33:42] <@LittleHelper> Nick: *
[13:33:42] <@LittleHelper> Mask: *!*@cpe-174-108-91-100.carolina.res.rr.com
[13:33:53] <@SatanicSanta> !unban Some-Annoying-Person
[13:33:54] <@LittleHelper> Nick: *
[13:33:54] <@LittleHelper> Mask: *!*@76.115.19.199

LittleHelper is the bot, xbony2|afk is just a random user, and Some-Annoying-person is a webchat user– just so you have some variety ;)

dominikh commented 8 years ago

Numerous things:

get a User object by the hostmask

This is semantically wrong and technically impossible. A ban, whether it is being added or removed, doesn't refer to a specific user. A ban consists of a mask that matches 0..n users. A ban that is being added or removed doesn't need to match any currently present users, either. A ban *!*@* is a valid ban, matching an infinite set of users.

regex really should not be used for this

There is nothing wrong with using regular expressions for this. Masks are regular. A mask consists of a nickname, a username and a hostname, separated by ! and @ respectively, and none of the fields may contain these characters.

the value that the regexp gets for the nick is actually the username, not the nickname

That is false. The mask is nick!user@host. Which is exactly the way the regexp treats the input:

m = Cinch::Mask.new("nick!user@host"); [m.nick, m.user, m.host]
# => ["nick", "user", "host"]

For example, it would get "LittleHel" rather than "LittleHelper".

You haven't yet provided example input that would trigger that problem.

If your complaint is that given an (un)ban event, you want all users in the channel that matched said ban (assuming these people weren't kicked/left and thus are still in the channel), you would use something like the following:

users = m.channel.users.select { |user, _|
  mask.match(user)
}

where mask is the ban mask you receive in the unban event.

If your complaint is that you want the user object(s) matching this mask, even if they aren't in any channels the bot is in, that is not possible. The server won't let you query that.

elifoster commented 8 years ago

That all makes sense.

When I said that thing about LittleHel vs LittleHelper, I was thinking of the line in my previous comment: [13:33:23] LittleHelper [~LittleHel@2601:1c2:e01:3f07:842d:d46f:6d0d:b7f7] has joined #FTB-Wiki-Dev

dominikh commented 8 years ago

Do note that

  1. that's not a full mask as is being parsed by Mask
  2. ~LittleHel is in fact the user name, not the nick. It's prefixed with ~ because there is no identd running. It's truncated because some component (client or server) decided on a maximum length for user names.
elifoster commented 8 years ago

That makes sense. As should be quite obvious, I'm not terribly familiar with high-level IRC stuff.