diff --git a/server/src/main/java/com/defold/extender/Extender.java b/server/src/main/java/com/defold/extender/Extender.java index 9322d7d3..0c54e198 100644 --- a/server/src/main/java/com/defold/extender/Extender.java +++ b/server/src/main/java/com/defold/extender/Extender.java @@ -2199,6 +2199,9 @@ private File[] buildClassesDex(List jars, File mainDexList) throws Exten context.put("jars", jars); context.put("engineJars", empty_list); context.put("mainDexList", mainDexList.getAbsolutePath()); + // Always bound, also for engines that don't send it, so that a build.yml referencing + // '--min-api {{minAndroidSdkVersion}}' can never render the flag without its argument. + context.put("minAndroidSdkVersion", buildState.getMinAndroidSdkVersion()); // replace parameter name because '--main-dex-list' is deprecated and reported as error // we can't change command format for older version of engine so replace parameter here. diff --git a/server/src/main/java/com/defold/extender/ExtenderBuildState.java b/server/src/main/java/com/defold/extender/ExtenderBuildState.java index bba87529..a7926261 100644 --- a/server/src/main/java/com/defold/extender/ExtenderBuildState.java +++ b/server/src/main/java/com/defold/extender/ExtenderBuildState.java @@ -8,6 +8,9 @@ public class ExtenderBuildState { static final String APPMANIFEST_BUILD_ARTIFACTS_KEYWORD = "buildArtifacts"; static final String APPMANIFEST_JETIFIER_KEYWORD = "jetifier"; static final String APPMANIFEST_DEBUG_SOURCE_PATH = "debugSourcePath"; + static final String APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD = "minAndroidSdkVersion"; + + static final int DEFAULT_MIN_ANDROID_SDK_VERSION = 21; File jobDir; File uploadDir; @@ -19,6 +22,7 @@ public class ExtenderBuildState { String hostPlatform; private final String buildArtifacts; private final String debugSourcePath; + private final int minAndroidSdkVersion; private final Boolean withSymbols; private final Boolean useJetifier; @@ -37,6 +41,7 @@ public class ExtenderBuildState { this.withSymbols = ExtenderUtil.getAppManifestContextBoolean(appManifest, APPMANIFEST_WITH_SYMBOLS_KEYWORD, true); this.buildArtifacts = ExtenderUtil.getAppManifestContextString(appManifest, APPMANIFEST_BUILD_ARTIFACTS_KEYWORD, ""); this.debugSourcePath = ExtenderUtil.getAppManifestContextString(appManifest, APPMANIFEST_DEBUG_SOURCE_PATH, null); + this.minAndroidSdkVersion = ExtenderUtil.getAppManifestContextInteger(appManifest, APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD, DEFAULT_MIN_ANDROID_SDK_VERSION); // assign configuration names started with upper letter because it used for cocoapods if (baseVariant != null && (baseVariant.equals("release") || baseVariant.equals("headless"))) { this.buildConfiguration = "Release"; @@ -101,6 +106,10 @@ public String getDebugSourcePath() { return debugSourcePath; } + public int getMinAndroidSdkVersion() { + return minAndroidSdkVersion; + } + public Boolean isNeedSymbols() { return withSymbols; } diff --git a/server/src/main/java/com/defold/extender/ExtenderUtil.java b/server/src/main/java/com/defold/extender/ExtenderUtil.java index 183558ff..a5ec2f72 100644 --- a/server/src/main/java/com/defold/extender/ExtenderUtil.java +++ b/server/src/main/java/com/defold/extender/ExtenderUtil.java @@ -411,6 +411,25 @@ static String getAppManifestContextString(AppManifestConfiguration manifest, Str return default_value; } + // The app manifest context is generated by bob without quoting, so an integer written there is + // parsed by snakeyaml as an Integer, while a quoted one arrives as a String. Accept both, and + // fall back to the default for anything else (including older clients that omit the key). + static Integer getAppManifestContextInteger(AppManifestConfiguration manifest, String name, Integer default_value) throws ExtenderException { + Object o = getAppManifestContextObject(manifest, name); + if (o instanceof Integer) { + return (Integer)o; + } + if (o instanceof String) { + try { + return Integer.valueOf(((String)o).trim()); + } catch (NumberFormatException e) { + throw new ExtenderException(String.format( + "Error in app.manifest: '%s' must be an integer, got '%s'.", name, o)); + } + } + return default_value; + } + static public boolean isListOfStrings(List list) { return list != null && list.stream().allMatch(o -> o instanceof String); } diff --git a/server/src/main/java/com/defold/extender/ExtensionManifestValidator.java b/server/src/main/java/com/defold/extender/ExtensionManifestValidator.java index 4c2bbbd0..c238a307 100644 --- a/server/src/main/java/com/defold/extender/ExtensionManifestValidator.java +++ b/server/src/main/java/com/defold/extender/ExtensionManifestValidator.java @@ -21,6 +21,7 @@ class ExtensionManifestValidator { private static final Pattern VALID_INCLUDE_PATH = Pattern.compile("^[A-Za-z0-9._+\\-/]+$"); private static final Pattern VALID_SYMBOL_IDENTIFIER = Pattern.compile("^[A-Za-z_][A-Za-z0-9_]*$"); + private static final Pattern VALID_API_LEVEL = Pattern.compile("^[0-9]+$"); ExtensionManifestValidator(WhitelistConfig whitelistConfig, List allowedFlags, List allowedSymbols) { this.allowedDefines.add(WhitelistConfig.compile(whitelistConfig.defineRe)); @@ -57,6 +58,16 @@ void validateAppManifestContext(Map appContext) throws ExtenderE ExtenderBuildState.APPMANIFEST_DEBUG_SOURCE_PATH, s)); } } + + // Reaches the command line as the argument of d8 --min-api, so it must be a bare number. + Object minAndroidSdkVersion = appContext.get(ExtenderBuildState.APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD); + if (minAndroidSdkVersion != null && !(minAndroidSdkVersion instanceof Integer)) { + if (!(minAndroidSdkVersion instanceof String) || !VALID_API_LEVEL.matcher((String) minAndroidSdkVersion).matches()) { + throw new ExtenderException(String.format( + "Error in app.manifest: '%s' must be an integer, got '%s'.", + ExtenderBuildState.APPMANIFEST_MIN_ANDROID_SDK_VERSION_KEYWORD, minAndroidSdkVersion)); + } + } } private void validateIncludePaths(String extensionName, File extensionFolder, List includes) throws ExtenderException { diff --git a/server/src/test/java/com/defold/extender/ExtenderUtilTest.java b/server/src/test/java/com/defold/extender/ExtenderUtilTest.java index 08c48187..0d14302f 100644 --- a/server/src/test/java/com/defold/extender/ExtenderUtilTest.java +++ b/server/src/test/java/com/defold/extender/ExtenderUtilTest.java @@ -376,4 +376,28 @@ public void testSanitizeJavacCmdFlagElsewhere() { String cmd = "javac -source 11 -proc:none Foo.java"; assertEquals(cmd, ExtenderUtil.sanitizeJavacCmd(cmd)); } + + @Test + public void testGetAppManifestContextInteger() throws ExtenderException { + // Exercised with the minAndroidSdkVersion key, the d8 --min-api value + AppManifestConfiguration manifest = new AppManifestConfiguration(); + + // Engines older than the --min-api change send no context at all, or a context without the + // key. Both must fall back to the default instead of failing the build. + assertEquals(Integer.valueOf(21), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21)); + manifest.context = new HashMap<>(); + assertEquals(Integer.valueOf(21), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21)); + + // bob writes the value unquoted, so snakeyaml parses it as an Integer + manifest.context.put("minAndroidSdkVersion", 24); + assertEquals(Integer.valueOf(24), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21)); + + // A quoted value arrives as a String + manifest.context.put("minAndroidSdkVersion", "24"); + assertEquals(Integer.valueOf(24), ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21)); + + manifest.context.put("minAndroidSdkVersion", "not-a-number"); + assertThrows(ExtenderException.class, + () -> ExtenderUtil.getAppManifestContextInteger(manifest, "minAndroidSdkVersion", 21)); + } } diff --git a/server/src/test/java/com/defold/extender/ExtensionManifestValidatorTest.java b/server/src/test/java/com/defold/extender/ExtensionManifestValidatorTest.java index f712dd27..97c2e9ea 100644 --- a/server/src/test/java/com/defold/extender/ExtensionManifestValidatorTest.java +++ b/server/src/test/java/com/defold/extender/ExtensionManifestValidatorTest.java @@ -274,6 +274,44 @@ public void testValidateAppManifestContextDebugSourcePath() throws ExtenderExcep } } + @Test + public void testValidateAppManifestContextMinAndroidSdkVersion() throws ExtenderException { + List empty = new ArrayList<>(); + ExtensionManifestValidator validator = new ExtensionManifestValidator(new WhitelistConfig(), empty, empty); + + // Missing key is accepted: engines older than the --min-api change never send it + assertDoesNotThrow(() -> validator.validateAppManifestContext(new HashMap<>())); + + // bob writes the value unquoted, so snakeyaml hands us an Integer; a quoted one is a String + for (Object v : new Object[] { 21, 24, "21", "24" }) { + Map ctx = new HashMap<>(); + ctx.put("minAndroidSdkVersion", v); + assertDoesNotThrow(() -> validator.validateAppManifestContext(ctx), + "expected to accept minAndroidSdkVersion: " + v); + } + + // The value ends up as the argument of d8 --min-api, and the rendered command is split on + // spaces, so anything that is not a bare number is argv injection + Object[] bad = new Object[] { + "24 --output /tmp/evil", + "24;rm -rf /", + "24$(whoami)", + "-1", + "", + "twentyfour", + true, + }; + for (Object v : bad) { + Map ctx = new HashMap<>(); + ctx.put("minAndroidSdkVersion", v); + ExtenderException exc = assertThrows(ExtenderException.class, + () -> validator.validateAppManifestContext(ctx), + "expected rejection of minAndroidSdkVersion: " + v); + assertTrue(exc.getMessage().contains("minAndroidSdkVersion"), + "message should mention minAndroidSdkVersion, got: " + exc.getMessage()); + } + } + @Test public void testValidateSymbols() throws ExtenderException { List empty = new ArrayList<>(); diff --git a/server/src/test/java/com/defold/extender/TemplateExecutorTest.java b/server/src/test/java/com/defold/extender/TemplateExecutorTest.java index df989d74..143fdc48 100644 --- a/server/src/test/java/com/defold/extender/TemplateExecutorTest.java +++ b/server/src/test/java/com/defold/extender/TemplateExecutorTest.java @@ -19,4 +19,18 @@ public void templateVariablesShouldBeReplacedByContext() { assertThat(result).isEqualTo("Hello James!"); } + @Test + public void minAndroidSdkVersionShouldRenderAsASingleArgumentToMinApi() { + // The rendered command is split on spaces, so an unbound or multi-token + // minAndroidSdkVersion would make d8 read the next flag as the value of --min-api. + TemplateExecutor templateExecutor = new TemplateExecutor(); + String template = "d8 --min-api {{minAndroidSdkVersion}} --main-dex-rules {{mainDexList}}"; + Map context = new HashMap<>(); + context.put("minAndroidSdkVersion", 24); + context.put("mainDexList", "/tmp/main.rules"); + String result = templateExecutor.execute(template, context); + assertThat(result).isEqualTo("d8 --min-api 24 --main-dex-rules /tmp/main.rules"); + assertThat(result.split(" ")).hasSize(5); + } + }