Skip to content

Commit

Permalink
DEF-3280 Fixed some null pointer accesses due to "malformed" manifest…
Browse files Browse the repository at this point in the history
… files (#91)
  • Loading branch information
mathiaswking authored Aug 22, 2018
1 parent 4550ee4 commit 0dae4ac
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 9 deletions.
10 changes: 5 additions & 5 deletions server/src/main/java/com/defold/extender/Extender.java
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ private List<File> buildJava(File rJar) throws ExtenderException {

Map<String, Object> manifestContext = new HashMap<>();
if (manifestConfig.platforms != null) {
manifestContext = getManifestContext(manifestConfig);
manifestContext = getManifestContext(platform, config, manifestConfig);
}

this.manifestValidator.validate(manifestConfig.name, manifestContext);
Expand All @@ -665,8 +665,8 @@ private List<File> buildJava(File rJar) throws ExtenderException {
return builtJars;
}

private Map<String, Object> getManifestContext(ManifestConfiguration manifestConfig) throws ExtenderException {
ManifestPlatformConfig manifestPlatformConfig = manifestConfig.platforms.get(this.platform);
public static Map<String, Object> getManifestContext(String platform, Configuration config, ManifestConfiguration manifestConfig) throws ExtenderException {
ManifestPlatformConfig manifestPlatformConfig = manifestConfig.platforms.get(platform);

// Check that the manifest only contains valid platforms
Set<String> allowedPlatforms = config.platforms.keySet();
Expand All @@ -676,7 +676,7 @@ private Map<String, Object> getManifestContext(ManifestConfiguration manifestCon
throw new ExtenderException(String.format("Extension %s contains invalid platform(s): %s. Allowed platforms: %s", manifestConfig.name, manifestPlatforms.toString(), allowedPlatforms.toString()));
}

if (manifestPlatformConfig != null) {
if (manifestPlatformConfig != null && manifestPlatformConfig.context != null) {
return manifestPlatformConfig.context;
}
return new HashMap<>();
Expand Down Expand Up @@ -749,7 +749,7 @@ private File buildEngine() throws ExtenderException {

Map<String, Object> manifestContext = new HashMap<>();
if (manifestConfig.platforms != null) {
manifestContext = getManifestContext(manifestConfig);
manifestContext = getManifestContext(platform, config, manifestConfig);
}

String relativePath = ExtenderUtil.getRelativePath(this.uploadDirectory, manifest);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/defold/extender/ExtenderUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public boolean accept(File file) {
public static Map<String, Object> getAppManifestContext(AppManifestConfiguration manifest, String platform) throws ExtenderException {
Map<String, Object> appManifestContext = new HashMap<>();

if( manifest == null )
if (manifest == null || manifest.platforms == null)
return appManifestContext;

if (manifest.platforms.containsKey("common")) {
Expand Down
25 changes: 22 additions & 3 deletions server/src/test/java/com/defold/extender/ExtenderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ public void testCollectJsFiles() {
public void testExcludeItems() throws IOException, InterruptedException, ExtenderException {

File root = new File("test-data");
File appManifestFile = new File("test-data/extendertest.app.manifest");
File appManifestFile = new File("test-data/extendertest.appmanifest");

AppManifestConfiguration appManifest = Extender.loadYaml(root, appManifestFile, AppManifestConfiguration.class);

Expand Down Expand Up @@ -450,10 +450,10 @@ public void testExcludeItems() throws IOException, InterruptedException, Extende
}
}
@Test
public void testAppManifestContext() throws IOException, InterruptedException, ExtenderException {
public void testAppManifestContext() throws IOException, ExtenderException {

File root = new File("test-data");
File appManifestFile = new File("test-data/extendertest.app.manifest");
File appManifestFile = new File("test-data/extendertest.appmanifest");

AppManifestConfiguration appManifest = Extender.loadYaml(root, appManifestFile, AppManifestConfiguration.class);

Expand All @@ -468,4 +468,23 @@ public void testAppManifestContext() throws IOException, InterruptedException, E
assertEquals( expectedItems, context.get("flags") );
}

@Test
public void testGetAppmanifest() throws IOException, ExtenderException {
File root = new File("test-data");

AppManifestConfiguration appManifest = Extender.loadYaml(root, new File("test-data/extendertest.platformnull.appmanifest"), AppManifestConfiguration.class);
// previous issue was that it threw a null pointer exception
ExtenderUtil.getAppManifestContext(appManifest, "x86_64-osx");
}

@Test
public void testGetManifestContext() throws IOException, ExtenderException {
File root = new File("test-data");
Configuration config = Extender.loadYaml(root, new File("test-data/sdk/a/defoldsdk/extender/build.yml"), Configuration.class);

ManifestConfiguration manifestConfig = Extender.loadYaml(root, new File("test-data/extendertest.emptycontext.manifest"), ManifestConfiguration.class);
// previous issue was that it returned a null pointer
Map<String, Object> manifestContext = Extender.getManifestContext("x86_64-osx", config, manifestConfig);
assertNotEquals(null, manifestContext);
}
}
File renamed without changes.
5 changes: 5 additions & 0 deletions server/test-data/extendertest.emptycontext.manifest
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Leaving the context empty will leave a null pointer
platforms:
x86_64-osx:
context:

2 changes: 2 additions & 0 deletions server/test-data/extendertest.platformnull.appmanifest
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# By leaving the value empty, it will become null
platforms:

0 comments on commit 0dae4ac

Please sign in to comment.