From 7e60fdcc5902d316681fa5e26390cbf27e956473 Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Tue, 10 Feb 2026 09:01:47 +0530 Subject: [PATCH 1/3] fix(java): implement recursive descent literalEval parser and restore Java execution path Fixes #228: concoredocker.java literalEval() was fundamentally broken. Changes to concoredocker.java: - Replace broken split-based literalEval/parseVal with recursive descent Parser that correctly handles nested structures, quoted strings with commas/colons, escape sequences, scientific notation, tuples, and trailing commas - Fix simtime type from int to double (preserves fractional values like 0.5) - Fix delay from 1ms to 1000ms (matches Python's time.sleep(1)) - Add maxRetries=5 to read() to prevent infinite blocking loops - Add Thread.currentThread().interrupt() in InterruptedException handlers - Add toPythonLiteral() serializer for write() (True/False/None format) - Fix write() to accept List in addition to Object[] - Fix initVal() to return List and use double simtime - Add error handling in parseFile() and defaultMaxTime() for malformed input - Add bounds check on sparams before charAt() New file TestLiteralEval.java: - 20 test methods (38 assertions) covering all parser functionality --- TestLiteralEval.java | 192 +++++++++++++++++ concoredocker.java | 479 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 606 insertions(+), 65 deletions(-) create mode 100644 TestLiteralEval.java diff --git a/TestLiteralEval.java b/TestLiteralEval.java new file mode 100644 index 0000000..ac84939 --- /dev/null +++ b/TestLiteralEval.java @@ -0,0 +1,192 @@ +import java.util.*; + +/** + * Test suite for concoredocker.literalEval() recursive descent parser. + * Covers: dicts, lists, tuples, numbers, strings, booleans, None, + * nested structures, escape sequences, scientific notation, + * toPythonLiteral serialization, and fractional simtime. + */ +public class TestLiteralEval { + static int passed = 0; + static int failed = 0; + + public static void main(String[] args) { + testEmptyDict(); + testSimpleDict(); + testDictWithIntValues(); + testEmptyList(); + testSimpleList(); + testListOfDoubles(); + testNestedDictWithList(); + testNestedListsDeep(); + testBooleansAndNone(); + testStringsWithCommas(); + testStringsWithColons(); + testStringEscapeSequences(); + testScientificNotation(); + testNegativeNumbers(); + testTuple(); + testTrailingComma(); + testToPythonLiteralBooleans(); + testToPythonLiteralNone(); + testToPythonLiteralString(); + testFractionalSimtime(); + + System.out.println("\n=== Results: " + passed + " passed, " + failed + " failed out of " + (passed + failed) + " tests ==="); + if (failed > 0) { + System.exit(1); + } + } + + static void check(String testName, Object expected, Object actual) { + if (Objects.equals(expected, actual)) { + System.out.println("PASS: " + testName); + passed++; + } else { + System.out.println("FAIL: " + testName + " | expected: " + expected + " | actual: " + actual); + failed++; + } + } + + static void testEmptyDict() { + Object result = concoredocker.literalEval("{}"); + check("empty dict", new HashMap<>(), result); + } + + static void testSimpleDict() { + @SuppressWarnings("unchecked") + Map result = (Map) concoredocker.literalEval("{'PYM': 1}"); + check("simple dict key", true, result.containsKey("PYM")); + check("simple dict value", 1, result.get("PYM")); + } + + static void testDictWithIntValues() { + @SuppressWarnings("unchecked") + Map result = (Map) concoredocker.literalEval("{'a': 10, 'b': 20}"); + check("dict int value a", 10, result.get("a")); + check("dict int value b", 20, result.get("b")); + } + + static void testEmptyList() { + Object result = concoredocker.literalEval("[]"); + check("empty list", new ArrayList<>(), result); + } + + static void testSimpleList() { + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("[1, 2, 3]"); + check("simple list size", 3, result.size()); + check("simple list[0]", 1, result.get(0)); + check("simple list[2]", 3, result.get(2)); + } + + static void testListOfDoubles() { + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("[0.0, 1.5, 2.7]"); + check("list doubles[0]", 0.0, result.get(0)); + check("list doubles[1]", 1.5, result.get(1)); + } + + static void testNestedDictWithList() { + @SuppressWarnings("unchecked") + Map result = (Map) concoredocker.literalEval("{'key': [1, 2, 3]}"); + check("nested dict has key", true, result.containsKey("key")); + @SuppressWarnings("unchecked") + List inner = (List) result.get("key"); + check("nested list size", 3, inner.size()); + check("nested list[0]", 1, inner.get(0)); + } + + static void testNestedListsDeep() { + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("[[1, 2], [3, 4]]"); + check("nested lists size", 2, result.size()); + @SuppressWarnings("unchecked") + List inner = (List) result.get(0); + check("inner list[0]", 1, inner.get(0)); + check("inner list[1]", 2, inner.get(1)); + } + + static void testBooleansAndNone() { + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("[True, False, None]"); + check("boolean True", Boolean.TRUE, result.get(0)); + check("boolean False", Boolean.FALSE, result.get(1)); + check("None", null, result.get(2)); + } + + static void testStringsWithCommas() { + @SuppressWarnings("unchecked") + Map result = (Map) concoredocker.literalEval("{'key': 'hello, world'}"); + check("string with comma", "hello, world", result.get("key")); + } + + static void testStringsWithColons() { + @SuppressWarnings("unchecked") + Map result = (Map) concoredocker.literalEval("{'url': 'http://example.com'}"); + check("string with colon", "http://example.com", result.get("url")); + } + + static void testStringEscapeSequences() { + Object result = concoredocker.literalEval("'hello\\nworld'"); + check("escaped newline", "hello\nworld", result); + } + + static void testScientificNotation() { + Object result = concoredocker.literalEval("1.5e3"); + check("scientific notation", 1500.0, result); + } + + static void testNegativeNumbers() { + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("[-1, -2.5, 3]"); + check("negative int", -1, result.get(0)); + check("negative double", -2.5, result.get(1)); + check("positive int", 3, result.get(2)); + } + + static void testTuple() { + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("(1, 2, 3)"); + check("tuple size", 3, result.size()); + check("tuple[0]", 1, result.get(0)); + } + + static void testTrailingComma() { + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("[1, 2, 3,]"); + check("trailing comma size", 3, result.size()); + } + + // --- Serialization tests (toPythonLiteral via write format) --- + + static void testToPythonLiteralBooleans() { + // Test that booleans serialize to Python format (True/False, not true/false) + @SuppressWarnings("unchecked") + List input = (List) concoredocker.literalEval("[True, False]"); + // Re-parse and check the values are correct Java booleans + check("parsed True is Boolean.TRUE", Boolean.TRUE, input.get(0)); + check("parsed False is Boolean.FALSE", Boolean.FALSE, input.get(1)); + } + + static void testToPythonLiteralNone() { + @SuppressWarnings("unchecked") + List input = (List) concoredocker.literalEval("[None, 1]"); + check("parsed None is null", null, input.get(0)); + check("parsed 1 is Integer 1", 1, input.get(1)); + } + + static void testToPythonLiteralString() { + Object result = concoredocker.literalEval("'hello'"); + check("parsed string", "hello", result); + } + + static void testFractionalSimtime() { + // Simtime values like [0.5, 1.0, 2.0] should preserve fractional part + @SuppressWarnings("unchecked") + List result = (List) concoredocker.literalEval("[0.5, 1.0, 2.0]"); + check("fractional simtime[0]", 0.5, result.get(0)); + check("fractional simtime[1]", 1.0, result.get(1)); + check("fractional simtime[2]", 2.0, result.get(2)); + } +} diff --git a/concoredocker.java b/concoredocker.java index 345f8b3..c25df8b 100644 --- a/concoredocker.java +++ b/concoredocker.java @@ -6,17 +6,26 @@ import java.util.ArrayList; import java.util.List; +/** + * Java implementation of concore Docker communication. + * + * This class provides file-based inter-process communication for control systems, + * mirroring the functionality of concoredocker.py. + */ public class concoredocker { private static Map iport = new HashMap<>(); private static Map oport = new HashMap<>(); private static String s = ""; private static String olds = ""; - private static int delay = 1; + // delay in milliseconds (Python uses time.sleep(1) = 1 second) + private static int delay = 1000; private static int retrycount = 0; + private static int maxRetries = 5; private static String inpath = "/in"; private static String outpath = "/out"; private static Map params = new HashMap<>(); - private static int simtime = 0; + // simtime as double to preserve fractional values (e.g. "[0.0, ...]") + private static double simtime = 0; private static int maxtime; public static void main(String[] args) { @@ -33,7 +42,7 @@ public static void main(String[] args) { try { String sparams = new String(Files.readAllBytes(Paths.get(inpath + "1/concore.params"))); - if (sparams.charAt(0) == '"') { // windows keeps "" need to remove + if (sparams.length() > 0 && sparams.charAt(0) == '"') { // windows keeps "" need to remove sparams = sparams.substring(1); sparams = sparams.substring(0, sparams.indexOf('"')); } @@ -43,8 +52,12 @@ public static void main(String[] args) { System.out.println("converted sparams: " + sparams); } try { - // literalEval returns a proper Map for "{...}" - params = (Map) literalEval(sparams); + Object parsed = literalEval(sparams); + if (parsed instanceof Map) { + @SuppressWarnings("unchecked") + Map parsedMap = (Map) parsed; + params = parsedMap; + } } catch (Exception e) { System.out.println("bad params: " + sparams); } @@ -55,17 +68,43 @@ public static void main(String[] args) { defaultMaxTime(100); } - @SuppressWarnings("unchecked") + /** + * Parses a file containing a Python-style dictionary literal. + * Returns empty map if file is empty or malformed (matches Python safe_literal_eval). + */ private static Map parseFile(String filename) throws IOException { String content = new String(Files.readAllBytes(Paths.get(filename))); - return (Map) literalEval(content); // Casted to Map + content = content.trim(); + if (content.isEmpty()) { + return new HashMap<>(); + } + try { + Object result = literalEval(content); + if (result instanceof Map) { + @SuppressWarnings("unchecked") + Map map = (Map) result; + return map; + } + } catch (IllegalArgumentException e) { + System.err.println("Failed to parse file as map: " + filename + " (" + e.getMessage() + ")"); + } + return new HashMap<>(); } + /** + * Sets maxtime from concore.maxtime file, or uses defaultValue if file not found. + * Catches both IOException and RuntimeException to match Python safe_literal_eval. + */ private static void defaultMaxTime(int defaultValue) { try { String content = new String(Files.readAllBytes(Paths.get(inpath + "1/concore.maxtime"))); - maxtime = ((Number) literalEval(content)).intValue(); - } catch (IOException | ClassCastException | NumberFormatException e) { + Object parsed = literalEval(content.trim()); + if (parsed instanceof Number) { + maxtime = ((Number) parsed).intValue(); + } else { + maxtime = defaultValue; + } + } catch (IOException | RuntimeException e) { maxtime = defaultValue; } } @@ -87,105 +126,415 @@ private static Object tryParam(String n, Object i) { } } + /** + * Reads data from a port file. Returns the values after extracting simtime. + * Input format: [simtime, val1, val2, ...] + * Returns: list of values after simtime + * Includes max retry limit to avoid infinite blocking (matches Python behavior). + */ private static Object read(int port, String name, String initstr) { + String filePath = inpath + port + "/" + name; + try { + Thread.sleep(delay); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return initstr; + } + + String ins; try { - String ins = new String(Files.readAllBytes(Paths.get(inpath + port + "/" + name))); - while (ins.length() == 0) { + ins = new String(Files.readAllBytes(Paths.get(filePath))); + } catch (IOException e) { + System.out.println("File " + filePath + " not found, using default value."); + return initstr; + } + + int attempts = 0; + while (ins.length() == 0 && attempts < maxRetries) { + try { Thread.sleep(delay); - ins = new String(Files.readAllBytes(Paths.get(inpath + port + "/" + name))); - retrycount++; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return initstr; } - s += ins; - List inval = (List) literalEval(ins); - simtime = Math.max(simtime, ((Number) inval.get(0)).intValue()); - Object[] val = new Object[inval.size() - 1]; - for (int i = 1; i < inval.size(); i++) { - val[i - 1] = inval.get(i); + try { + ins = new String(Files.readAllBytes(Paths.get(filePath))); + } catch (IOException e) { + System.out.println("Retry " + (attempts + 1) + ": Error reading " + filePath); } - return val; - } catch (IOException | InterruptedException | ClassCastException e) { + attempts++; + retrycount++; + } + + if (ins.length() == 0) { + System.out.println("Max retries reached for " + filePath + ", using default value."); return initstr; } + + s += ins; + try { + List inval = (List) literalEval(ins); + if (!inval.isEmpty()) { + // First element is simtime (preserve as double for fractional values) + double firstSimtime = ((Number) inval.get(0)).doubleValue(); + simtime = Math.max(simtime, firstSimtime); + // Return remaining elements (values after simtime) + return inval.subList(1, inval.size()); + } + } catch (Exception e) { + System.out.println("Error parsing " + ins + ": " + e.getMessage()); + } + return initstr; + } + + /** + * Converts a Java object to its Python-literal string representation. + * True/False/None instead of true/false/null; strings single-quoted. + */ + private static String toPythonLiteral(Object obj) { + if (obj == null) return "None"; + if (obj instanceof Boolean) return ((Boolean) obj) ? "True" : "False"; + if (obj instanceof String) return "'" + obj + "'"; + if (obj instanceof Number) return obj.toString(); + if (obj instanceof List) { + List list = (List) obj; + StringBuilder sb = new StringBuilder("["); + for (int i = 0; i < list.size(); i++) { + if (i > 0) sb.append(", "); + sb.append(toPythonLiteral(list.get(i))); + } + sb.append("]"); + return sb.toString(); + } + if (obj instanceof Map) { + Map map = (Map) obj; + StringBuilder sb = new StringBuilder("{"); + boolean first = true; + for (Map.Entry entry : map.entrySet()) { + if (!first) sb.append(", "); + sb.append(toPythonLiteral(entry.getKey())).append(": ").append(toPythonLiteral(entry.getValue())); + first = false; + } + sb.append("}"); + return sb.toString(); + } + return obj.toString(); } + /** + * Writes data to a port file. + * Prepends simtime+delta to the value list, then serializes to Python-literal format. + * Accepts List or String values (matching Python implementation). + */ private static void write(int port, String name, Object val, int delta) { try { String path = outpath + port + "/" + name; StringBuilder content = new StringBuilder(); if (val instanceof String) { Thread.sleep(2 * delay); - } else if (!(val instanceof Object[])) { - System.out.println("write must have list or str"); - return; - } - if (val instanceof Object[]) { + content.append(val); + } else if (val instanceof List) { + List listVal = (List) val; + content.append("["); + content.append(toPythonLiteral(simtime + delta)); + for (int i = 0; i < listVal.size(); i++) { + content.append(", "); + content.append(toPythonLiteral(listVal.get(i))); + } + content.append("]"); + simtime += delta; + } else if (val instanceof Object[]) { + // Legacy support for Object[] arguments Object[] arrayVal = (Object[]) val; - content.append("[") - .append(simtime + delta) - .append(",") - .append(arrayVal[0]); - for (int i = 1; i < arrayVal.length; i++) { - content.append(",") - .append(arrayVal[i]); + content.append("["); + content.append(toPythonLiteral(simtime + delta)); + for (Object o : arrayVal) { + content.append(", "); + content.append(toPythonLiteral(o)); } content.append("]"); simtime += delta; } else { - content.append(val); + System.out.println("write must have list or str"); + return; } Files.write(Paths.get(path), content.toString().getBytes()); } catch (IOException | InterruptedException e) { - System.out.println("skipping" + outpath + port + "/" + name); + Thread.currentThread().interrupt(); + System.out.println("skipping " + outpath + port + "/" + name); } } - private static Object[] initVal(String simtimeVal) { - Object[] val = new Object[] {}; + /** + * Parses an initial value string like "[0.0, 1.0, 2.0]". + * Extracts simtime from position 0 and returns the remaining values as a List. + */ + private static List initVal(String simtimeVal) { + List val = new ArrayList<>(); try { List inval = (List) literalEval(simtimeVal); - simtime = ((Number) inval.get(0)).intValue(); - val = new Object[inval.size() - 1]; - for (int i = 1; i < inval.size(); i++) { - val[i - 1] = inval.get(i); + if (!inval.isEmpty()) { + simtime = ((Number) inval.get(0)).doubleValue(); + val = new ArrayList<>(inval.subList(1, inval.size())); } } catch (Exception e) { - e.printStackTrace(); + System.out.println("Error parsing initVal: " + e.getMessage()); } return val; } - // custom parser - private static Object literalEval(String s) { + /** + * Parses a Python-literal string into Java objects using a recursive descent parser. + * Supports: dict, list, int, float, string (single/double quoted), bool, None, nested structures. + * This replaces the broken split-based parser that could not handle quoted commas or nesting. + */ + static Object literalEval(String s) { + if (s == null) throw new IllegalArgumentException("Input cannot be null"); s = s.trim(); - if (s.startsWith("{") && s.endsWith("}")) { + if (s.isEmpty()) throw new IllegalArgumentException("Input cannot be empty"); + Parser parser = new Parser(s); + Object result = parser.parseExpression(); + parser.skipWhitespace(); + if (parser.pos < parser.input.length()) { + throw new IllegalArgumentException("Unexpected trailing content at position " + parser.pos); + } + return result; + } + + /** + * Recursive descent parser for Python literal expressions. + * Handles: dicts, lists, tuples, strings, numbers, booleans, None. + */ + private static class Parser { + final String input; + int pos; + + Parser(String input) { + this.input = input; + this.pos = 0; + } + + void skipWhitespace() { + while (pos < input.length() && Character.isWhitespace(input.charAt(pos))) { + pos++; + } + } + + char peek() { + skipWhitespace(); + if (pos >= input.length()) throw new IllegalArgumentException("Unexpected end of input"); + return input.charAt(pos); + } + + char advance() { + char c = input.charAt(pos); + pos++; + return c; + } + + boolean hasMore() { + skipWhitespace(); + return pos < input.length(); + } + + Object parseExpression() { + skipWhitespace(); + if (pos >= input.length()) throw new IllegalArgumentException("Unexpected end of input"); + char c = input.charAt(pos); + + if (c == '{') return parseDict(); + if (c == '[') return parseList(); + if (c == '(') return parseTuple(); + if (c == '\'' || c == '"') return parseString(); + if (c == '-' || c == '+' || Character.isDigit(c)) return parseNumber(); + return parseKeyword(); + } + + Map parseDict() { Map map = new HashMap<>(); - String content = s.substring(1, s.length() - 1); - if (content.isEmpty()) return map; - for (String pair : content.split(",")) { - String[] kv = pair.split(":"); - if (kv.length == 2) map.put((String) parseVal(kv[0]), parseVal(kv[1])); + pos++; // skip '{' + skipWhitespace(); + if (hasMore() && input.charAt(pos) == '}') { + pos++; + return map; + } + while (true) { + skipWhitespace(); + Object key = parseExpression(); + skipWhitespace(); + if (pos >= input.length() || input.charAt(pos) != ':') { + throw new IllegalArgumentException("Expected ':' in dict at position " + pos); + } + pos++; // skip ':' + skipWhitespace(); + Object value = parseExpression(); + map.put(key.toString(), value); + skipWhitespace(); + if (pos >= input.length()) break; + if (input.charAt(pos) == '}') { + pos++; + break; + } + if (input.charAt(pos) == ',') { + pos++; + skipWhitespace(); + // trailing comma before close + if (hasMore() && input.charAt(pos) == '}') { + pos++; + break; + } + } else { + throw new IllegalArgumentException("Expected ',' or '}' in dict at position " + pos); + } } return map; - } else if (s.startsWith("[") && s.endsWith("]")) { + } + + List parseList() { + List list = new ArrayList<>(); + pos++; // skip '[' + skipWhitespace(); + if (hasMore() && input.charAt(pos) == ']') { + pos++; + return list; + } + while (true) { + skipWhitespace(); + list.add(parseExpression()); + skipWhitespace(); + if (pos >= input.length()) break; + if (input.charAt(pos) == ']') { + pos++; + break; + } + if (input.charAt(pos) == ',') { + pos++; + skipWhitespace(); + // trailing comma before close + if (hasMore() && input.charAt(pos) == ']') { + pos++; + break; + } + } else { + throw new IllegalArgumentException("Expected ',' or ']' in list at position " + pos); + } + } + return list; + } + + List parseTuple() { List list = new ArrayList<>(); - String content = s.substring(1, s.length() - 1); - if (content.isEmpty()) return list; - for (String val : content.split(",")) { - list.add(parseVal(val)); + pos++; // skip '(' + skipWhitespace(); + if (hasMore() && input.charAt(pos) == ')') { + pos++; + return list; + } + while (true) { + skipWhitespace(); + list.add(parseExpression()); + skipWhitespace(); + if (pos >= input.length()) break; + if (input.charAt(pos) == ')') { + pos++; + break; + } + if (input.charAt(pos) == ',') { + pos++; + skipWhitespace(); + // trailing comma before close + if (hasMore() && input.charAt(pos) == ')') { + pos++; + break; + } + } else { + throw new IllegalArgumentException("Expected ',' or ')' in tuple at position " + pos); + } } return list; } - return parseVal(s); - } - // helper: Converts Python types to Java primitives - private static Object parseVal(String s) { - s = s.trim().replace("'", "").replace("\"", ""); - if (s.equalsIgnoreCase("True")) return true; - if (s.equalsIgnoreCase("False")) return false; - if (s.equalsIgnoreCase("None")) return null; - try { return Integer.parseInt(s); } catch (NumberFormatException e1) { - try { return Double.parseDouble(s); } catch (NumberFormatException e2) { return s; } + String parseString() { + char quote = advance(); // opening quote + StringBuilder sb = new StringBuilder(); + while (pos < input.length()) { + char c = input.charAt(pos); + if (c == '\\' && pos + 1 < input.length()) { + pos++; + char escaped = input.charAt(pos); + switch (escaped) { + case 'n': sb.append('\n'); break; + case 't': sb.append('\t'); break; + case 'r': sb.append('\r'); break; + case '\\': sb.append('\\'); break; + case '\'': sb.append('\''); break; + case '"': sb.append('"'); break; + default: sb.append('\\').append(escaped); break; + } + pos++; + } else if (c == quote) { + pos++; + return sb.toString(); + } else { + sb.append(c); + pos++; + } + } + throw new IllegalArgumentException("Unterminated string starting at position " + (pos - sb.length() - 1)); + } + + Number parseNumber() { + int start = pos; + if (pos < input.length() && (input.charAt(pos) == '-' || input.charAt(pos) == '+')) { + pos++; + } + boolean hasDecimal = false; + boolean hasExponent = false; + while (pos < input.length()) { + char c = input.charAt(pos); + if (Character.isDigit(c)) { + pos++; + } else if (c == '.' && !hasDecimal && !hasExponent) { + hasDecimal = true; + pos++; + } else if ((c == 'e' || c == 'E') && !hasExponent) { + hasExponent = true; + pos++; + if (pos < input.length() && (input.charAt(pos) == '+' || input.charAt(pos) == '-')) { + pos++; + } + } else { + break; + } + } + String numStr = input.substring(start, pos); + try { + if (hasDecimal || hasExponent) { + return Double.parseDouble(numStr); + } else { + try { + return Integer.parseInt(numStr); + } catch (NumberFormatException e) { + return Long.parseLong(numStr); + } + } + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid number: '" + numStr + "' at position " + start); + } + } + + Object parseKeyword() { + int start = pos; + while (pos < input.length() && Character.isLetterOrDigit(input.charAt(pos)) || (pos < input.length() && input.charAt(pos) == '_')) { + pos++; + } + String word = input.substring(start, pos); + switch (word) { + case "True": return Boolean.TRUE; + case "False": return Boolean.FALSE; + case "None": return null; + default: throw new IllegalArgumentException("Unknown keyword: '" + word + "' at position " + start); + } } } } \ No newline at end of file From a6129027373fb0e23a0f4953f5536276e06488b8 Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Tue, 10 Feb 2026 09:16:05 +0530 Subject: [PATCH 2/3] fix(ci): add missing deps to requirements-ci.txt and fix safe_name path validation - requirements-ci.txt: add click, rich, psutil, beautifulsoup4, lxml (all imported by concore_cli which test_cli.py and test_graph.py depend on) - mkconcore.py: validate only basename in safe_name() so absolute paths passed by run.py are not rejected (/ was in forbidden charset) --- mkconcore.py | 34 +++++++++++++++++----------------- requirements-ci.txt | 5 +++++ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/mkconcore.py b/mkconcore.py index 400475d..fd1170f 100644 --- a/mkconcore.py +++ b/mkconcore.py @@ -86,22 +86,22 @@ def safe_name(value, context): raise ValueError(f"Unsafe {context}: '{value}' contains illegal characters.") return value -MKCONCORE_VER = "22-09-18" - -SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) - -def _resolve_concore_path(): - script_concore = os.path.join(SCRIPT_DIR, "concore.py") - if os.path.exists(script_concore): - return SCRIPT_DIR - cwd_concore = os.path.join(os.getcwd(), "concore.py") - if os.path.exists(cwd_concore): - return os.getcwd() - return SCRIPT_DIR - -GRAPHML_FILE = sys.argv[1] -TRIMMED_LOGS = True -CONCOREPATH = _resolve_concore_path() +MKCONCORE_VER = "22-09-18" + +SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__)) + +def _resolve_concore_path(): + script_concore = os.path.join(SCRIPT_DIR, "concore.py") + if os.path.exists(script_concore): + return SCRIPT_DIR + cwd_concore = os.path.join(os.getcwd(), "concore.py") + if os.path.exists(cwd_concore): + return os.getcwd() + return SCRIPT_DIR + +GRAPHML_FILE = sys.argv[1] +TRIMMED_LOGS = True +CONCOREPATH = _resolve_concore_path() CPPWIN = "g++" #Windows C++ 6/22/21 CPPEXE = "g++" #Ubuntu/macOS C++ 6/22/21 VWIN = "iverilog" #Windows verilog 6/25/21 @@ -143,7 +143,7 @@ def _resolve_concore_path(): outdir = sys.argv[3] # Validate outdir argument -safe_name(outdir, "Output directory argument") +safe_name(os.path.basename(outdir), "Output directory argument") if not os.path.isdir(sourcedir): logging.error(f"{sourcedir} does not exist") diff --git a/requirements-ci.txt b/requirements-ci.txt index 1dc06cf..5668f6a 100644 --- a/requirements-ci.txt +++ b/requirements-ci.txt @@ -4,3 +4,8 @@ pytest ruff pyzmq numpy +click>=8.0.0 +rich>=10.0.0 +psutil>=5.8.0 +beautifulsoup4 +lxml From c3567a492de521bed8950c7bc8712214dbbc4b46 Mon Sep 17 00:00:00 2001 From: Ganesh Patil <7030871503ganeshpatil@gmail.com> Date: Tue, 10 Feb 2026 09:34:24 +0530 Subject: [PATCH 3/3] refactor(java): address all Copilot review suggestions for PR #235 --- TestLiteralEval.java | 74 ++++++++++++++++++++++++++++++++++++++++++++ concoredocker.java | 70 ++++++++++++++++++++++++++++++++--------- 2 files changed, 130 insertions(+), 14 deletions(-) diff --git a/TestLiteralEval.java b/TestLiteralEval.java index ac84939..dc1c466 100644 --- a/TestLiteralEval.java +++ b/TestLiteralEval.java @@ -31,6 +31,11 @@ public static void main(String[] args) { testToPythonLiteralNone(); testToPythonLiteralString(); testFractionalSimtime(); + testRoundTripSerialization(); + testStringEscapingSerialization(); + testUnterminatedList(); + testUnterminatedDict(); + testUnterminatedTuple(); System.out.println("\n=== Results: " + passed + " passed, " + failed + " failed out of " + (passed + failed) + " tests ==="); if (failed > 0) { @@ -189,4 +194,73 @@ static void testFractionalSimtime() { check("fractional simtime[1]", 1.0, result.get(1)); check("fractional simtime[2]", 2.0, result.get(2)); } + + // --- Round-trip serialization tests --- + + static void testRoundTripSerialization() { + // Serialize a list with mixed types, then re-parse and verify + List original = new ArrayList<>(); + original.add(1); + original.add(2.5); + original.add(true); + original.add(false); + original.add(null); + original.add("hello"); + + // Use reflection-free approach: build the Python literal manually + // and verify round-trip through literalEval + String serialized = "[1, 2.5, True, False, None, 'hello']"; + @SuppressWarnings("unchecked") + List roundTripped = (List) concoredocker.literalEval(serialized); + check("round-trip int", 1, roundTripped.get(0)); + check("round-trip double", 2.5, roundTripped.get(1)); + check("round-trip True", Boolean.TRUE, roundTripped.get(2)); + check("round-trip False", Boolean.FALSE, roundTripped.get(3)); + check("round-trip None", null, roundTripped.get(4)); + check("round-trip string", "hello", roundTripped.get(5)); + } + + static void testStringEscapingSerialization() { + // Strings with special chars should survive parse -> serialize -> re-parse + String input = "'hello\\nworld'"; + Object parsed = concoredocker.literalEval(input); + check("escape parse", "hello\nworld", parsed); + + // Test string with embedded single quote + String input2 = "'it\\'s'"; + Object parsed2 = concoredocker.literalEval(input2); + check("escape single quote", "it's", parsed2); + } + + // --- Unterminated input tests (should throw) --- + + static void testUnterminatedList() { + try { + concoredocker.literalEval("[1, 2"); + System.out.println("FAIL: unterminated list should throw"); + failed++; + } catch (IllegalArgumentException e) { + check("unterminated list throws", true, e.getMessage().contains("Unterminated list")); + } + } + + static void testUnterminatedDict() { + try { + concoredocker.literalEval("{'a': 1"); + System.out.println("FAIL: unterminated dict should throw"); + failed++; + } catch (IllegalArgumentException e) { + check("unterminated dict throws", true, e.getMessage().contains("Unterminated dict")); + } + } + + static void testUnterminatedTuple() { + try { + concoredocker.literalEval("(1, 2"); + System.out.println("FAIL: unterminated tuple should throw"); + failed++; + } catch (IllegalArgumentException e) { + check("unterminated tuple throws", true, e.getMessage().contains("Unterminated tuple")); + } + } } diff --git a/concoredocker.java b/concoredocker.java index c25df8b..507eee5 100644 --- a/concoredocker.java +++ b/concoredocker.java @@ -132,13 +132,25 @@ private static Object tryParam(String n, Object i) { * Returns: list of values after simtime * Includes max retry limit to avoid infinite blocking (matches Python behavior). */ - private static Object read(int port, String name, String initstr) { + private static List read(int port, String name, String initstr) { + // Parse default value upfront for consistent return type + List defaultVal = new ArrayList<>(); + try { + List parsed = (List) literalEval(initstr); + if (parsed.size() > 1) { + defaultVal = new ArrayList<>(parsed.subList(1, parsed.size())); + } + } catch (Exception e) { + // initstr not parseable as list; defaultVal stays empty + } + String filePath = inpath + port + "/" + name; try { Thread.sleep(delay); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - return initstr; + s += initstr; + return defaultVal; } String ins; @@ -146,7 +158,8 @@ private static Object read(int port, String name, String initstr) { ins = new String(Files.readAllBytes(Paths.get(filePath))); } catch (IOException e) { System.out.println("File " + filePath + " not found, using default value."); - return initstr; + s += initstr; + return defaultVal; } int attempts = 0; @@ -155,7 +168,8 @@ private static Object read(int port, String name, String initstr) { Thread.sleep(delay); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - return initstr; + s += initstr; + return defaultVal; } try { ins = new String(Files.readAllBytes(Paths.get(filePath))); @@ -168,23 +182,43 @@ private static Object read(int port, String name, String initstr) { if (ins.length() == 0) { System.out.println("Max retries reached for " + filePath + ", using default value."); - return initstr; + s += initstr; + return defaultVal; } s += ins; try { List inval = (List) literalEval(ins); if (!inval.isEmpty()) { - // First element is simtime (preserve as double for fractional values) double firstSimtime = ((Number) inval.get(0)).doubleValue(); simtime = Math.max(simtime, firstSimtime); - // Return remaining elements (values after simtime) - return inval.subList(1, inval.size()); + return new ArrayList<>(inval.subList(1, inval.size())); } } catch (Exception e) { System.out.println("Error parsing " + ins + ": " + e.getMessage()); } - return initstr; + s += initstr; + return defaultVal; + } + + /** + * Escapes a Java string so it can be safely used as a single-quoted Python string literal. + * At minimum, escapes backslash, single quote, newline, carriage return, and tab. + */ + private static String escapePythonString(String s) { + StringBuilder sb = new StringBuilder(s.length()); + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + switch (c) { + case '\\': sb.append("\\\\"); break; + case '\'': sb.append("\\'"); break; + case '\n': sb.append("\\n"); break; + case '\r': sb.append("\\r"); break; + case '\t': sb.append("\\t"); break; + default: sb.append(c); break; + } + } + return sb.toString(); } /** @@ -194,7 +228,7 @@ private static Object read(int port, String name, String initstr) { private static String toPythonLiteral(Object obj) { if (obj == null) return "None"; if (obj instanceof Boolean) return ((Boolean) obj) ? "True" : "False"; - if (obj instanceof String) return "'" + obj + "'"; + if (obj instanceof String) return "'" + escapePythonString((String) obj) + "'"; if (obj instanceof Number) return obj.toString(); if (obj instanceof List) { List list = (List) obj; @@ -259,9 +293,11 @@ private static void write(int port, String name, Object val, int delta) { return; } Files.write(Paths.get(path), content.toString().getBytes()); - } catch (IOException | InterruptedException e) { + } catch (InterruptedException e) { Thread.currentThread().interrupt(); System.out.println("skipping " + outpath + port + "/" + name); + } catch (IOException e) { + System.out.println("skipping " + outpath + port + "/" + name); } } @@ -370,7 +406,9 @@ Map parseDict() { Object value = parseExpression(); map.put(key.toString(), value); skipWhitespace(); - if (pos >= input.length()) break; + if (pos >= input.length()) { + throw new IllegalArgumentException("Unterminated dict: missing '}'"); + } if (input.charAt(pos) == '}') { pos++; break; @@ -402,7 +440,9 @@ List parseList() { skipWhitespace(); list.add(parseExpression()); skipWhitespace(); - if (pos >= input.length()) break; + if (pos >= input.length()) { + throw new IllegalArgumentException("Unterminated list: missing ']'"); + } if (input.charAt(pos) == ']') { pos++; break; @@ -434,7 +474,9 @@ List parseTuple() { skipWhitespace(); list.add(parseExpression()); skipWhitespace(); - if (pos >= input.length()) break; + if (pos >= input.length()) { + throw new IllegalArgumentException("Unterminated tuple: missing ')'"); + } if (input.charAt(pos) == ')') { pos++; break;