Guard against NPE in RegistrationRequest#build()

quehanwei · Nov 17, 2016 · 9f2c118 · 9f2c118
1 parent 705be71
commit 9f2c118
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 10 deletions.
diff --git a/java/server/src/org/openqa/grid/common/RegistrationRequest.java b/java/server/src/org/openqa/grid/common/RegistrationRequest.java
@@ -200,21 +200,22 @@ public static RegistrationRequest build(GridNodeConfiguration configuration, Str
    * @return
    */
   public static RegistrationRequest build(GridNodeConfiguration configuration, String name, String description) {
-    RegistrationRequest pendingRequest = (configuration == null) ?
-      new RegistrationRequest(new GridNodeConfiguration(), name, description) :
-      new RegistrationRequest(configuration, name, description);
+    GridNodeConfiguration pendingConfiguration = (configuration == null) ?
+                                                 new GridNodeConfiguration() : configuration;
 
-    if (configuration.nodeConfigFile != null) {
-      pendingRequest.configuration = GridNodeConfiguration.loadFromJSON(configuration.nodeConfigFile);
+    RegistrationRequest pendingRequest = new RegistrationRequest(pendingConfiguration, name, description);
+
+    if (pendingConfiguration.nodeConfigFile != null) {
+      pendingRequest.configuration = GridNodeConfiguration.loadFromJSON(pendingConfiguration.nodeConfigFile);
     }
 
-    pendingRequest.configuration.merge(configuration);
+    pendingRequest.configuration.merge(pendingConfiguration);
     //update important merge protected values for the pendingRequest we are building.
-    if (configuration.host != null) {
-      pendingRequest.configuration.host = configuration.host;
+    if (pendingConfiguration.host != null) {
+      pendingRequest.configuration.host = pendingConfiguration.host;
     }
-    if (configuration.port != null) {
-      pendingRequest.configuration.port = configuration.port;
+    if (pendingConfiguration.port != null) {
+      pendingRequest.configuration.port = pendingConfiguration.port;
     }
 
     // make sure we have a valid host

diff --git a/java/server/test/org/openqa/grid/common/RegistrationRequestTest.java b/java/server/test/org/openqa/grid/common/RegistrationRequestTest.java
@@ -19,6 +19,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import com.beust.jcommander.JCommander;
@@ -178,4 +179,16 @@ public void testBuildWithConfiguration() {
 
     assertEquals(expectedConfig.toString(), req.getConfiguration().toString());
   }
+
+  /**
+   * Tests that RegistrationRequest.build() performs as expected
+   */
+  @Test
+  public void testBuild() {
+    RegistrationRequest req = RegistrationRequest.build();
+    assertNotNull(req);
+    assertNotNull(req.getConfiguration());
+    assertNull(req.getName());
+    assertNull(req.getDescription());
+  }
 }