IRC bot gives 'ValueError' when fed a singlequote

[tycho]

Example:

<neunon> CrawlBuild, force build crawl-debug-x86_64-centos attempt to bomb with rvollmert's latest changes
<CrawlBuild> Something bad happened (see logs): <type 'exceptions.ValueError'>

Gives:

2009-10-22 00:45:22-0700 [IrcStatusBot,client] irc command force
2009-10-22 00:45:22-0700 [IrcStatusBot,client] Unhandled Error
        Traceback (most recent call last):
          File "/usr/lib/python2.6/site-packages/twisted/words/protocols/irc.py", line 1484, in lineReceived
            self.handleCommand(command, prefix, params)
          File "/usr/lib/python2.6/site-packages/twisted/words/protocols/irc.py", line 1496, in handleCommand
            method(prefix, params)
          File "/usr/lib/python2.6/site-packages/twisted/words/protocols/irc.py", line 1041, in irc_PRIVMSG
            self.privmsg(user, channel, message)
          File "/usr/lib/python2.6/site-packages/buildbot/status/words.py", line 724, in privmsg
            contact.handleMessage(message, user)
        --- <exception caught here> ---
          File "/usr/lib/python2.6/site-packages/buildbot/status/words.py", line 635, in handleMessage
            meth(args.strip(), who)
          File "/usr/lib/python2.6/site-packages/buildbot/status/words.py", line 380, in command_FORCE
            args = shlex.split(args) # TODO: this requires python2.3 or newer
          File "/usr/lib/python2.6/shlex.py", line 279, in split
            return list(lex)
          File "/usr/lib/python2.6/shlex.py", line 269, in next
            token = self.get_token()
          File "/usr/lib/python2.6/shlex.py", line 96, in get_token
            raw = self.read_token()
          File "/usr/lib/python2.6/shlex.py", line 172, in read_token
            raise ValueError, "No closing quotation"
        exceptions.ValueError: No closing quotation
        

Should I be reporting this to Twisted or can you guys work around this?

And uh, it looks to me like this is a route to a security hole if it's not escaping the singlequote.

[ipv6guru]

Just confirming that this is reproducible every time. Would be nice to see a fix for the next 0.7.11 release.

[dustin]

It's not a security problem - shlex is just string.split() on steriods.

This error should be handled more smoothly, but that just means presenting a more useful error message via IRC. The fix is to not use unevenly-quoted strings in the IRC bot:

CrawlBuild force build crawl-debug-x86_64-centos "attempt to bomb with rvollmert's latest changes"

[ddunbar]

Why do we use shlex anyway? Are we are only using shlex so we can parse "foo bar" as a single token?

It seems an unintuitive interface for an IRC bot. Should we just reimplement the command tokenization to be robust?

[dustin]

That's basically all shlex does, is parse quotes into an arglist. The problem is that we don't use it everywhere in IRC - only for some commands.

[ddunbar]

The problem is shlex rules allow it to fail, which isn't ideal in an IRC bot. I expect the syntax to be

force build foo any text I want

not

force build foo "any text I want"

When do we need to allow things to be quoted? The only options I can think off are builder names (and the like) that had spaces.

Looking at the code, I'd be tempted to just completely redo how the IRC bot does command parsing.

[dustin]

Sounds good to me. I disagree about not needing quotes, tho - I do *not* like terminating things at EOL. It's so DOS.

[dustin]

This is not release-critical, since it is expected (if odd) behavior.

[dustin]

MichaelMayorov? fixed this up in  https://github.com/buildbot/buildbot/compare/ea6f4707306f...dd04ff1ab5e3