<div class="gmail_quote">On Fri, Aug 6, 2010 at 21:59, Ezio Melotti <span dir="ltr">&lt;<a href="mailto:ezio.melotti@gmail.com">ezio.melotti@gmail.com</a>&gt;</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&lt;frame-objects&gt;` 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 (&#39;win&#39;, &#39;os2&#39;) or sys.platform == &#39;riscos&#39;:<br>
+if sys.platform == &#39;os2&#39; or sys.platform == &#39;riscos&#39;:<br>
      raise unittest.SkipTest(&quot;Can&#39;t test signal on %s&quot; % \<br>
                                     sys.platform)<br>
<br>
@@ -37,6 +37,7 @@<br>
          return None<br>
<br>
<br>
+@unittest.skipIf(sys.platform == &quot;win32&quot;, &quot;Not valid on Windows&quot;)<br>
</blockquote>
<br>
In the previous chunk sys.platform[:3] was used instead of just sys.platform. Are there some other &quot;winXX&quot; platform that should be skipped too?<br></blockquote><div><br></div><div>The sliced check was to make it more convenient to also check &quot;os2&quot; at the same time in the first hunk of the change. Windows is &quot;win32&quot; 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 == &quot;win32&quot;, &quot;Not valid on Windows&quot;)<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 == &quot;win32&quot;, &quot;Windows specific&quot;)<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&#39;t raise an error but the second does the test will pass even if it shouldn&#39;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 == &quot;win32&quot;, &quot;Not valid on Windows&quot;)<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 == &quot;win32&quot;, &quot;Not valid on Windows&quot;)<br>
  class SiginterruptTest(unittest.TestCase):<br>
-    signum = signal.SIGUSR1<br>
<br>
      def setUp(self):<br>
          &quot;&quot;&quot;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>
          &quot;&quot;&quot;<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 == &quot;win32&quot;, &quot;Not valid on Windows&quot;)<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 == &quot;win32&quot;:<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__ == &quot;__main__&quot;:<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, &quot;iO:signal&quot;,&amp;sig_num,&amp;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&lt;  num_valid_sigs; cur_sig++)<br>
+        valid_sig |= (sig_num == valid_sigs[cur_sig]);<br>
+    if (!valid_sig) {<br>
+        PyErr_SetString(PyExc_ValueError, &quot;signal number out of range&quot;);<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>