Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GREENRAT-K405 wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a607e75
add robust parameter parsing, fixes #184
GREENRAT-K405 Feb 4, 2026
0791056
fix dict issue, delimiter causing break in line, and file remains opened
GREENRAT-K405 Feb 4, 2026
6877489
add test class TestParseParams
GREENRAT-K405 Feb 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions concore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +175 to +187
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_params splits on ; without honoring quoting/escaping, so any value containing a semicolon (e.g., URLs or user strings) will be split into multiple items and parsed incorrectly. If semicolons are expected inside values, switch to a parser that tokenizes key/value pairs while tracking quote/bracket depth (or require dict/JSON literals for such cases).

Copilot uses AI. Check for mistakes.

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:
Expand Down
57 changes: 45 additions & 12 deletions concoredocker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +44 to +56
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_params splits on commas unconditionally (s.split(",")), which will corrupt values that legally contain commas (e.g., list literals like a=[1,2], CSV-like strings, or quoted strings containing commas). This contradicts the goal of supporting structured values via literal_eval. Consider implementing a delimiter-splitting routine that respects quotes/bracket nesting (or require full dict literals/JSON when values may contain commas).

Copilot uses AI. Check for mistakes.
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
Expand Down
39 changes: 38 additions & 1 deletion tests/test_concore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}