From 297c6df1b1f5524aced8be5a84b56ffa9d8237b4 Mon Sep 17 00:00:00 2001 From: AGulev Date: Wed, 20 May 2026 15:43:35 +0200 Subject: [PATCH] better command line arguments parser --- .../process/CommandLineTokenizer.java | 160 ++++++++++++++++++ .../extender/process/ProcessExecutor.java | 5 +- .../services/cocoapods/PodBuildSpec.java | 16 +- .../services/cocoapods/XCConfigParser.java | 122 +++++++++---- .../process/CommandLineTokenizerTest.java | 109 ++++++++++++ .../services/cocoapods/PodBuildSpecTest.java | 31 ++++ .../cocoapods/XCConfigParserTest.java | 9 +- 7 files changed, 406 insertions(+), 46 deletions(-) create mode 100644 server/src/main/java/com/defold/extender/process/CommandLineTokenizer.java create mode 100644 server/src/test/java/com/defold/extender/process/CommandLineTokenizerTest.java diff --git a/server/src/main/java/com/defold/extender/process/CommandLineTokenizer.java b/server/src/main/java/com/defold/extender/process/CommandLineTokenizer.java new file mode 100644 index 00000000..fe7abf0c --- /dev/null +++ b/server/src/main/java/com/defold/extender/process/CommandLineTokenizer.java @@ -0,0 +1,160 @@ +package com.defold.extender.process; + +import java.util.ArrayList; +import java.util.List; + +public final class CommandLineTokenizer { + private CommandLineTokenizer() {} + + public static List parse(String command) { + return tokenize(command, false); + } + + public static List splitPreservingEscapedWhitespace(String arguments) { + return tokenize(arguments, true); + } + + public static String escapeWhitespace(String argument) { + StringBuilder result = new StringBuilder(); + boolean escaping = false; + + for (int i = 0; i < argument.length(); ++i) { + char c = argument.charAt(i); + if (escaping) { + result.append('\\'); + result.append(c); + escaping = false; + } else if (c == '\\') { + escaping = true; + } else if (Character.isWhitespace(c)) { + result.append('\\'); + result.append(c); + } else { + result.append(c); + } + } + + if (escaping) { + result.append('\\'); + } + + return result.toString(); + } + + private static List tokenize(String input, boolean preserveEscapes) { + List result = new ArrayList<>(); + if (input == null || input.trim().isEmpty()) { + return result; + } + + StringBuilder token = new StringBuilder(); + boolean inToken = false; + boolean escaping = false; + char quote = 0; + boolean literalQuote = false; + + for (int i = 0; i < input.length(); ++i) { + char c = input.charAt(i); + + if (escaping) { + appendEscaped(token, c, preserveEscapes); + inToken = true; + escaping = false; + continue; + } + + if (c == '\\') { + escaping = true; + inToken = true; + continue; + } + + if (quote != 0) { + if (c == quote) { + if (literalQuote) { + token.append(c); + } + quote = 0; + literalQuote = false; + } else { + appendLiteral(token, c, preserveEscapes); + } + inToken = true; + continue; + } + + if (c == '\'' || c == '"') { + literalQuote = isLiteralQuote(token, preserveEscapes); + if (literalQuote) { + token.append(c); + } + quote = c; + inToken = true; + continue; + } + + if (Character.isWhitespace(c)) { + if (inToken) { + result.add(token.toString()); + token.setLength(0); + inToken = false; + } + continue; + } + + token.append(c); + inToken = true; + } + + if (escaping) { + throw new IllegalArgumentException("Dangling escape in command line: " + input); + } + if (quote != 0) { + throw new IllegalArgumentException("Unclosed quote in command line: " + input); + } + if (inToken) { + result.add(token.toString()); + } + + return result; + } + + private static void appendEscaped(StringBuilder token, char c, boolean preserveEscapes) { + if (preserveEscapes) { + token.append('\\'); + token.append(c); + } else if (isEscapable(c)) { + token.append(c); + } else { + token.append('\\'); + token.append(c); + } + } + + private static void appendLiteral(StringBuilder token, char c, boolean preserveEscapes) { + if (preserveEscapes && Character.isWhitespace(c)) { + token.append('\\'); + token.append(c); + } else { + token.append(c); + } + } + + private static boolean isEscapable(char c) { + return Character.isWhitespace(c) || c == '\\' || c == '\'' || c == '"'; + } + + private static boolean isLiteralQuote(StringBuilder token, boolean preserveEscapes) { + if (preserveEscapes) { + return false; + } + + String currentToken = token.toString(); + if (currentToken.startsWith("-D") && currentToken.indexOf("=") != -1) { + return true; + } + + int assignmentIndex = currentToken.indexOf("="); + return assignmentIndex != -1 && assignmentIndex < currentToken.length() - 1; + } +} diff --git a/server/src/main/java/com/defold/extender/process/ProcessExecutor.java b/server/src/main/java/com/defold/extender/process/ProcessExecutor.java index c91cd5e4..6313ce28 100644 --- a/server/src/main/java/com/defold/extender/process/ProcessExecutor.java +++ b/server/src/main/java/com/defold/extender/process/ProcessExecutor.java @@ -7,7 +7,6 @@ import java.io.StringWriter; import java.io.PrintWriter; import java.util.*; -import java.util.stream.Collectors; import com.defold.extender.ExtenderException; @@ -23,9 +22,7 @@ public class ProcessExecutor { public int execute(String command) throws IOException, InterruptedException { // To avoid an issue where an extra space was interpreted as an argument - List args = Arrays.stream(command.split(" ")) - .filter(s -> !s.isEmpty()) - .collect(Collectors.toList()); + List args = CommandLineTokenizer.parse(command); return execute(args); } diff --git a/server/src/main/java/com/defold/extender/services/cocoapods/PodBuildSpec.java b/server/src/main/java/com/defold/extender/services/cocoapods/PodBuildSpec.java index 1b880768..2e27cfce 100644 --- a/server/src/main/java/com/defold/extender/services/cocoapods/PodBuildSpec.java +++ b/server/src/main/java/com/defold/extender/services/cocoapods/PodBuildSpec.java @@ -4,7 +4,6 @@ import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -14,7 +13,8 @@ import java.util.Map; import java.util.Set; -import org.apache.commons.text.StringEscapeUtils; +import com.defold.extender.process.CommandLineTokenizer; + // similar to PodSpec but contains some runtime information that used during the build public class PodBuildSpec { @@ -132,7 +132,7 @@ void updateFlagsFromConfig(Map parsedConfig) { // defines List defs = argumentsAsList(parsedConfig.getOrDefault("GCC_PREPROCESSOR_DEFINITIONS", null)); if (defs != null) { - this.defines.addAll(unescapeStrings(defs)); + this.defines.addAll(defs); } // linker flags // https://xcodebuildsettings.com/#other_ldflags @@ -285,14 +285,6 @@ static boolean hasString(Map parsedConfig, String key) { return value != null && !value.trim().isEmpty(); } - static List unescapeStrings(List strings) { - List unescapedStrings = new ArrayList<>(); - for (String s : strings) { - unescapedStrings.add(StringEscapeUtils.unescapeJava(s)); - } - return unescapedStrings; - } - // check if the value for a specific key matches an expected value static boolean compareString(Map config, String key, String expected) { String value = config.get(key); @@ -307,7 +299,7 @@ static List argumentsAsList(String arguments) { if (arguments == null || arguments.isEmpty()) { return null; } - return new ArrayList<>(Arrays.asList(arguments.split(" "))); + return new ArrayList<>(CommandLineTokenizer.splitPreservingEscapedWhitespace(arguments)); } /** diff --git a/server/src/main/java/com/defold/extender/services/cocoapods/XCConfigParser.java b/server/src/main/java/com/defold/extender/services/cocoapods/XCConfigParser.java index ec471630..5c67e79b 100644 --- a/server/src/main/java/com/defold/extender/services/cocoapods/XCConfigParser.java +++ b/server/src/main/java/com/defold/extender/services/cocoapods/XCConfigParser.java @@ -4,7 +4,6 @@ import java.io.IOException; import java.nio.file.Files; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -18,9 +17,11 @@ import org.slf4j.LoggerFactory; import com.defold.extender.ExtenderBuildState; +import com.defold.extender.process.CommandLineTokenizer; public class XCConfigParser implements IConfigParser { private static final Logger LOGGER = LoggerFactory.getLogger(XCConfigParser.class); + private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\$[\\(|{]([\\w]+)[\\)|}]"); private File buildDir; private File podsDir; private String platform; @@ -74,35 +75,86 @@ void parseIncludes(String line) { * Merged values from base values (like directory paths) and values obtained from xcconfig */ String postProcessValue(String value, Map allValues) { - List tmpList = new ArrayList<>(Arrays.asList(value.split(" "))); - tmpList.remove("$(inherited)"); - - // check for $(....) pattern - Pattern p = Pattern.compile("\\$[\\(|{]([\\w]+)[\\)|}]"); - - // substitute values if any placeholders are presented - for (int idx = 0 ; idx < tmpList.size(); ++idx) { - Set visitedKeys = new HashSet<>(); - String element = tmpList.get(idx); - Matcher matcher = p.matcher(element); - while (matcher.find()) { - String replaceKey = matcher.group(1); - String replaceValue = allValues.containsKey(replaceKey) && !visitedKeys.contains(replaceKey) ? allValues.get(replaceKey) : null; - visitedKeys.add(replaceKey); - if (replaceValue != null) { - element = element.replace(matcher.group(0), replaceValue); - element = element.replaceAll("\"", ""); - // update matcher every time because during replace new values for substitution can be introduced. - // For example: ${PODS_ROOT}/Headers (where PODS_ROOT=${SRCROOT}) -> ${SRCROOT}/Headers - matcher = p.matcher(element); - } else { - LOGGER.warn("Can't find value for substitution for key {}", replaceKey); + return String.join(" ", postProcessTokens(value, allValues, new HashSet<>())); + } + + private List postProcessTokens(String value, Map allValues, Set visitedKeys) { + List result = new ArrayList<>(); + List tmpList = new ArrayList<>(CommandLineTokenizer.splitPreservingEscapedWhitespace(value)); + + for (String token : tmpList) { + if ("$(inherited)".equals(token)) { + continue; + } + result.addAll(postProcessToken(token, allValues, visitedKeys)); + } + + return result; + } + + private List postProcessToken(String token, Map allValues, Set visitedKeys) { + Matcher matcher = VARIABLE_PATTERN.matcher(token); + if (matcher.matches()) { + String replaceKey = matcher.group(1); + if (visitedKeys.contains(replaceKey)) { + return List.of(CommandLineTokenizer.escapeWhitespace(token)); + } + + String replaceValue = allValues.get(replaceKey); + if (replaceValue != null) { + Set nextVisitedKeys = new HashSet<>(visitedKeys); + nextVisitedKeys.add(replaceKey); + if (isListBuildSetting(replaceKey)) { + return postProcessTokens(replaceValue, allValues, nextVisitedKeys); } + return List.of(postProcessScalarValue(replaceValue, allValues, nextVisitedKeys)); + } else { + LOGGER.warn("Can't find value for substitution for key {}", replaceKey); + return List.of(CommandLineTokenizer.escapeWhitespace(token)); + } + } + + return List.of(postProcessSingleToken(token, allValues, visitedKeys)); + } + + private boolean isListBuildSetting(String key) { + return key.endsWith("FLAGS") + || key.endsWith("PATHS") + || key.endsWith("DEFINITIONS") + || key.endsWith("ARCHS") + || key.endsWith("LIBRARIES"); + } + + private String postProcessScalarValue(String value, Map allValues, Set visitedKeys) { + String scalarValue = String.join(" ", CommandLineTokenizer.splitPreservingEscapedWhitespace(value)); + return postProcessSingleToken(scalarValue, allValues, visitedKeys); + } + + private String postProcessSingleToken(String token, Map allValues, Set visitedKeys) { + String element = token; + Set localVisitedKeys = new HashSet<>(visitedKeys); + Matcher matcher = VARIABLE_PATTERN.matcher(element); + while (matcher.find()) { + String replaceKey = matcher.group(1); + if (localVisitedKeys.contains(replaceKey)) { + continue; + } + + String replaceValue = allValues.get(replaceKey); + localVisitedKeys.add(replaceKey); + if (replaceValue != null) { + String resolvedValue = String.join(" ", postProcessTokens(replaceValue, allValues, localVisitedKeys)); + element = element.replace(matcher.group(0), resolvedValue); + element = element.replaceAll("(? ${SRCROOT}/Headers + matcher = VARIABLE_PATTERN.matcher(element); + } else { + LOGGER.warn("Can't find value for substitution for key {}", replaceKey); } - tmpList.set(idx, element); } - return String.join(" ", tmpList); + return CommandLineTokenizer.escapeWhitespace(element); } Pair parseLine(String line) { @@ -125,8 +177,6 @@ Pair parseLine(String line) { parseIncludes(line); return null; } - // replace " to avoid problem with arguments in ProcessBuilder - line = line.replaceAll("(? parseLine(String line) { break; } } - return Pair.of(varBuilder.toString(), valueBuilder.toString()); + return Pair.of(varBuilder.toString(), normalizeParsedValue(valueBuilder.toString())); + } + + private String normalizeParsedValue(String value) { + if (value.length() >= 2) { + char quote = value.charAt(0); + if ((quote == '\'' || quote == '"') && value.charAt(value.length() - 1) == quote) { + String quotedValue = value.substring(1, value.length() - 1); + if (quotedValue.chars().noneMatch(Character::isWhitespace)) { + return quotedValue; + } + } + } + + return value; } @Override diff --git a/server/src/test/java/com/defold/extender/process/CommandLineTokenizerTest.java b/server/src/test/java/com/defold/extender/process/CommandLineTokenizerTest.java new file mode 100644 index 00000000..a0990825 --- /dev/null +++ b/server/src/test/java/com/defold/extender/process/CommandLineTokenizerTest.java @@ -0,0 +1,109 @@ +package com.defold.extender.process; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertIterableEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.List; + +import org.junit.jupiter.api.Test; + +public class CommandLineTokenizerTest { + @Test + public void testParseIgnoresRepeatedWhitespace() { + assertIterableEquals( + List.of("clang", "-c", "file.m"), + CommandLineTokenizer.parse("clang -c\tfile.m") + ); + } + + @Test + public void testParseEscapedWhitespace() { + assertIterableEquals( + List.of("clang", "-DCLS_SDK_NAME=Crashlytics SDK iOS", "file.m"), + CommandLineTokenizer.parse("clang -DCLS_SDK_NAME=Crashlytics\\ SDK\\ iOS file.m") + ); + } + + @Test + public void testParseQuotes() { + assertIterableEquals( + List.of("clang", "path with spaces/file.m"), + CommandLineTokenizer.parse("clang \"path with spaces/file.m\"") + ); + } + + @Test + public void testParsePrefixQuotes() { + assertIterableEquals( + List.of("clang", "-Ipath with spaces"), + CommandLineTokenizer.parse("clang -I\"path with spaces\"") + ); + } + + @Test + public void testParseQuotedDefineValue() { + assertIterableEquals( + List.of("clang", "-DDLIB_LOG_DOMAIN=\"FIREBASEEXT\""), + CommandLineTokenizer.parse("clang -DDLIB_LOG_DOMAIN=\"FIREBASEEXT\"") + ); + } + + @Test + public void testParseQuotedDefineValueWithWhitespace() { + assertIterableEquals( + List.of("clang", "-DCLS_SDK_NAME=\"Crashlytics SDK iOS\""), + CommandLineTokenizer.parse("clang -DCLS_SDK_NAME=\"Crashlytics SDK iOS\"") + ); + } + + @Test + public void testParseEscapedQuotes() { + assertIterableEquals( + List.of("clang", "-DNAME=\"Crashlytics\""), + CommandLineTokenizer.parse("clang -DNAME=\\\"Crashlytics\\\"") + ); + } + + @Test + public void testParsePreservesQuotedValuesInsideAssignmentLists() { + assertIterableEquals( + List.of("em++", "-s", "EXPORTED_RUNTIME_METHODS=[\"ccall\",\"callMain\",\"UTF8ToString\"]"), + CommandLineTokenizer.parse("em++ -s EXPORTED_RUNTIME_METHODS=[\"ccall\",\"callMain\",\"UTF8ToString\"]") + ); + } + + @Test + public void testPreserveEscapedWhitespace() { + assertIterableEquals( + List.of("CLS_SDK_NAME=Crashlytics\\ SDK\\ iOS"), + CommandLineTokenizer.splitPreservingEscapedWhitespace("CLS_SDK_NAME=Crashlytics\\ SDK\\ iOS") + ); + } + + @Test + public void testPreserveQuotedWhitespaceAsEscapedWhitespace() { + assertIterableEquals( + List.of("path\\ with\\ spaces/file.m"), + CommandLineTokenizer.splitPreservingEscapedWhitespace("\"path with spaces/file.m\"") + ); + } + + @Test + public void testEscapeWhitespacePreservesExistingEscapes() { + assertEquals( + "Crashlytics\\ SDK\\ iOS", + CommandLineTokenizer.escapeWhitespace("Crashlytics\\ SDK iOS") + ); + } + + @Test + public void testDanglingEscapeThrows() { + assertThrows(IllegalArgumentException.class, () -> CommandLineTokenizer.parse("clang file\\")); + } + + @Test + public void testUnclosedQuoteThrows() { + assertThrows(IllegalArgumentException.class, () -> CommandLineTokenizer.parse("clang \"file")); + } +} diff --git a/server/src/test/java/com/defold/extender/services/cocoapods/PodBuildSpecTest.java b/server/src/test/java/com/defold/extender/services/cocoapods/PodBuildSpecTest.java index 4300c691..1e1dee0a 100644 --- a/server/src/test/java/com/defold/extender/services/cocoapods/PodBuildSpecTest.java +++ b/server/src/test/java/com/defold/extender/services/cocoapods/PodBuildSpecTest.java @@ -1,6 +1,7 @@ package com.defold.extender.services.cocoapods; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; @@ -65,6 +66,36 @@ private void deleteRecursively(Path path) throws IOException { }); } + @Test + public void testPreprocessorDefinitionsPreserveEscapedSpaces() { + PodBuildSpec buildSpec = new PodBuildSpec(); + + buildSpec.updateFlagsFromConfig(Map.of( + "GCC_PREPROCESSOR_DEFINITIONS", + "COCOAPODS=1 CLS_SDK_NAME=Crashlytics\\ SDK\\ iOS PB_ENABLE_MALLOC=1" + )); + + assertTrue(buildSpec.defines.contains("COCOAPODS=1")); + assertTrue(buildSpec.defines.contains("CLS_SDK_NAME=Crashlytics\\ SDK\\ iOS")); + assertTrue(buildSpec.defines.contains("PB_ENABLE_MALLOC=1")); + assertFalse(buildSpec.defines.contains("SDK")); + assertFalse(buildSpec.defines.contains("iOS")); + } + + @Test + public void testPreprocessorDefinitionsEscapeQuotedSpaces() { + PodBuildSpec buildSpec = new PodBuildSpec(); + + buildSpec.updateFlagsFromConfig(Map.of( + "GCC_PREPROCESSOR_DEFINITIONS", + "CLS_SDK_NAME=\"Crashlytics SDK iOS\"" + )); + + assertTrue(buildSpec.defines.contains("CLS_SDK_NAME=Crashlytics\\ SDK\\ iOS")); + assertFalse(buildSpec.defines.contains("SDK")); + assertFalse(buildSpec.defines.contains("iOS")); + } + @Test public void testValueSubstitution() throws IOException, ExtenderException { String jsonSpec = Files.readString(Path.of("test-data/pod_specs/Sentry.json")); diff --git a/server/src/test/java/com/defold/extender/services/cocoapods/XCConfigParserTest.java b/server/src/test/java/com/defold/extender/services/cocoapods/XCConfigParserTest.java index ce0a55ac..8e6663ad 100644 --- a/server/src/test/java/com/defold/extender/services/cocoapods/XCConfigParserTest.java +++ b/server/src/test/java/com/defold/extender/services/cocoapods/XCConfigParserTest.java @@ -44,7 +44,9 @@ private Map createMockBaseVars() { "PODS_BUILD_DIR", "/var/tmp/tmp-dir/pod-build-dir", "CONFIGURATION", "debug", "EFFECTIVE_PLATFORM_NAME","iphoneos", - "TOOLCHAIN_DIR", "/opt/platformsdk/XcodeDefaults16.0.toolchain" + "TOOLCHAIN_DIR", "/opt/platformsdk/XcodeDefaults16.0.toolchain", + "BASE_LDFLAGS", "-framework Foo -framework Bar", + "BASE_PATH", "/Users/test-pod/Build Products" ); } @@ -54,6 +56,7 @@ private static Stream lineData() { Arguments.of("\t ARG=1 // comment end line", Pair.of("ARG", "1")), Arguments.of("LINE_WITHSEMICOLON=value1 value_w2;;;;", Pair.of("LINE_WITHSEMICOLON", "value1 value_w2")), Arguments.of("\t\t\t \t ARG\t\t = \t\t\t\t \t\t\t\t \"some_values\"", Pair.of("ARG", "some_values")), + Arguments.of("QUOTED_VALUE_WITH_SPACES = \"some values\"", Pair.of("QUOTED_VALUE_WITH_SPACES", "\"some values\"")), Arguments.of("PODS_VARIANT1[sdk=*][version=13.0] = value1 value2", Pair.of("PODS_VARIANT1", "value1 value2")), Arguments.of("PODS_VARIANT2[sdk=iphone, version=11.0, configuration=debug]= single value", Pair.of("PODS_VARIANT2", "single value")) ); @@ -65,6 +68,10 @@ private static Stream postProcessData() { Arguments.of("VALUE_SUBSTITUTION1=$(inherited) ${PODS_ROOT}/Headers/Private ${PODS_ROOT}/Headers/Private/KSCrash ${PODS_ROOT}/Headers/Public ${PODS_ROOT}/Headers/Public/KSCrash", "/Users/test-pod/Pods/Headers/Private /Users/test-pod/Pods/Headers/Private/KSCrash /Users/test-pod/Pods/Headers/Public /Users/test-pod/Pods/Headers/Public/KSCrash"), Arguments.of("VALUE_SUBSTITUTION2=${PODS_BUILD_DIR}/$(CONFIGURATION)/$(EFFECTIVE_PLATFORM_NAME)", "/var/tmp/tmp-dir/pod-build-dir/debug/iphoneos"), Arguments.of("VALUE_SUBSTITUTION3=${PODS_BUILD_DIR}/${CONFIGURATION}/$(EFFECTIVE_PLATFORM_NAME)", "/var/tmp/tmp-dir/pod-build-dir/debug/iphoneos"), + Arguments.of("GCC_PREPROCESSOR_DEFINITIONS = $(inherited) COCOAPODS=1 CLS_SDK_NAME=Crashlytics\\ SDK\\ iOS PB_ENABLE_MALLOC=1", "COCOAPODS=1 CLS_SDK_NAME=Crashlytics\\ SDK\\ iOS PB_ENABLE_MALLOC=1"), + Arguments.of("QUOTED_VALUE=\"${PODS_ROOT}/Firebase Crashlytics\" tail", "/Users/test-pod/Pods/Firebase\\ Crashlytics tail"), + Arguments.of("OTHER_LDFLAGS=$(BASE_LDFLAGS) -ObjC", "-framework Foo -framework Bar -ObjC"), + Arguments.of("SCALAR_PATH=$(BASE_PATH)", "/Users/test-pod/Build\\ Products"), Arguments.of("POD_VERSION=$(POD_VERSION)", "$(POD_VERSION)"), Arguments.of("POD_VERSION=${POD_VERSION}", "${POD_VERSION}") );