Codeql improvements

client/src/main/java/com/defold/extender/client/ExtenderClientCache.java

@@ -35,10 +35,10 @@ public ExtenderClientCache(File cacheDir) throws IOException {
      */
     public String getHash(ExtenderResource extenderResource) throws ExtenderClientException {
         String path = extenderResource.getPath();
-        Long fileTimestamp = extenderResource.getLastModified();
+        long fileTimestamp = extenderResource.getLastModified();
         Long timestamp = this.timestamps.get(path);
 
-        if (timestamp != null && fileTimestamp.equals(timestamp) ) {
+        if (timestamp != null && timestamp.longValue() == fileTimestamp) {
             String hash = this.hashes.get(path);
             if (hash != null) {
                 return hash;
@@ -180,7 +180,7 @@ private static String hash(ExtenderResource extenderResource) throws ExtenderCli
             md.update(data);
             return hashToString(md.digest());
         } catch(Exception e){
-            throw new ExtenderClientException(String.format("Failed to hash resource: ", extenderResource.getPath()), e);
+            throw new ExtenderClientException(String.format("Failed to hash resource: %s", extenderResource.getPath()), e);
         }
     }
 

server/manifestmergetool/src/main/java/com/defold/manifestmergetool/ManifestMergeTool.java

@@ -92,48 +92,73 @@ public static void merge(Platform platform, File main, File output, List<File> l
         logger.log(Level.FINE, "Merging done");
     }
 
-    /**
-     * Merges a main manifest with several stubs
-     */
-    public static void main(String[] args) throws Exception {
-
+    static class ParsedArgs {
+        Platform platform = Platform.UNKNOWN;
         File main = null;
         File output = null;
         List<File> libraries = new ArrayList<>();
+    }
 
-        Platform platform = Platform.UNKNOWN;
+    static ParsedArgs parseArgs(String[] args) {
+        if ((args.length % 2) != 0) {
+            throw new IllegalArgumentException("Expected key/value argument pairs, got odd number of arguments: " + args.length);
+        }
 
-        int index = 0;
-        for (int i = 0; i < args.length; ++i) {
-            if (args[i].equals("--main") && (index+1) < args.length) {
-                main = new File(args[++i]);
-            }
-            else if (args[i].equals("--out") && (index+1) < args.length) {
-                output = new File(args[++i]);
-            }
-            else if (args[i].equals("--lib") && (index+1) < args.length) {
-                libraries.add(new File(args[++i]));
+        ParsedArgs parsed = new ParsedArgs();
+        for (int i = 0; i < args.length; i += 2) {
+            String key = args[i];
+            String value = args[i + 1];
+            switch (key) {
+                case "--main":
+                    parsed.main = new File(value);
+                    break;
+                case "--out":
+                    parsed.output = new File(value);
+                    break;
+                case "--lib":
+                    parsed.libraries.add(new File(value));
+                    break;
+                case "--platform":
+                    switch (value) {
+                        case "android":
+                            parsed.platform = Platform.ANDROID;
+                            break;
+                        case "ios":
+                            parsed.platform = Platform.IOS;
+                            break;
+                        case "osx":
+                            parsed.platform = Platform.OSX;
+                            break;
+                        case "web":
+                            parsed.platform = Platform.WEB;
+                            break;
+                        default:
+                            throw new IllegalArgumentException(String.format("Unsupported platform: %s", value));
+                    }
+                    break;
+                default:
+                    throw new IllegalArgumentException(String.format("Unknown argument: %s", key));
             }
+        }
+        return parsed;
+    }
 
-            if (args[i].equals("--platform") && (index+1) < args.length) {
-                ++i;
-                if (args[i].equals("android")) {
-                    platform = Platform.ANDROID;
-                } else if (args[i].equals("ios")) {
-                    platform = Platform.IOS;
-                } else if (args[i].equals("osx")) {
-                    platform = Platform.OSX;
-                } else if (args[i].equals("web")) {
-                    platform = Platform.WEB;
-                } else {
-                    ManifestMergeTool.logger.log(Level.SEVERE, String.format("Unsupported platform: %s", args[i]));
-                    System.exit(1);
-                }
-            }
+    /**
+     * Merges a main manifest with several stubs
+     */
+    public static void main(String[] args) throws Exception {
+
+        ParsedArgs parsed;
+        try {
+            parsed = parseArgs(args);
+        } catch (IllegalArgumentException e) {
+            ManifestMergeTool.logger.log(Level.SEVERE, e.getMessage());
+            System.exit(1);
+            return;
         }
 
         try {
-            merge(platform, main, output, libraries);
+            merge(parsed.platform, parsed.main, parsed.output, parsed.libraries);
         } catch(Exception e) {
             ManifestMergeTool.logger.log(Level.SEVERE, e.toString());
             System.exit(1);

server/manifestmergetool/src/test/java/com/defold/manifestmergetool/ManifestMergeToolTest.java

@@ -9,6 +9,8 @@
 import java.nio.file.Files;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.apache.commons.io.FileUtils;
@@ -850,4 +852,89 @@ public void testMergeHTML5() throws IOException {
 
         assertEquals(expected, merged);
     }
+
+    @Test
+    public void testParseArgsEmpty() {
+        ManifestMergeTool.ParsedArgs parsed = ManifestMergeTool.parseArgs(new String[]{});
+        assertEquals(Platform.UNKNOWN, parsed.platform);
+        assertNull(parsed.main);
+        assertNull(parsed.output);
+        assertTrue(parsed.libraries.isEmpty());
+    }
+
+    @Test
+    public void testParseArgsAllFlags() {
+        String[] args = {
+            "--platform", "android",
+            "--main", "AndroidManifest.xml",
+            "--lib", "lib1.xml",
+            "--lib", "lib2.xml",
+            "--out", "merged.xml",
+        };
+        ManifestMergeTool.ParsedArgs parsed = ManifestMergeTool.parseArgs(args);
+        assertEquals(Platform.ANDROID, parsed.platform);
+        assertEquals(new File("AndroidManifest.xml"), parsed.main);
+        assertEquals(new File("merged.xml"), parsed.output);
+        assertEquals(2, parsed.libraries.size());
+        assertEquals(new File("lib1.xml"), parsed.libraries.get(0));
+        assertEquals(new File("lib2.xml"), parsed.libraries.get(1));
+    }
+
+    @Test
+    public void testParseArgsPlatformIos() {
+        ManifestMergeTool.ParsedArgs parsed = ManifestMergeTool.parseArgs(new String[]{"--platform", "ios"});
+        assertEquals(Platform.IOS, parsed.platform);
+    }
+
+    @Test
+    public void testParseArgsPlatformOsx() {
+        ManifestMergeTool.ParsedArgs parsed = ManifestMergeTool.parseArgs(new String[]{"--platform", "osx"});
+        assertEquals(Platform.OSX, parsed.platform);
+    }
+
+    @Test
+    public void testParseArgsPlatformWeb() {
+        ManifestMergeTool.ParsedArgs parsed = ManifestMergeTool.parseArgs(new String[]{"--platform", "web"});
+        assertEquals(Platform.WEB, parsed.platform);
+    }
+
+    @Test
+    public void testParseArgsLibrariesOrderPreserved() {
+        String[] args = {
+            "--lib", "a.xml",
+            "--lib", "b.xml",
+            "--lib", "c.xml",
+        };
+        ManifestMergeTool.ParsedArgs parsed = ManifestMergeTool.parseArgs(args);
+        assertEquals(3, parsed.libraries.size());
+        assertEquals(new File("a.xml"), parsed.libraries.get(0));
+        assertEquals(new File("b.xml"), parsed.libraries.get(1));
+        assertEquals(new File("c.xml"), parsed.libraries.get(2));
+    }
+
+    @Test
+    public void testParseArgsUnsupportedPlatform() {
+        IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
+                () -> ManifestMergeTool.parseArgs(new String[]{"--platform", "windows"}));
+        assertTrue(e.getMessage().contains("windows"));
+    }
+
+    @Test
+    public void testParseArgsUnknownFlag() {
+        IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
+                () -> ManifestMergeTool.parseArgs(new String[]{"--bogus", "value"}));
+        assertTrue(e.getMessage().contains("--bogus"));
+    }
+
+    @Test
+    public void testParseArgsOddNumberOfArgs() {
+        assertThrows(IllegalArgumentException.class,
+                () -> ManifestMergeTool.parseArgs(new String[]{"--main"}));
+    }
+
+    @Test
+    public void testParseArgsOddNumberOfArgsTrailing() {
+        assertThrows(IllegalArgumentException.class,
+                () -> ManifestMergeTool.parseArgs(new String[]{"--main", "a.xml", "--lib"}));
+    }
 }

server/src/main/java/com/defold/extender/ExtenderController.java

@@ -437,6 +437,6 @@ private RemoteInstanceConfig getRemoteBuilderConfig(String platform, String plat
         } else if (this.remoteBuilderPlatformMappings.containsKey(fallbackKey)) {
             return this.remoteBuilderPlatformMappings.get(fallbackKey);
         }
-        throw new ExtenderException(String.format("No suitable remote builder found for %", fullKey));
+        throw new ExtenderException(String.format("No suitable remote builder found for %s", fullKey));
     }
 }

server/src/main/java/com/defold/extender/process/ProcessExecutor.java

@@ -46,7 +46,7 @@ public int execute(List<String> args) throws IOException, InterruptedException {
         if (DM_DEBUG_COMMANDS) {
             StringBuffer debugBuffer = new StringBuffer();
             debugBuffer.append(String.format("CMD %d: %s\n", commandId, String.join(" ", args)));
-            debugBuffer.append(String.format("\tWorking dir: \n", this.cwd == null ? "(null)" : this.cwd.toString()));
+            debugBuffer.append(String.format("\tWorking dir: %s\n", this.cwd == null ? "(null)" : this.cwd.toString()));
             debugBuffer.append("\tEnvironment:\n");
             for (Map.Entry<String, String> envEntry : this.env.entrySet()) {
                 debugBuffer.append(String.format("\t%s=%s\n", envEntry.getKey(), envEntry.getValue()));

server/src/main/java/com/defold/extender/services/cocoapods/CocoaPodsService.java

@@ -65,6 +65,7 @@ private class InstalledPods {
     private static final Logger LOGGER = LoggerFactory.getLogger(CocoaPodsService.class);
     private static final String CURRENT_CACHE_DIR_FILE = "current_pod_cache.txt";
     private static final String OLD_CACHE_DIR_FILE = "old_pod_caches.txt";
+    private final Object syncLock = new Object();
     private final TemplateExecutor templateExecutor = new TemplateExecutor();
 
     private final String podfileTemplateContents;
@@ -88,13 +89,13 @@ public void runAfterStartup() {
         // initialize cache directory
         Path currentCacheDir = readCurrentCacheDir();
         if (currentCacheDir != null && currentCacheDir.startsWith(this.homeDirPrefix)) {
-            synchronized(this.currentCacheDir) {
+            synchronized(this.syncLock) {
                 this.currentCacheDir = currentCacheDir;
             }
             updateSpecRepo();
         } else {
             LOGGER.info("Cocoapods has no current cache dir or prefix is changed. Created...");
-            synchronized(this.currentCacheDir) {
+            synchronized(this.syncLock) {
                 this.currentCacheDir = generateCacheDirPath();
                 storeCurrentCacheDir(this.currentCacheDir);
             }
@@ -216,7 +217,7 @@ private InstalledPods installPods(ExtenderBuildState buildState, CocoaPodsServic
         LOGGER.info("Installing pods");
         Path cacheDir;
         // store current cache dir into local variable to use the same value for all 'pod' runs
-        synchronized(currentCacheDir) {
+        synchronized(syncLock) {
             cacheDir = currentCacheDir;
         }
         InstalledPods installedPods = new InstalledPods();
@@ -501,7 +502,7 @@ private void storeCurrentCacheDir(Path currentCacheDir) {
     private void initializeTrunkRepo() {
         try {
             Path cacheDir;
-            synchronized(currentCacheDir) {
+            synchronized(syncLock) {
                 cacheDir = currentCacheDir;
             }
             String log = ProcessUtils.execCommand(List.of(
@@ -524,7 +525,7 @@ public void rotatePodCacheDirectory() {
         LOGGER.info("Rotate pod cache directory");
         Path newCacheDir = generateCacheDirPath();
         Path cacheDir;
-        synchronized(this.currentCacheDir) {
+        synchronized(this.syncLock) {
             cacheDir = this.currentCacheDir;
         }
         try {
@@ -541,7 +542,7 @@ public void rotatePodCacheDirectory() {
         } catch(IOException exc) {
             LOGGER.warn("Error while writing to old cache paths file", exc);
         }
-        synchronized(this.currentCacheDir) {
+        synchronized(this.syncLock) {
             this.currentCacheDir = newCacheDir;
             storeCurrentCacheDir(currentCacheDir);
         }
@@ -578,7 +579,7 @@ public void updateSpecRepo() {
         try {
             LOGGER.info("Run pod spec update");
             Path cacheDir;
-            synchronized(currentCacheDir) {
+            synchronized(this.syncLock) {
                 cacheDir = currentCacheDir;
             }
             String log = ProcessUtils.execCommand(List.of(

server/src/main/java/com/defold/extender/services/cocoapods/PodfileParser.java

@@ -31,6 +31,7 @@ public ParseResult mergeWith(ParseResult other) throws PodfileParsingException {
                 minVersion = other.minVersion;
             }
 
+
             if (platform == null) {
                 platform = other.platform;
             } else if (other.platform != null && !platform.equals(other.platform)) {
@@ -45,19 +46,40 @@ public ParseResult mergeWith(ParseResult other) throws PodfileParsingException {
         }
     }
 
+    // Strip semver pre-release and build metadata (everything from the first '-' or '+'),
+    // e.g. "1.0.0-beta" -> "1.0.0", "2.0.0+build.7" -> "2.0.0". Comparison ignores these segments.
+    private static String stripVersionSuffix(String version) {
+        int cut = version.length();
+        for (int i = 0; i < version.length(); i++) {
+            char c = version.charAt(i);
+            if (c == '-' || c == '+') {
+                cut = i;
+                break;
+            }
+        }
+        return version.substring(0, cut);
+    }
+
     // https://www.baeldung.com/java-comparing-versions#customSolution
-    static int compareVersions(String version1, String version2) {
+    static int compareVersions(String version1, String version2) throws PodfileParsingException {
         int result = 0;
-        String[] parts1 = version1.split("\\.");
-        String[] parts2 = version2.split("\\.");
+        String[] parts1 = stripVersionSuffix(version1).split("\\.");
+        String[] parts2 = stripVersionSuffix(version2).split("\\.");
         int length = Math.max(parts1.length, parts2.length);
         for (int i = 0; i < length; i++) {
-            Integer v1 = i < parts1.length ? Integer.parseInt(parts1[i]) : 0;
-            Integer v2 = i < parts2.length ? Integer.parseInt(parts2[i]) : 0;
-            int compare = v1.compareTo(v2);
-            if (compare != 0) {
-                result = compare;
-                break;
+            try {
+                int v1 = i < parts1.length && !parts1[i].isEmpty() ? Integer.parseInt(parts1[i]) : 0;
+                int v2 = i < parts2.length && !parts2[i].isEmpty() ? Integer.parseInt(parts2[i]) : 0;
+                if (v1 < v2) {
+                    result = -1;
+                    break;
+                } else if (v1 > v2) {
+                    result = 1;
+                    break;
+                }
+        } catch (NumberFormatException exc) {
+                throw new PodfileParsingException(
+                    String.format("Failed to compare pod versions '%s' and '%s'", version1, version2), exc);
             }
         }
         return result;

server/src/main/java/com/defold/extender/services/cocoapods/PodfileParsingException.java

@@ -6,4 +6,8 @@ public class PodfileParsingException extends ExtenderException {
     public PodfileParsingException(String reason) {
         super(reason);
     }
+
+    public PodfileParsingException(String reason, Exception cause) {
+        super(cause, reason);
+    }
 }

server/src/main/java/com/defold/extender/services/cocoapods/XCConfigParser.java

@@ -144,8 +144,6 @@ Pair<String, String> parseLine(String line) {
                         || (c >= 'A' && c <= 'Z')
                         || c == '_') {
                             varBuilder.append(c);
-                    } else if (c == '[') {
-                        currentMode = ParseMode.FLAVOUR_START;
                     } else if (c == '=') {
                         currentMode = ParseMode.ASSIGMENT_OPERATOR;
                     } else {