Merge pull request #201 from GREENRAT-K405/fix/fragile-parameter-parsing

Fix fragile parameter parsing in concore.py and concoredocker.py

Author: pradeeban

concore.py

@@ -156,26 +156,50 @@ def safe_literal_eval(filename, defaultValue):
 # ===================================================================
 # Parameter Parsing
 # ===================================================================
+def parse_params(sparams: str) -> dict:
+    params = {}
+    if not sparams:
+        return params
+
+    s = sparams.strip()
+
+    #full dict literal
+    if s.startswith("{") and s.endswith("}"):
+        try:
+            val = literal_eval(s)
+            if isinstance(val, dict):
+                return val
+        except (ValueError, SyntaxError):
+            pass
+
+    for item in s.split(";"):
+        if "=" in item:
+            key, value = item.split("=", 1)  # split only once
+            key=key.strip()
+            value=value.strip()
+            #try to convert to python type (int, float, list, etc.)
+            # Use literal_eval to preserve backward compatibility (integers/lists)
+            # Fallback to string for unquoted values (paths, URLs)
+            try:
+                params[key] = literal_eval(value)
+            except (ValueError, SyntaxError):
+                params[key] = value
+    return params
+
 try:
     sparams_path = concore_params_file
     if os.path.exists(sparams_path):
         with open(sparams_path, "r") as f:
-            sparams = f.read()
+            sparams = f.read().strip()
         if sparams: # Ensure sparams is not empty
             # Windows sometimes keeps quotes
             if sparams[0] == '"' and sparams[-1] == '"':  #windows keeps "" need to remove
                 sparams = sparams[1:-1]
 
-            # Convert key=value;key2=value2 to Python dict format
-            if sparams != '{' and not (sparams.startswith('{') and sparams.endswith('}')): # Check if it needs conversion
-                logging.debug("converting sparams: "+sparams)
-                sparams = "{'"+re.sub(';',",'",re.sub('=',"':",re.sub(' ','',sparams)))+"}"
-                logging.debug("converted sparams: " + sparams)
-            try:
-                params = literal_eval(sparams)
-            except Exception as e:
-                logging.warning(f"bad params content: {sparams}, error: {e}")
-                params = dict()
+            # Parse params using clean function instead of regex
+            logging.debug("parsing sparams: "+sparams)
+            params = parse_params(sparams)
+            logging.debug("parsed params: " + str(params))
         else:
             params = dict()
     else:

concoredocker.py

@@ -25,20 +25,53 @@ def safe_literal_eval(filename, defaultValue):
 concore_maxtime_file = os.path.join(inpath, "1", "concore.maxtime")
 
 #9/21/22
+def parse_params(sparams):
+    params = {}
+    if not sparams:
+        return params
+
+    s = sparams.strip()
+
+    # full dict literal
+    if s.startswith("{") and s.endswith("}"):
+        try:
+            val = literal_eval(s)
+            if isinstance(val, dict):
+                return val
+        except (ValueError, SyntaxError):
+            pass
+
+    # keep backward compatibility: comma-separated params
+    for item in s.split(","):
+        if "=" in item:
+            key, value = item.split("=", 1)
+            key = key.strip()
+            value = value.strip()
+            #try to convert to python type (int, float, list, etc.)
+            # Use literal_eval to preserve backward compatibility (integers/lists)
+            # Fallback to string for unquoted values (paths, URLs)
+            try:
+                params[key] = literal_eval(value)
+            except (ValueError, SyntaxError):
+                params[key] = value
+    return params
+
 try:
-    sparams = open(concore_params_file).read()
-    if sparams[0] == '"':  #windows keeps "" need to remove
+    with open(concore_params_file, "r") as f:
+        sparams = f.read().strip()
+
+    if sparams and sparams[0] == '"':  # windows keeps quotes
         sparams = sparams[1:]
-        sparams = sparams[0:sparams.find('"')]
-    if sparams != '{':
-        print("converting sparams: "+sparams)
-        sparams = "{'"+re.sub(',',",'",re.sub('=',"':",re.sub(' ','',sparams)))+"}"
-        print("converted sparams: " + sparams)
-    try:
-        params = literal_eval(sparams)
-    except:
-        print("bad params: "+sparams)
-except:
+        if '"' in sparams:
+            sparams = sparams[:sparams.find('"')]
+
+    if sparams:
+        print("parsing sparams:", sparams)
+        params = parse_params(sparams)
+    else:
+        params = dict()
+except Exception as e:
+    print(f"Error reading concore.params: {e}")
     params = dict()
 
 #9/30/22

tests/test_concore.py

@@ -84,4 +84,41 @@ def test_core_functions_exist(self):
 
         assert callable(safe_literal_eval)
         assert callable(tryparam)
-        assert callable(default_maxtime)
+        assert callable(default_maxtime)
+
+
+class TestParseParams:
+
+    def test_simple_key_value_pairs(self):
+        from concore import parse_params
+        params = parse_params("a=1;b=2")
+        assert params == {"a": 1, "b": 2}
+
+    def test_preserves_whitespace_in_values(self):
+        from concore import parse_params
+        params = parse_params("label = hello world ; x = 5")
+        assert params["label"] == "hello world"
+        assert params["x"] == 5
+
+    def test_embedded_equals_in_value(self):
+        from concore import parse_params
+        params = parse_params("url=https://example.com?a=1&b=2")
+        assert params["url"] == "https://example.com?a=1&b=2"
+
+    def test_numeric_and_list_coercion(self):
+        from concore import parse_params
+        params = parse_params("delay=5;coeffs=[1,2,3]")
+        assert params["delay"] == 5
+        assert params["coeffs"] == [1, 2, 3]
+
+    def test_dict_literal_backward_compatibility(self):
+        from concore import parse_params
+        params = parse_params("{'a': 1, 'b': 2}")
+        assert params == {"a": 1, "b": 2}
+
+    def test_windows_quoted_input(self):
+        from concore import parse_params
+        s = "\"a=1;b=2\""
+        s = s[1:-1]  # simulate quote stripping before parse_params
+        params = parse_params(s)
+        assert params == {"a": 1, "b": 2}