Merge pull request #89 from oremanj/fix-str-segfault

Check whether payload is NULL before accessing it in __str__

Author: oremanj

netfilterqueue/_impl.pyx

@@ -57,7 +57,15 @@ cdef class Packet:
         self._given_payload = None
 
     def __str__(self):
-        cdef iphdr *hdr = <iphdr*>self.payload
+        cdef unsigned char *payload = NULL
+        if self._owned_payload:
+            payload = self._owned_payload
+        elif self.payload != NULL:
+            payload = self.payload
+        else:
+            return "%d byte packet, contents unretained" % (self.payload_len,)
+
+        cdef iphdr *hdr = <iphdr*>payload
         protocol = PROTOCOLS.get(hdr.protocol, "Unknown protocol")
         return "%s packet, %s bytes" % (protocol, self.payload_len)
 

setup.py

@@ -3,7 +3,7 @@
 
 exec(open("netfilterqueue/_version.py", encoding="utf-8").read())
 
-setup_requires = []
+setup_requires = ["wheel"]
 try:
     # Use Cython
     from Cython.Build import cythonize
@@ -21,7 +21,7 @@
     if "egg_info" in sys.argv:
         # We're being run by pip to figure out what we need. Request cython in
         # setup_requires below.
-        setup_requires = ["cython"]
+        setup_requires += ["cython"]
     elif not os.path.exists(
         os.path.join(os.path.dirname(__file__), "netfilterqueue/_impl.c")
     ):

tests/test_basic.py

@@ -23,6 +23,7 @@ async def test_comms_without_queue(harness):
 async def test_queue_dropping(harness):
     async def drop(packets, msg):
         async for packet in packets:
+            assert "UDP packet" in str(packet)
             if packet.get_payload()[28:] == msg:
                 packet.drop()
             else:
@@ -190,6 +191,7 @@ async def test_errors(harness):
 async def test_unretained(harness):
     def cb(chan, pkt):
         # Can access payload within callback
+        assert "UDP packet" in str(pkt)
         assert pkt.get_payload()[-3:] in (b"one", b"two")
         chan.send_nowait(pkt)
 
@@ -202,6 +204,7 @@ def cb(chan, pkt):
                 RuntimeError, match="Payload data is no longer available"
             ):
                 p.get_payload()
+            assert "contents unretained" in str(p)
             # Can still issue verdicts though
             if accept:
                 p.accept()