Throw ConfigParseException consistently in all JSON deserializers on parsing empty or null datafile (#77)

Author: onufryk

core-api/src/main/java/com/optimizely/ab/Optimizely.java

@@ -498,6 +498,13 @@ private void track(@Nonnull String eventName,
      * @return a {@link ProjectConfig} instance given a json string
      */
     private static ProjectConfig getProjectConfig(String datafile) throws ConfigParseException {
+        if (datafile == null) {
+            throw new ConfigParseException("Unable to parse null datafile.");
+        }
+        if (datafile.length() == 0) {
+            throw new ConfigParseException("Unable to parse empty datafile.");
+        }
+
         return DefaultConfigParser.getInstance().parseProjectConfig(datafile);
     }
 

core-api/src/main/java/com/optimizely/ab/config/parser/GsonConfigParser.java

@@ -33,6 +33,12 @@ final class GsonConfigParser implements ConfigParser {
 
     @Override
     public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParseException {
+        if (json == null) {
+            throw new ConfigParseException("Unable to parse null json.");
+        }
+        if (json.length() == 0) {
+            throw new ConfigParseException("Unable to parse empty json.");
+        }
         Gson gson = new GsonBuilder()
             .registerTypeAdapter(ProjectConfig.class, new ProjectConfigGsonDeserializer())
             .registerTypeAdapter(Audience.class, new AudienceGsonDeserializer())

core-api/src/test/java/com/optimizely/ab/OptimizelyBuilderTest.java

@@ -164,6 +164,19 @@ public void withCustomClientVersion() throws Exception {
         assertThat(((EventBuilderV2)optimizelyClient.eventBuilder).clientVersion, is("0.0.0"));
     }
 
+    @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation")
+    @Test
+    public void builderThrowsConfigParseExceptionForNullDatafile() throws Exception {
+        thrown.expect(ConfigParseException.class);
+        Optimizely.builder(null, mockEventHandler).build();
+    }
+
+    @Test
+    public void builderThrowsConfigParseExceptionForEmptyDatafile() throws Exception {
+        thrown.expect(ConfigParseException.class);
+        Optimizely.builder("", mockEventHandler).build();
+    }
+
     @Test
     public void builderThrowsConfigParseExceptionForInvalidDatafile() throws Exception {
         thrown.expect(ConfigParseException.class);

core-api/src/test/java/com/optimizely/ab/config/parser/DefaultConfigParserTest.java

@@ -27,4 +27,4 @@ public class DefaultConfigParserTest {
     public void createThrowException() throws Exception {
         // FIXME - mdodsworth: hmmm, this isn't going to be the easiest thing to test
     }
-}
+}

core-api/src/test/java/com/optimizely/ab/config/parser/GsonConfigParserTest.java

@@ -18,6 +18,7 @@
 
 import com.optimizely.ab.config.ProjectConfig;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -86,4 +87,27 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception {
         GsonConfigParser parser = new GsonConfigParser();
         parser.parseProjectConfig("{\"valid\": \"json\"}");
     }
-}
+
+    /**
+     * Verify that empty string JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    public void emptyJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        GsonConfigParser parser = new GsonConfigParser();
+        parser.parseProjectConfig("");
+    }
+
+    /**
+     * Verify that null JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation")
+    public void nullJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        GsonConfigParser parser = new GsonConfigParser();
+        parser.parseProjectConfig(null);
+    }
+}

core-api/src/test/java/com/optimizely/ab/config/parser/JacksonConfigParserTest.java

@@ -18,6 +18,7 @@
 
 import com.optimizely.ab.config.ProjectConfig;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -86,4 +87,27 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception {
         JacksonConfigParser parser = new JacksonConfigParser();
         parser.parseProjectConfig("{\"valid\": \"json\"}");
     }
-}
+
+    /**
+     * Verify that empty string JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    public void emptyJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        JacksonConfigParser parser = new JacksonConfigParser();
+        parser.parseProjectConfig("");
+    }
+
+    /**
+     * Verify that null JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation")
+    public void nullJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        JacksonConfigParser parser = new JacksonConfigParser();
+        parser.parseProjectConfig(null);
+    }
+}

core-api/src/test/java/com/optimizely/ab/config/parser/JsonConfigParserTest.java

@@ -18,6 +18,7 @@
 
 import com.optimizely.ab.config.ProjectConfig;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -86,4 +87,27 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception {
         JsonConfigParser parser = new JsonConfigParser();
         parser.parseProjectConfig("{\"valid\": \"json\"}");
     }
-}
+
+    /**
+     * Verify that empty string JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    public void emptyJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        JsonConfigParser parser = new JsonConfigParser();
+        parser.parseProjectConfig("");
+    }
+
+    /**
+     * Verify that null JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation")
+    public void nullJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        JsonConfigParser parser = new JsonConfigParser();
+        parser.parseProjectConfig(null);
+    }
+}

core-api/src/test/java/com/optimizely/ab/config/parser/JsonSimpleConfigParserTest.java

@@ -18,6 +18,7 @@
 
 import com.optimizely.ab.config.ProjectConfig;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -86,4 +87,25 @@ public void validJsonRequiredFieldMissingExceptionWrapping() throws Exception {
         JsonSimpleConfigParser parser = new JsonSimpleConfigParser();
         parser.parseProjectConfig("{\"valid\": \"json\"}");
     }
-}
+    /**
+     * Verify that empty string JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    public void emptyJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        JsonSimpleConfigParser parser = new JsonSimpleConfigParser();
+        parser.parseProjectConfig("");
+    }
+    /**
+     * Verify that null JSON results in a {@link ConfigParseException} being thrown.
+     */
+    @Test
+    @SuppressFBWarnings(value="NP_NONNULL_PARAM_VIOLATION", justification="Testing nullness contract violation")
+    public void nullJsonExceptionWrapping() throws Exception {
+        thrown.expect(ConfigParseException.class);
+
+        JsonSimpleConfigParser parser = new JsonSimpleConfigParser();
+        parser.parseProjectConfig(null);
+    }
+}