Fix possible NPE when missing os.version system property

google-api-client/src/main/java/com/google/api/client/googleapis/services/AbstractGoogleClientRequest.java

@@ -12,6 +12,9 @@
 
 package com.google.api.client.googleapis.services;
 
+import static com.google.common.base.StandardSystemProperty.OS_NAME;
+import static com.google.common.base.StandardSystemProperty.OS_VERSION;
+
 import com.google.api.client.googleapis.GoogleUtils;
 import com.google.api.client.googleapis.MethodOverride;
 import com.google.api.client.googleapis.batch.BatchCallback;
@@ -125,7 +128,10 @@ protected AbstractGoogleClientRequest(AbstractGoogleClient abstractGoogleClient,
       requestHeaders.setUserAgent(USER_AGENT_SUFFIX);
     }
     // Set the header for the Api Client version (Java and OS version)
-    requestHeaders.set(API_VERSION_HEADER, ApiClientVersion.build(abstractGoogleClient));
+    requestHeaders.set(
+      API_VERSION_HEADER,
+      ApiClientVersion.getDefault().build(abstractGoogleClient.getClass().getSimpleName())
+    );
   }
 
   /**
@@ -135,21 +141,34 @@ protected AbstractGoogleClientRequest(AbstractGoogleClient abstractGoogleClient,
    * See <a href="https://cloud.google.com/apis/docs/system-parameters"></a>
    *
    */
-  private static class ApiClientVersion {
-    private static final String JAVA_VERSION = getJavaVersion();
-    private static final String OS_NAME = formatName(System.getProperty("os.name"));
-    private static final String OS_VERSION = formatSemver(System.getProperty("os.version"));
-
-    private static String build(AbstractGoogleClient client) {
-      // TODO(chingor): add the API version from the generated client
-      return String.format(
-          "java/%s http-google-%s/%s %s/%s",
-          JAVA_VERSION,
-          formatName(client.getClass().getSimpleName()),
-          formatSemver(GoogleUtils.VERSION),
-          OS_NAME,
-          OS_VERSION
-      );
+  static class ApiClientVersion {
+    private static final ApiClientVersion DEFAULT_VERSION = new ApiClientVersion();
+    private final String headerTemplate;
+
+    ApiClientVersion() {
+      this(getJavaVersion(), OS_NAME.value(), OS_VERSION.value(), GoogleUtils.VERSION);
+    }
+
+    ApiClientVersion(String javaVersion, String osName, String osVersion, String clientVersion) {
+      StringBuilder sb = new StringBuilder("java/");
+      sb.append(formatSemver(javaVersion));
+      sb.append(" http-google-%s/");
+      sb.append(formatSemver(clientVersion));
+      if (osName != null && osVersion != null) {
+        sb.append(" ");
+        sb.append(formatName(osName));
+        sb.append("/");
+        sb.append(formatSemver(osVersion));
+      }
+      this.headerTemplate = sb.toString();
+    }
+
+    String build(String clientName) {
+      return String.format(headerTemplate, formatName(clientName));
+    }
+
+    private static ApiClientVersion getDefault() {
+      return DEFAULT_VERSION;
     }
 
     private static String getJavaVersion() {
@@ -168,6 +187,10 @@ private static String formatName(String name) {
     }
 
     private static String formatSemver(String version) {
+      if (version == null) {
+        return null;
+      }
+
       // Take only the semver version: x.y.z-a_b_c -> x.y.z
       Matcher m = Pattern.compile("(\\d+\\.\\d+\\.\\d+).*").matcher(version);
       if (m.find()) {

google-api-client/src/test/java/com/google/api/client/googleapis/services/AbstractGoogleClientRequestTest.java

@@ -12,6 +12,7 @@
 
 package com.google.api.client.googleapis.services;
 
+import com.google.api.client.googleapis.services.AbstractGoogleClientRequest.ApiClientVersion;
 import com.google.api.client.googleapis.testing.services.MockGoogleClient;
 import com.google.api.client.googleapis.testing.services.MockGoogleClientRequest;
 import com.google.api.client.http.EmptyContent;
@@ -50,6 +51,26 @@ public class AbstractGoogleClientRequestTest extends TestCase {
       "{\"error\":{\"code\":401,\"errors\":[{\"domain\":\"global\","
       + "\"location\":\"Authorization\",\"locationType\":\"header\","
       + "\"message\":\"me\",\"reason\":\"authError\"}],\"message\":\"me\"}}";
+  private String originalOsName;
+  private String originalOsVersion;
+
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+
+    // save the original system properties so we can mock them out
+    this.originalOsName = System.getProperty("os.name");
+    this.originalOsVersion = System.getProperty("os.version");
+  }
+
+  @Override
+  protected void tearDown() throws Exception {
+    // restore the original system properties
+    System.setProperty("os.name", originalOsName);
+    System.setProperty("os.version", originalOsVersion);
+
+    super.tearDown();
+  }
 
   public void testExecuteUnparsed_error() throws Exception {
     HttpTransport transport = new MockHttpTransport() {
@@ -208,6 +229,23 @@ public void testSetsApiClientHeader() throws Exception {
     request.executeUnparsed();
   }
 
+  public void testSetsApiClientHeaderWithOsVersion() throws Exception {
+    System.setProperty("os.name", "My OS");
+    System.setProperty("os.version", "1.2.3");
+
+    String version = new ApiClientVersion().build("My Client");
+    assertTrue("Api version should contain the os version", version.matches(".* my-os/1.2.3"));
+  }
+
+  public void testSetsApiClientHeaderWithoutOsVersion() throws Exception {
+    System.setProperty("os.name", "My OS");
+    System.clearProperty("os.version");
+    assertNull(System.getProperty("os.version"));
+
+    String version = new ApiClientVersion().build("My Client");
+    assertFalse("Api version should not contain the os version", version.matches(".*my-os.*"));
+  }
+
   private class AssertHeaderTransport extends MockHttpTransport {
     String expectedHeader;
     String expectedHeaderValue;