ElementalAlchemist / txircd

Modular IRCd built using Twisted. Made to be extremely customizable.
BSD 3-Clause "New" or "Revised" License
19 stars 5 forks source link

Fix a bug where whowas tries to add a user with no nick #7

Closed ekimekim closed 10 years ago

ekimekim commented 10 years ago

If a user times out on registering a nick (_timeoutRegistration), then whoWas.addUserToWhowas gets called but user.nick is None. This causes an exception like so:

ERROR:twisted:Unhandled Error Traceback (most recent call last): File "/var/env/txircd/lib/python2.7/site-packages/twisted/application/app.py", line 392, in startReactor self.config, oldstdout, oldstderr, self.profiler, reactor) File "/var/env/txircd/lib/python2.7/site-packages/twisted/application/app.py", line 313, in runReactorWithLogging reactor.run() File "/var/env/txircd/lib/python2.7/site-packages/twisted/internet/base.py", line 1192, in run self.mainLoop() File "/var/env/txircd/lib/python2.7/site-packages/twisted/internet/base.py", line 1201, in mainLoop self.runUntilCurrent() --- --- File "/var/env/txircd/lib/python2.7/site-packages/twisted/internet/base.py", line 824, in runUntilCurrent call.func(_call.args, _call.kw) File "/var/env/txircd/txircd/txircd/user.py", line 203, in _timeoutRegistration self.disconnect("Registration timeout") File "/var/env/txircd/txircd/txircd/user.py", line 193, in disconnect self.ircd.runActionStandard("quit", self, reason, users=self) File "/var/env/txircd/txircd/txircd/ircd.py", line 486, in runActionStandard action[0](params, **kw) File "/var/env/txircd/txircd/txircd/modules/rfc/cmd_whowas.py", line 40, in addUserToWhowas lowerNick = ircLower(user.nick) File "/var/env/txircd/txircd/txircd/utils.py", line 16, in ircLower return string.lower().replace("[", "{").replace("]", "}").replace("\", "|") exceptions.AttributeError: 'NoneType' object has no attribute 'lower'

ekimekim commented 10 years ago

DO NOT PULL. Look at problem in second commit and tell me what behaviour would be best.

ElementalAlchemist commented 10 years ago

Do you know the message on which it was choking?

ekimekim commented 10 years ago

Actually, no I don't. But looking at the code I can easily construct a message where it occurs. Specifically, looking at twisted.words.protocols.irc.parsemsg, it occurs when a line starts with : but doesn't contain any spaces, eg. ":baddata\r\n"

ElementalAlchemist commented 10 years ago

Anything that sends that is absolutely not conforming to IRC. I would argue that's a Twisted bug and we (also arguably) don't need to worry about it. I'll leave it up to you how to deal with it. Since dataReceived isn't guaranteed to have an integral number of lines (i.e. it can process up to the middle of a line if it wants), just passing is likely to lead to a garbage command in the future and simply dropping a number of commands in the meantime, so a disconnection is probably for the best. You can disconnect a user using the disconnect method, such as (since you're already in the IRCUser class) self.disconnect("Invalid data").

ekimekim commented 10 years ago

Fixed. And yes, it's technically a twisted bug, but we don't want to upgrade or start shipping a custom version of twisted, so we're stuck with needing to handle it ourselves.

Heufneutje commented 10 years ago

By the way, why are we not upgrading to the latest version of Twisted?

ElementalAlchemist commented 10 years ago

There wasn't particular benefit, so I never bothered. I don't see why not I guess. Mostly I'm waiting on this to be merged.