From 6456a35817257cbabeb56e6ba4f6576da04155c8 Mon Sep 17 00:00:00 2001 From: venth Date: Sun, 4 May 2014 20:31:26 +0200 Subject: [PATCH 1/7] ignore Intellij IDEA related files --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 189ab2749..2bc0fda2b 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,6 @@ test-logs *.pyc docs/_build docs/plugins +# Intellij PyCharm / IDEA related files +*.iml +.idea From 76599db94443fd2bc7f1bc2b613399424c6d5fe6 Mon Sep 17 00:00:00 2001 From: venth Date: Mon, 5 May 2014 19:15:34 +0200 Subject: [PATCH 2/7] questions.yn: Perform string, not identity, comparison against 'y' The `is` operator performs object identity comparison. Changing to `==` implements the expected behavior. Use the mock library to add tests verifying the API of questions.yn. --- setup.py | 4 ++ src/questions.py | 2 +- test/test_yn.py | 99 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 test/test_yn.py diff --git a/setup.py b/setup.py index 7c6addd0e..646dcfb18 100644 --- a/setup.py +++ b/setup.py @@ -123,6 +123,10 @@ setup( 'python-dateutil <2.0,>=1.3', 'feedparser', ], + + tests_require=[ + 'mock', + ] ) diff --git a/src/questions.py b/src/questions.py index 5cee897d6..ab13fe5d9 100644 --- a/src/questions.py +++ b/src/questions.py @@ -112,7 +112,7 @@ def yn(prompt, default=None): else: default = 'n' s = expect(prompt, ['y', 'n'], default=default) - if s is 'y': + if s == 'y': return True else: return False diff --git a/test/test_yn.py b/test/test_yn.py new file mode 100644 index 000000000..92340c340 --- /dev/null +++ b/test/test_yn.py @@ -0,0 +1,99 @@ +### +# Copyright (c) 2014, Artur Krysiak +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, +# this list of conditions, and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions, and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# * Neither the name of the author of this software nor the name of +# contributors to this software may be used to endorse or promote products +# derived from this software without specific prior written consent. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +### + +from supybot import questions +from supybot.test import SupyTestCase + +import mock + +# so complicated construction because I want to +# gain the string 'y' instead of the character 'y' +# the reason of usage this construction is to prove +# that comparing strings by 'is' is wrong +# better solution is usage of '==' operator ;) +_yes_answer = ''.join(['', 'y']) + +class TestYn(SupyTestCase): + def test_default_yes_selected(self): + questions.expect = mock.Mock(return_value=_yes_answer) + + answer = questions.yn('up', default='y') + + self.assertTrue(answer) + + def test_default_no_selected(self): + questions.expect = mock.Mock(return_value='n') + + answer = questions.yn('up', default='n') + + self.assertFalse(answer) + + def test_yes_selected_without_defaults(self): + questions.expect = mock.Mock(return_value=_yes_answer) + + answer = questions.yn('up') + + self.assertTrue(answer) + + def test_no_selected_without_defaults(self): + questions.expect = mock.Mock(return_value='n') + + answer = questions.yn('up') + + self.assertFalse(answer) + + def test_no_selected_with_default_yes(self): + questions.expect = mock.Mock(return_value='n') + + answer = questions.yn('up', default='y') + + self.assertFalse(answer) + + def test_yes_selected_with_default_yes(self): + questions.expect = mock.Mock(return_value=_yes_answer) + + answer = questions.yn('up', default='y') + + self.assertTrue(answer) + + def test_yes_selected_with_default_no(self): + questions.expect = mock.Mock(return_value=_yes_answer) + + answer = questions.yn('up', default='n') + + self.assertTrue(answer) + + def test_no_selected_with_default_no(self): + questions.expect = mock.Mock(return_value='n') + + answer = questions.yn('up', default='n') + + self.assertFalse(answer) + +# vim:set shiftwidth=4 softtabstop=4 expandtab textwidth=79: From 7838cae3bc5f23c271c8baba0630d6969d8f1b55 Mon Sep 17 00:00:00 2001 From: James McCoy Date: Mon, 9 Jun 2014 23:44:25 -0400 Subject: [PATCH 3/7] callbacks: Use Raise=True for nested limit errors Signed-off-by: James McCoy --- src/callbacks.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/callbacks.py b/src/callbacks.py index 01b398408..dcae2e203 100644 --- a/src/callbacks.py +++ b/src/callbacks.py @@ -1,6 +1,6 @@ ### # Copyright (c) 2002-2005, Jeremiah Fincher -# Copyright (c) 2008-2010, James McCoy +# Copyright (c) 2014, James McCoy # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -588,7 +588,7 @@ class NestedCommandsIrcProxy(ReplyIrcProxy): log.warning('%s attempted more than %s levels of nesting.', self.msg.prefix, maxNesting) return self.error('You\'ve attempted more nesting than is ' - 'currently allowed on this bot.') + 'currently allowed on this bot.', Raise=True) # The deepcopy here is necessary for Scheduler; it re-runs already # tokenized commands. There's a possibility a simple copy[:] would # work, but we're being careful. From f5df6695c04305cfa04a393ad863e334bf6c57ae Mon Sep 17 00:00:00 2001 From: James McCoy Date: Mon, 9 Jun 2014 23:48:41 -0400 Subject: [PATCH 4/7] Alias: Restore recursion limit and limit memory use The (faulty) detection of recursive Aliases was removed in a656fd06930541103c70297825d6f5f5956ecc45, claiming that "our nesting limit will catch issues now." However, nested Aliases weren't actually increasing the nesting level. Actually increasing the nesting level when an alias is executed restores the intended behavior. Additionally, limiting the size of the expanded arguments to an alias prevents exponential growth of memory usage for certain malicious inputs/aliases. Signed-off-by: James McCoy --- plugins/Alias/plugin.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/plugins/Alias/plugin.py b/plugins/Alias/plugin.py index f6dfc229a..d84abcff9 100644 --- a/plugins/Alias/plugin.py +++ b/plugins/Alias/plugin.py @@ -1,6 +1,6 @@ ### # Copyright (c) 2002-2004, Jeremiah Fincher -# Copyright (c) 2009-2010, James McCoy +# Copyright (c) 2014, James McCoy # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -78,9 +78,6 @@ def getArgs(args, required=1, optional=0, wildcard=0): class AliasError(Exception): pass -class RecursiveAlias(AliasError): - pass - dollarRe = re.compile(r'\$(\d+)') def findBiggestDollar(alias): dollars = dollarRe.findall(alias) @@ -156,7 +153,11 @@ def makeNewAlias(name, alias): return True return False everythingReplace(tokens) - self.Proxy(irc, msg, tokens) + # Limit memory use by constraining the size of the message being passed + # in to the alias. Also tracking nesting to avoid endless recursion. + maxLength = conf.supybot.reply.maximumLength() + tokens = [t[:maxLength] for t in tokens] + self.Proxy(irc, msg, tokens, nested=irc.nested + 1) flexargs = '' if biggestDollar and (wildcard or biggestAt): flexargs = ' at least' @@ -258,11 +259,8 @@ class Alias(callbacks.Plugin): (currentAlias, locked, _) = self.aliases[name] if locked and currentAlias != alias: raise AliasError, format('Alias %q is locked.', name) - try: - f = makeNewAlias(name, alias) - f = new.instancemethod(f, self, Alias) - except RecursiveAlias: - raise AliasError, 'You can\'t define a recursive alias.' + f = makeNewAlias(name, alias) + f = new.instancemethod(f, self, Alias) aliasGroup = self.registryValue('aliases', value=False) if name in self.aliases: # We gotta remove it so its value gets updated. From 27ffff7ad6488596ecf0a06d890616a8b675a150 Mon Sep 17 00:00:00 2001 From: James McCoy Date: Sun, 22 Jun 2014 22:36:16 -0400 Subject: [PATCH 5/7] release: Remove reference to freshmeat^Wfreecode freecode.com, nee freshmeat.net, is now a completely static site. Since it is no longer allowing updates, no need to worry about pushing information about Supybot releases to it. Signed-off-by: James McCoy --- sandbox/release.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sandbox/release.py b/sandbox/release.py index bc90db6e5..e6d1316c0 100644 --- a/sandbox/release.py +++ b/sandbox/release.py @@ -186,6 +186,3 @@ if __name__ == '__main__': # 'test-data', 'test-logs', 'tmp') # for fn in configFiles: # os.remove(fn) - -# This is the part where we do our release on Freshmeat using XMLRPC and -# ESR's software to do it: http://freshmeat.net/p/freshmeat-submit/ From 3d993a0cabe41a63223e2a6c5f3e8639de643d39 Mon Sep 17 00:00:00 2001 From: James McCoy Date: Sun, 29 Jun 2014 19:12:22 -0400 Subject: [PATCH 6/7] callbacks: Properly handle nested command errors Using Raise=True was only papering over the real problem in the nested command error handling. The actual issue is that we were trying to return an IrcMsg from NestedCommandsIrcProxy.__init__. Dropping Raise=True and moving return to its own line is the correct fix and resolves the test failure in testMaximumNestingDepth. This commit reverts 7838cae3bc5f23c271c8baba0630d6969d8f1b55 Signed-off-by: James McCoy --- src/callbacks.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/callbacks.py b/src/callbacks.py index dcae2e203..b52d3e726 100644 --- a/src/callbacks.py +++ b/src/callbacks.py @@ -587,8 +587,9 @@ class NestedCommandsIrcProxy(ReplyIrcProxy): if maxNesting and self.nested > maxNesting: log.warning('%s attempted more than %s levels of nesting.', self.msg.prefix, maxNesting) - return self.error('You\'ve attempted more nesting than is ' - 'currently allowed on this bot.', Raise=True) + self.error('You\'ve attempted more nesting than is ' + 'currently allowed on this bot.') + return # The deepcopy here is necessary for Scheduler; it re-runs already # tokenized commands. There's a possibility a simple copy[:] would # work, but we're being careful. From a629f51328be80e76f255c329624eda8f3503d59 Mon Sep 17 00:00:00 2001 From: James McCoy Date: Wed, 9 Jul 2014 21:03:26 -0400 Subject: [PATCH 7/7] Anonymous: Move "say $nick" functionality to new tell command Allowing Anonymous.say to send a message to either a nick or an (implicit) channel through the use of first('nick', 'inChannel') changed the behavior of the command by making it impossible for 'inChannel' to take effect. This meant that any previous users of the command that expected "say some text" to send "some text" to the current channel would instead try to send "text" to the user "some". Depending on the value of conf.plugins.Anonymous.allowPrivateTarget, this would result in either an error or a strange message to a random user. Creating a new tell command solves this issue as Anonymous.channel now goes back to its simple 'inChannel' wrapper. Signed-off-by: James McCoy --- plugins/Anonymous/plugin.py | 23 ++++++++++++++++++----- plugins/Anonymous/test.py | 16 ++++++++++++---- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/plugins/Anonymous/plugin.py b/plugins/Anonymous/plugin.py index 7715cfafc..748014b72 100644 --- a/plugins/Anonymous/plugin.py +++ b/plugins/Anonymous/plugin.py @@ -1,6 +1,6 @@ ### # Copyright (c) 2005, Daniel DiPaolo -# Copyright (c) 2010, James McCoy +# Copyright (c) 2014, James McCoy # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -74,17 +74,30 @@ class Anonymous(callbacks.Plugin): Raise=True) def say(self, irc, msg, args, target, text): - """ + """ - Sends to . Can only send to if + Sends to . Can only send to if supybot.plugins.Anonymous.allowPrivateTarget is True. """ self._preCheck(irc, msg, target, 'say') - self.log.info('Saying %q to %s due to %s.', + self.log.info('Saying %q in %s due to %s.', text, target, msg.prefix) irc.queueMsg(ircmsgs.privmsg(target, text)) irc.noReply() - say = wrap(say, [first('nick', 'inChannel'), 'text']) + say = wrap(say, ['inChannel', 'text']) + + def tell(self, irc, msg, args, target, text): + """ + + Sends to . Can only be used if + supybot.plugins.Anonymous.allowPrivateTarget is True. + """ + self._preCheck(irc, msg, target, 'tell') + self.log.info('Telling %q to %s due to %s.', + text, target, msg.prefix) + irc.queueMsg(ircmsgs.privmsg(target, text)) + irc.noReply() + tell = wrap(tell, ['nick', 'text']) def do(self, irc, msg, args, channel, text): """ diff --git a/plugins/Anonymous/test.py b/plugins/Anonymous/test.py index 96a570853..4f9713496 100644 --- a/plugins/Anonymous/test.py +++ b/plugins/Anonymous/test.py @@ -1,6 +1,6 @@ ### # Copyright (c) 2005, Daniel DiPaolo -# Copyright (c) 2010, James McCoy +# Copyright (c) 2014, James McCoy # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -34,15 +34,23 @@ class AnonymousTestCase(ChannelPluginTestCase): plugins = ('Anonymous',) def testSay(self): self.assertError('anonymous say %s I love you!' % self.channel) - self.assertError('anonymous say %s I love you!' % self.nick) origreg = conf.supybot.plugins.Anonymous.requireRegistration() - origpriv = conf.supybot.plugins.Anonymous.allowPrivateTarget() try: conf.supybot.plugins.Anonymous.requireRegistration.setValue(False) m = self.assertNotError('anonymous say %s foo!' % self.channel) self.failUnless(m.args[1] == 'foo!') + finally: + conf.supybot.plugins.Anonymous.requireRegistration.setValue(origreg) + + def testTell(self): + self.assertError('anonymous tell %s I love you!' % self.nick) + origreg = conf.supybot.plugins.Anonymous.requireRegistration() + origpriv = conf.supybot.plugins.Anonymous.allowPrivateTarget() + try: + conf.supybot.plugins.Anonymous.requireRegistration.setValue(False) + self.assertError('anonymous tell %s foo!' % self.channel) conf.supybot.plugins.Anonymous.allowPrivateTarget.setValue(True) - m = self.assertNotError('anonymous say %s foo!' % self.nick) + m = self.assertNotError('anonymous tell %s foo!' % self.nick) self.failUnless(m.args[1] == 'foo!') finally: conf.supybot.plugins.Anonymous.requireRegistration.setValue(origreg)