<div class="gmail_quote">On Fri, Aug 6, 2010 at 21:59, Ezio Melotti <span dir="ltr"><<a href="mailto:ezio.melotti@gmail.com">ezio.melotti@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi,<br>
<br>
On 06/08/2010 22.27, brian.curtin wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: brian.curtin<br>
Date: Fri Aug 6 21:27:32 2010<br>
New Revision: 83763<br>
<br>
Log:<br>
Fix #9324: Add parameter validation to signal.signal on Windows in order<br>
to prevent crashes.<br>
<br>
<br>
Modified:<br>
python/branches/py3k/Doc/library/signal.rst<br>
python/branches/py3k/Lib/test/test_signal.py<br>
python/branches/py3k/Misc/NEWS<br>
python/branches/py3k/Modules/signalmodule.c<br>
<br>
Modified: python/branches/py3k/Doc/library/signal.rst<br>
==============================================================================<br>
--- python/branches/py3k/Doc/library/signal.rst (original)<br>
+++ python/branches/py3k/Doc/library/signal.rst Fri Aug 6 21:27:32 2010<br>
@@ -230,6 +230,10 @@<br>
see the :ref:`description in the type hierarchy<frame-objects>` or see the<br>
attribute descriptions in the :mod:`inspect` module).<br>
<br>
+ On Windows, :func:`signal` can only be called with :const:`SIGABRT`,<br>
+ :const:`SIGFPE`, :const:`SIGILL`, :const:`SIGINT`, :const:`SIGSEGV`, or<br>
+ :const:`SIGTERM`. A :exc:`ValueError` will be raised in any other case.<br>
+<br>
<br>
.. _signal-example:<br>
<br>
<br>
Modified: python/branches/py3k/Lib/test/test_signal.py<br>
==============================================================================<br>
--- python/branches/py3k/Lib/test/test_signal.py (original)<br>
+++ python/branches/py3k/Lib/test/test_signal.py Fri Aug 6 21:27:32 2010<br>
@@ -9,7 +9,7 @@<br>
import traceback<br>
import sys, os, time, errno<br>
<br>
-if sys.platform[:3] in ('win', 'os2') or sys.platform == 'riscos':<br>
+if sys.platform == 'os2' or sys.platform == 'riscos':<br>
raise unittest.SkipTest("Can't test signal on %s" % \<br>
sys.platform)<br>
<br>
@@ -37,6 +37,7 @@<br>
return None<br>
<br>
<br>
+@unittest.skipIf(sys.platform == "win32", "Not valid on Windows")<br>
</blockquote>
<br>
In the previous chunk sys.platform[:3] was used instead of just sys.platform. Are there some other "winXX" platform that should be skipped too?<br></blockquote><div><br></div><div>The sliced check was to make it more convenient to also check "os2" at the same time in the first hunk of the change. Windows is "win32" regardless of 32 or 64-bit so that check works. </div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
class InterProcessSignalTests(unittest.TestCase):<br>
MAX_DURATION = 20 # Entire test should last at most 20 sec.<br>
<br>
@@ -186,6 +187,7 @@<br>
self.MAX_DURATION)<br>
<br>
<br>
+@unittest.skipIf(sys.platform == "win32", "Not valid on Windows")<br>
class BasicSignalTests(unittest.TestCase):<br>
def trivial_signal_handler(self, *args):<br>
pass<br>
@@ -208,6 +210,23 @@<br>
self.assertEquals(signal.getsignal(signal.SIGHUP), hup)<br>
<br>
<br>
+@unittest.skipUnless(sys.platform == "win32", "Windows specific")<br>
+class WindowsSignalTests(unittest.TestCase):<br>
+ def test_issue9324(self):<br>
+ handler = lambda x, y: None<br>
+ signal.signal(signal.SIGABRT, handler)<br>
+ signal.signal(signal.SIGFPE, handler)<br>
+ signal.signal(signal.SIGILL, handler)<br>
+ signal.signal(signal.SIGINT, handler)<br>
+ signal.signal(signal.SIGSEGV, handler)<br>
+ signal.signal(signal.SIGTERM, handler)<br>
+<br>
+ with self.assertRaises(ValueError):<br>
+ signal.signal(-1, handler)<br>
+ sinal.signal(7, handler)<br>
</blockquote>
<br>
You should use two separate assertRaises here, otherwise the second line is not executed if the first one raises an error (and if the first doesn't raise an error but the second does the test will pass even if it shouldn't). This also masks the typo in the second line.<br>
</blockquote><div><br></div><div>Thanks for noticing this. Corrected in r83771 (py3k), r83772 (release31-maint), and r83773 (release27-maint).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+<br>
+@unittest.skipIf(sys.platform == "win32", "Not valid on Windows")<br>
class WakeupSignalTests(unittest.TestCase):<br>
TIMEOUT_FULL = 10<br>
TIMEOUT_HALF = 5<br>
@@ -253,14 +272,15 @@<br>
os.close(self.write)<br>
signal.signal(signal.SIGALRM, self.alrm)<br>
<br>
+@unittest.skipIf(sys.platform == "win32", "Not valid on Windows")<br>
class SiginterruptTest(unittest.TestCase):<br>
- signum = signal.SIGUSR1<br>
<br>
def setUp(self):<br>
"""Install a no-op signal handler that can be set to allow<br>
interrupts or not, and arrange for the original signal handler to be<br>
re-installed when the test is finished.<br>
"""<br>
+ self.signum = signal.SIGUSR1<br>
oldhandler = signal.signal(self.signum, lambda x,y: None)<br>
self.addCleanup(signal.signal, self.signum, oldhandler)<br>
<br>
@@ -354,7 +374,7 @@<br>
self.assertFalse(i)<br>
<br>
<br>
-<br>
+@unittest.skipIf(sys.platform == "win32", "Not valid on Windows")<br>
class ItimerTest(unittest.TestCase):<br>
def setUp(self):<br>
self.hndl_called = False<br>
@@ -463,8 +483,11 @@<br>
self.assertEqual(self.hndl_called, True)<br>
<br>
def test_main():<br>
- support.run_unittest(BasicSignalTests, InterProcessSignalTests,<br>
- WakeupSignalTests, SiginterruptTest, ItimerTest)<br>
+ if sys.platform == "win32":<br>
+ support.run_unittest(WindowsSignalTests)<br>
+ else:<br>
+ support.run_unittest(BasicSignalTests, InterProcessSignalTests,<br>
+ WakeupSignalTests, SiginterruptTest, ItimerTest)<br>
</blockquote>
<br>
Is this necessary?<br>
If the tests are marked with a skip decorator they will be skipped anyway (and also marked as skipped in the output).<br></blockquote><div><br></div><div>Good point. Fixed in the above revisions.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
if __name__ == "__main__":<br>
<br>
Modified: python/branches/py3k/Misc/NEWS<br>
==============================================================================<br>
--- python/branches/py3k/Misc/NEWS (original)<br>
+++ python/branches/py3k/Misc/NEWS Fri Aug 6 21:27:32 2010<br>
@@ -24,6 +24,9 @@<br>
Extensions<br>
----------<br>
<br>
+- Issue #9324: Add parameter validation to signal.signal on Windows in order<br>
+ to prevent crashes.<br>
+<br>
- Issue #9526: Remove some outdated (int) casts that were preventing<br>
the array module from working correctly with arrays of more than<br>
2**31 elements.<br>
<br>
Modified: python/branches/py3k/Modules/signalmodule.c<br>
==============================================================================<br>
--- python/branches/py3k/Modules/signalmodule.c (original)<br>
+++ python/branches/py3k/Modules/signalmodule.c Fri Aug 6 21:27:32 2010<br>
@@ -255,8 +255,23 @@<br>
int sig_num;<br>
PyObject *old_handler;<br>
void (*func)(int);<br>
+#ifdef MS_WINDOWS<br>
+ int cur_sig, num_valid_sigs = 6;<br>
+ static int valid_sigs[] = {SIGABRT, SIGFPE, SIGILL, SIGINT,<br>
+ SIGSEGV, SIGTERM};<br>
+ BOOL valid_sig = FALSE;<br>
+#endif<br>
if (!PyArg_ParseTuple(args, "iO:signal",&sig_num,&obj))<br>
return NULL;<br>
+#ifdef MS_WINDOWS<br>
+ /* Validate that sig_num is one of the allowable signals */<br>
+ for (cur_sig = 0; cur_sig< num_valid_sigs; cur_sig++)<br>
+ valid_sig |= (sig_num == valid_sigs[cur_sig]);<br>
+ if (!valid_sig) {<br>
+ PyErr_SetString(PyExc_ValueError, "signal number out of range");<br>
+ return NULL;<br>
+ }<br>
+#endif<br>
#ifdef WITH_THREAD<br>
if (PyThread_get_thread_ident() != main_thread) {<br>
PyErr_SetString(PyExc_ValueError,<br>
</blockquote>
<br>
Best Regards,<br><font color="#888888">
Ezio Melotti</font></blockquote></div>