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.