Webservice: Use ast.literal_eval() for parsing ZMQ requests

Description

We were using eval() to parse incoming requests. This runs arbitrary code, so anyone who discovers the port number could run code as xcal/xcaltst. Even if we're not concerned about that, it makes the system harder to understand, because theoretically a valid request could include a function call that runs when it's eval-ed.

Looking at the scripts in this repo and the code in myMDC, I believe that requests are always Python literals (specifically, lists/tuples of strings). So we can use the safe ast.literal_eval() function to parse them.

Using JSON would be a nicer approach (mentioned in detectors/calibration_workshop#261), but that would involve changes to the code making requests as well. Using literal_eval is an easy change because it doesn't require any coordination with other code.

How Has This Been Tested?

Deployed on max-exfl017, tested submitting one run for correction and one dark calibration job (involving 3 runs) from mymdc test environment.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• My code follows the code style of this project.

Reviewers

@danilevc @ahmedk

webservice/webservice.py

 import argparse
+import ast
 import asyncio
 import copy
 import getpass
@@ -669,15 +670,13 @@ class ActionsServer:
 
         The main server loop handles remote requests via a ZMQ interface.
 
-        Requests are the form of ZMQ.REQuest and have the format
-
-            command, *params
-
-        where *parms is a string-encoded python list as defined by the
-        commands.
+        Requests are serialised as the repr of a Python list/tuple, with the
+        first element identifying the action, and subsequent elements arguments
+        to be passed to that action. The number & meaning of arguments depends
+        on the action.
         """
         while True:
-            req = await self.socket.recv_multipart()
+            req = await self.socket.recv()
             logging.debug("Raw request data: %r", req)
             try:
                 resp = await self.handle_one_req(req)
@@ -688,13 +687,13 @@ class ActionsServer:
             logging.debug("Sending response: %r", resp)
             await self.socket.send(resp)
 
-    async def handle_one_req(self, req: List[bytes]) -> bytes:
-        if len(req) == 1:
-            try:  # protect against unparseable requests
-                req = eval(req[0])
-            except SyntaxError as e:
-                logging.error(str(e))
-                return Errors.REQUEST_FAILED.encode()
+    async def handle_one_req(self, raw_req: bytes) -> bytes:
+        """Handle one request, and return the reply to be sent"""
+        try:  # protect against unparseable requests
+            req = ast.literal_eval(raw_req.decode('utf-8'))
+        except SyntaxError as e:
+            logging.error(str(e))
+            return Errors.REQUEST_FAILED.encode()
 
         if len(req) < 2:  # catch parseable but malformed requests
             logging.error(Errors.REQUEST_MALFORMED.format(req))
@@ -860,7 +859,7 @@ class ActionsServer:
             karabo_das = karabo_das.split(',')
 
             for i, run in enumerate(runs):
-                erun = eval(run)
+                erun = ast.literal_eval(run)
                 if isinstance(erun, (list, tuple)):
                     typ, runnr = erun
                     if typ == "reservation":
@@ -983,7 +982,7 @@ class ActionsServer:
         """
         request_time = datetime.now().strftime('%Y-%m-%dT%H:%M:%S')
         try:
-            pdus, karabo_das, wait_runs = eval(','.join(extra))
+            pdus, karabo_das, wait_runs = ast.literal_eval(','.join(extra))
 
             karabo_das = [val.strip() for val in karabo_das]
             runs = [str(val) for val in wait_runs]

[Thomas Kluyver]

None of the code I could see sent multipart messages, so I simplified it to receive single-part messages.