papi: harden socket handling

In the previous implementation of socket transport for PAPI,
socket methods .send and .recv_into were used.
But they are not guaranteed to send/receive all the data
for the full message. The receive part contained a loop,
but it handled only the main message, not the header.

This patch replaces .send with .sendall
and uses newly defined _read_fixed method.

Also, removed Paul from maintainers,
as he is not active much, lately.

Type: fix

Change-Id: Iae1a68bf8f9e666856b7c7d62ebfe22defc5dfe1
Signed-off-by: Vratko Polak <vrpolak@cisco.com>

diff --git a/MAINTAINERS b/MAINTAINERS
index 4f5e24a..7e1b65b 100644 (file)
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -522,7 +522,6 @@ F:  src/plugins/hs_apps/
 Python binding for the VPP API
 I:     papi
 M:     Ole Troan <ot@cisco.com>
-M:     Paul Vinciguerra <pvinci@vinciconsulting.com>
 F:     src/vpp-api/python
 
 Plugin - SCTP

diff --git a/src/vpp-api/python/vpp_papi/vpp_transport_socket.py b/src/vpp-api/python/vpp_papi/vpp_transport_socket.py
index 3c3c8e5..3c1ea32 100644 (file)
--- a/src/vpp-api/python/vpp_papi/vpp_transport_socket.py
+++ b/src/vpp-api/python/vpp_papi/vpp_transport_socket.py
@@ -188,31 +188,37 @@ class VppTransport(object):
 
         # Send header
         header = self.header.pack(0, len(buf), 0)
-        n = self.socket.send(header)
-        n = self.socket.send(buf)
-        if n == 0:
-            raise VppTransportSocketIOError(1, 'Not connected')
+        if self.socket.sendall(header) is None:
+            raise VppTransportSocketIOError(1, 'Failed to send')
+        if self.socket.sendall(buf) is None:
+            raise VppTransportSocketIOError(1, 'Failed to send')
+
+    def _read_fixed(self, size):
+        """Repeat receive until fixed size is read. Return empty on error."""
+        buf = bytearray(size)
+        view = memoryview(buf)
+        left = size
+        while 1:
+            got = self.socket.recv_into(view, left)
+            if got <= 0:
+                # Read error.
+                return ""
+            if got >= left:
+                # TODO: Raise if got > left?
+                break
+            left -= got
+            view = view[got:]
+        return buf
 
     def _read(self):
-        hdr = self.socket.recv(16)
+        """Read single complete message, return it or empty on error."""
+        hdr = self._read_fixed(16)
         if not hdr:
             return
         (_, hdrlen, _) = self.header.unpack(hdr)  # If at head of message
 
         # Read rest of message
-        msg = self.socket.recv(hdrlen)
-        if hdrlen > len(msg):
-            nbytes = len(msg)
-            buf = bytearray(hdrlen)
-            view = memoryview(buf)
-            view[:nbytes] = msg
-            view = view[nbytes:]
-            left = hdrlen - nbytes
-            while left:
-                nbytes = self.socket.recv_into(view, left)
-                view = view[nbytes:]
-                left -= nbytes
-            return buf
+        msg = self._read_fixed(hdrlen)
         if hdrlen == len(msg):
             return msg
         raise VppTransportSocketIOError(1, 'Unknown socket read error')