vici: Generalize timeout support in Python bindings

Since 3dd5dc5011 ("Merge branch 'vici-python-timeout'"), any timeout
set directly on the socket is reset by `vici.Transport.receive()` unless
called by `vici.Session.listen()`. This prevents configuring a default
timeout directly on the socket.

However, setting a timeout directly on the socket also had drawbacks
since it can cause `vici.Transport.receive()` to raise a timeout error
while a subset of the data have been received, with no way to recover.

This commit merges both approaches by considering the timeout configured
on the socket by default (when no timeout is explicitly set) and
keeping the switch to blocking receive once the first byte has been
received. When the full expected data have been received, the timeout
configured on the socket is restored.

Co-authored-by: Tobias Brunner <[email protected]>
Signed-off-by: Jean-Tiare Le Bigot <[email protected]>

Closes strongswan/strongswan#1562

src/libcharon/plugins/vici/python/vici/protocol.py

@@ -8,6 +8,9 @@ from collections import OrderedDict
 from .exception import DeserializationException
 
 
+RECV_TIMEOUT_DEFAULT = object()
+
+
 class Transport(object):
     HEADER_LENGTH = 4
     MAX_SEGMENT = 512 * 1024
@@ -18,7 +21,7 @@ class Transport(object):
     def send(self, packet):
         self.socket.sendall(struct.pack("!I", len(packet)) + packet)
 
-    def receive(self, timeout=None):
+    def receive(self, timeout=RECV_TIMEOUT_DEFAULT):
         raw_length = self._recvall(self.HEADER_LENGTH, timeout)
         length, = struct.unpack("!I", raw_length)
         payload = self._recvall(length)
@@ -28,17 +31,21 @@ class Transport(object):
         self.socket.shutdown(socket.SHUT_RDWR)
         self.socket.close()
 
-    def _recvall(self, count, timeout=None):
+    def _recvall(self, count, timeout=RECV_TIMEOUT_DEFAULT):
         """Ensure to read count bytes from the socket"""
         data = b""
-        if count > 0:
+        old_timeout = self.socket.gettimeout()
+        if timeout is not RECV_TIMEOUT_DEFAULT:
             self.socket.settimeout(timeout)
-        while len(data) < count:
-            buf = self.socket.recv(count - len(data))
-            self.socket.settimeout(None)
-            if not buf:
-                raise socket.error('Connection closed')
-            data += buf
+        try:
+            while len(data) < count:
+                buf = self.socket.recv(count - len(data))
+                self.socket.settimeout(None)
+                if not buf:
+                    raise socket.error('Connection closed')
+                data += buf
+        finally:
+            self.socket.settimeout(old_timeout)
         return data
 
 

src/libcharon/plugins/vici/python/vici/session.py

@@ -1,12 +1,30 @@
 import socket
 
 from .exception import SessionException, CommandException, EventUnknownException
-from .protocol import Transport, Packet, Message
+from .protocol import Transport, Packet, Message, RECV_TIMEOUT_DEFAULT
 from .command_wrappers import CommandWrappers
 
 
 class Session(CommandWrappers, object):
     def __init__(self, sock=None):
+        """Establish a session with an IKE daemon.
+
+        By default, the session will connect to the `/var/run/charon.vici` Unix
+        domain socket.
+
+        If there is a need to connect a socket in another location or set
+        specific settings on the socket (like a timeout), create and connect
+        a socket and pass it to the `sock` parameter.
+
+        .. note::
+
+            In case a timeout is set on the socket, the internal read code
+            will temporarily disable it after receiving the first byte to avoid
+            partial read corruptions.
+
+        :param sock: socket connected to the IKE daemon (optional)
+        :type sock: socket.socket
+        """
         if sock is None:
             sock = socket.socket(socket.AF_UNIX)
             sock.connect("/var/run/charon.vici")
@@ -141,7 +159,7 @@ class Session(CommandWrappers, object):
                     )
                 )
 
-    def listen(self, event_types, timeout=None):
+    def listen(self, event_types, timeout=RECV_TIMEOUT_DEFAULT):
         """Register and listen for the given events.
 
         If a timeout is given, the generator produces a (None, None) tuple