matrix-org / matrix-appservice-irc

Node.js IRC bridge for Matrix
Apache License 2.0
467 stars 152 forks source link

Bad casing on PM nicks can spam create rooms #137

Open kegsay opened 8 years ago

kegsay commented 8 years ago

This happens because the bridge doesn't think there is a PM room as the correct nick is NickServ not nickserv (It is doing a case sensitive comparison when it shouldn't be). It then doesn't queue up the room creation request, so as NickServ responds it, the bridge creates new rooms. We should be queuing them up like we do with processing invites for a room.

lukebarnard1 commented 8 years ago

Can't repro with NickServ

TJuberg commented 8 years ago

I ran into this when joining OFTC the other day and issued a nickname change to a nickname that happened to be registered.

4 or 5 rooms where created, one for each line of text sent by NickServ informing me I was using an already registered nick etc.

Closing all but one and using the last established one to continue my interaction with nickserv did not create any new rooms upon further interactions with nickserv.

kegsay commented 8 years ago

That's probably why @lukebarnard1 couldn't repro - since he already had a mapping with NickServ in the database and/or didn't get multi-line responses. AFAIK this is still a valid issue.

lukebarnard1 commented 8 years ago

Digging around on this, it looks like we'd have to add a new function to matrix-as-bridge RoomStore so that we can get entities by ID in a case-insensitive way...

OR

We could always force the PM room IDs to lowercase upon creation and lookup and then change every database entry ID to be lowercase.

kegsay commented 8 years ago

That doesn't explain why 4-5 rooms are created though (which is presumably due to not locking when creating new PM rooms so they race).

lukebarnard1 commented 8 years ago

Right, so this is two problems in one, solved by:

kegsay commented 8 years ago

Yup, SGTM.