From: Jean-Tiare Le Bigot <jt@yadutaf.fr>
Date: Thu, 2 Mar 2023 22:20:13 +0000 (+0100)
Subject: vici: Generalize timeout support in Python bindings
X-Git-Tag: 5.9.11dr1~18
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bb6174a4d1114648a5c04d05fc5a66262bfe3248;p=thirdparty%2Fstrongswan.git

vici: Generalize timeout support in Python bindings

Since 3dd5dc50119d ("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 <tobias@strongswan.org>
Signed-off-by: Jean-Tiare Le Bigot <jt@yadutaf.fr>

Closes strongswan/strongswan#1562
---

diff --git a/src/libcharon/plugins/vici/python/vici/protocol.py b/src/libcharon/plugins/vici/python/vici/protocol.py
index 8793651882..45aec5312a 100644
--- a/src/libcharon/plugins/vici/python/vici/protocol.py
+++ b/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
 
 
diff --git a/src/libcharon/plugins/vici/python/vici/session.py b/src/libcharon/plugins/vici/python/vici/session.py
index 79f2bda815..f45ff952e5 100644
--- a/src/libcharon/plugins/vici/python/vici/session.py
+++ b/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