prepare 5.0.1 release (#227)

## [5.0.1] - 2023-08-25 ### Fixed: - Updated how Auto Environment Attributes sanitizes and validates provided values to provide a more user friendly experience. --------- Co-authored-by: Ember Stevens <[email protected]> Co-authored-by: Ember Stevens <[email protected]> Co-authored-by: Todd Anderson <[email protected]> Co-authored-by: tanderson-ld <[email protected]> Co-authored-by: ld-repository-standards[bot] <113625520+ld-repository-standards[bot]@users.noreply.github.com> Co-authored-by: Kane Parkinson <[email protected]> Co-authored-by: LaunchDarklyReleaseBot <[email protected]> Co-authored-by: Matthew M. Keeler <[email protected]>
launchdarkly · Aug 25, 2023 · 3ebe19f · 3ebe19f
1 parent 4662e4c
commit 3ebe19f
Show file tree

Hide file tree

Showing 17 changed files with 267 additions and 39 deletions.
diff --git a/contract-tests/src/main/java/com/launchdarkly/sdktest/Representations.java b/contract-tests/src/main/java/com/launchdarkly/sdktest/Representations.java
@@ -56,7 +56,9 @@ public static class SdkConfigEventParams {
 
     public static class SdkConfigTagParams {
         String applicationId;
+        String applicationName;
         String applicationVersion;
+        String applicationVersionName;
     }
 
     public static class SdkConfigServiceEndpointParams {

diff --git a/contract-tests/src/main/java/com/launchdarkly/sdktest/SdkClientEntity.java b/contract-tests/src/main/java/com/launchdarkly/sdktest/SdkClientEntity.java
@@ -313,9 +313,15 @@ private LDConfig buildSdkConfig(SdkConfigParams params, LDLogAdapter logAdapter,
       if (params.tags.applicationId != null) {
         ab.applicationId(params.tags.applicationId);
       }
+      if (params.tags.applicationName != null) {
+        ab.applicationName(params.tags.applicationName);
+      }
       if (params.tags.applicationVersion != null) {
         ab.applicationVersion(params.tags.applicationVersion);
       }
+      if (params.tags.applicationVersionName != null) {
+        ab.applicationVersionName(params.tags.applicationVersionName);
+      }
       builder.applicationInfo(ab);
     }
 

diff --git a/launchdarkly-android-client-sdk/build.gradle b/launchdarkly-android-client-sdk/build.gradle
@@ -59,6 +59,7 @@ configurations.all {
 ext {}
 ext.versions = [
     "androidAnnotation": "1.2.0",
+    "androidAppcompat": "1.1.0",
     "eventsource": "3.0.0",
     "gson": "2.8.9",
     "jacksonCore": "2.10.5",
@@ -82,6 +83,7 @@ dependencies {
 
     implementation("com.google.code.gson:gson:${versions.gson}")
     implementation("androidx.annotation:annotation:${versions.androidAnnotation}")
+    implementation("androidx.appcompat:appcompat:${versions.androidAppcompat}")
     implementation("com.launchdarkly:launchdarkly-java-sdk-internal:${versions.launchdarklyJavaSdkInternal}")
     implementation("com.launchdarkly:okhttp-eventsource:${versions.eventsource}")
     implementation("com.squareup.okhttp3:okhttp:${versions.okhttp}")

diff --git a/...sdk/src/androidTest/java/com/launchdarkly/sdk/android/EnvironmentReporterBuilderTest.java b/...sdk/src/androidTest/java/com/launchdarkly/sdk/android/EnvironmentReporterBuilderTest.java
@@ -47,6 +47,17 @@ public void defaultsToSDKValues() {
         Assert.assertEquals(LDPackageConsts.SDK_NAME, reporter.getApplicationInfo().getApplicationName());
         Assert.assertEquals(BuildConfig.VERSION_NAME, reporter.getApplicationInfo().getApplicationVersion());
         Assert.assertEquals(BuildConfig.VERSION_NAME, reporter.getApplicationInfo().getApplicationVersionName());
+    }
 
+    @Test
+    public void fallBackWhenIDMissing() {
+        EnvironmentReporterBuilder builder = new EnvironmentReporterBuilder();
+        ApplicationInfo manualInfoInput = new ApplicationInfoBuilder().applicationName("imNotAnID!").createApplicationInfo();
+        builder.setApplicationInfo(manualInfoInput);
+        IEnvironmentReporter reporter = builder.build();
+        Assert.assertEquals(LDPackageConsts.SDK_NAME, reporter.getApplicationInfo().getApplicationId());
+        Assert.assertEquals(LDPackageConsts.SDK_NAME, reporter.getApplicationInfo().getApplicationName());
+        Assert.assertEquals(BuildConfig.VERSION_NAME, reporter.getApplicationInfo().getApplicationVersion());
+        Assert.assertEquals(BuildConfig.VERSION_NAME, reporter.getApplicationInfo().getApplicationVersionName());
     }
 }
diff --git a/...android-client-sdk/src/main/java/com/launchdarkly/sdk/android/AutoEnvContextModifier.java b/...android-client-sdk/src/main/java/com/launchdarkly/sdk/android/AutoEnvContextModifier.java
@@ -13,6 +13,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.Callable;
 
 /**
@@ -139,8 +140,8 @@ private List<ContextRecipe> makeRecipeList() {
                 new ContextRecipe(
                         ldApplicationKind,
                         () -> LDUtil.urlSafeBase64Hash(
-                                environmentReporter.getApplicationInfo().getApplicationId() + ":"
-                                        + environmentReporter.getApplicationInfo().getApplicationVersion()
+                                Objects.toString(environmentReporter.getApplicationInfo().getApplicationId(), "") + ":"
+                                        + Objects.toString(environmentReporter.getApplicationInfo().getApplicationVersion(), "")
                         ),
                         applicationCallables
                 ),

diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDConfig.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDConfig.java
@@ -44,7 +44,6 @@ public class LDConfig {
      */
     public static final int MIN_BACKGROUND_POLL_INTERVAL_MILLIS = 900_000;
 
-    static final String DEFAULT_LOGGER_NAME = "LaunchDarklySdk";
     static final LDLogLevel DEFAULT_LOG_LEVEL = LDLogLevel.INFO;
 
     static final MediaType JSON = MediaType.parse("application/json; charset=utf-8");
@@ -205,7 +204,7 @@ public enum AutoEnvAttributes {
         private PersistentDataStore persistentDataStore;
 
         private LDLogAdapter logAdapter = defaultLogAdapter();
-        private String loggerName = DEFAULT_LOGGER_NAME;
+        private String loggerName = LDPackageConsts.DEFAULT_LOGGER_NAME;
         private LDLogLevel logLevel = null;
 
         /**
@@ -607,7 +606,7 @@ public Builder logLevel(LDLogLevel logLevel) {
          * @see #logLevel(LDLogLevel) 
          */
         public Builder loggerName(String loggerName) {
-            this.loggerName = loggerName == null ? DEFAULT_LOGGER_NAME : loggerName;
+            this.loggerName = loggerName == null ? LDPackageConsts.DEFAULT_LOGGER_NAME : loggerName;
             return this;
         }
 

diff --git a/...darkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDPackageConsts.java b/...darkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDPackageConsts.java
@@ -4,4 +4,5 @@ public class LDPackageConsts {
     public static final String SDK_NAME = "android-client-sdk";
     public static final String SDK_PLATFORM_NAME = "Android";
     public static final String SDK_CLIENT_NAME = "AndroidClient";
+    public static final String DEFAULT_LOGGER_NAME = "LaunchDarklySdk";
 }
diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java
@@ -3,6 +3,7 @@
 import android.util.Base64;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 
 import com.launchdarkly.logging.LDLogger;
 import com.launchdarkly.logging.LogValues;
@@ -28,7 +29,7 @@
 
 import okhttp3.Headers;
 
-class LDUtil {
+public class LDUtil {
     static final String AUTH_SCHEME = "api_key ";
     static final String USER_AGENT_HEADER_VALUE = LDPackageConsts.SDK_CLIENT_NAME + "/" + BuildConfig.VERSION_NAME;
 
@@ -44,8 +45,41 @@ public void onError(Throwable error) {
         };
     }
 
-    // Tag values must not be empty, and only contain letters, numbers, `.`, `_`, or `-`.
-    private static Pattern TAG_VALUE_REGEX = Pattern.compile("^[-a-zA-Z0-9._]+$");
+    // Key, kind, tag, and several other system values must not be empty, contain only letters,
+    // numbers, `.`, `_`, or `-`.
+    private static final Pattern VALID_CHARS_REGEX = Pattern.compile("^[-a-zA-Z0-9._]+$");
+
+    /**
+     * @param s the string to validate
+     * @return null if valid, otherwise string describing issue
+     */
+    @Nullable
+    public static String validateStringValue(@NonNull String s) {
+        if (s.isEmpty()) {
+            return "Empty string.";
+        }
+
+        if (s.length() > 64) {
+            return "Longer than 64 characters.";
+        }
+
+        if (!VALID_CHARS_REGEX.matcher(s).matches()) {
+            return "Contains invalid characters.";
+        }
+        return null;
+    }
+
+    /**
+     * Replaces spaces with hyphens.  In the future, if this function is made more generic,
+     * understand that customer data may already exist and changing this sanitization may
+     * have adverse consequences.
+     *
+     * @param s the string to sanitize
+     * @return the sanitized string
+     */
+    public static String sanitizeSpaces(String s) {
+        return s.replace(' ', '-');
+    }
 
     /**
      * Builds the "X-LaunchDarkly-Tags" HTTP header out of the configured application info.
@@ -56,6 +90,7 @@ public void onError(Throwable error) {
     static String applicationTagHeader(ApplicationInfo applicationInfo, LDLogger logger) {
         String[][] tags = {
                 {"applicationId", "application-id", applicationInfo.getApplicationId()},
+                {"applicationName", "application-name", applicationInfo.getApplicationName()},
                 {"applicationVersion", "application-version", applicationInfo.getApplicationVersion()},
                 {"applicationVersionName", "application-version-name", applicationInfo.getApplicationVersionName()}
         };
@@ -67,12 +102,9 @@ static String applicationTagHeader(ApplicationInfo applicationInfo, LDLogger log
             if (tagVal == null) {
                 continue;
             }
-            if (!TAG_VALUE_REGEX.matcher(tagVal).matches()) {
-                logger.warn("Value of ApplicationInfo.{} contained invalid characters and was discarded", javaKey);
-                continue;
-            }
-            if (tagVal.length() > 64) {
-                logger.warn("Value of ApplicationInfo.{} was longer than 64 characters and was discarded", javaKey);
+            String error = validateStringValue(tagVal);
+            if (error != null) {
+                logger.warn("Value of ApplicationInfo.{} was invalid. {}", javaKey, error);
                 continue;
             }
             parts.add(tagKey + "/" + tagVal);

diff --git a/...client-sdk/src/main/java/com/launchdarkly/sdk/android/env/AndroidEnvironmentReporter.java b/...client-sdk/src/main/java/com/launchdarkly/sdk/android/env/AndroidEnvironmentReporter.java
@@ -7,6 +7,7 @@
 import androidx.annotation.NonNull;
 
 import com.launchdarkly.sdk.android.LDPackageConsts;
+import com.launchdarkly.sdk.android.integrations.ApplicationInfoBuilder;
 import com.launchdarkly.sdk.android.subsystems.ApplicationInfo;
 
 import java.util.Locale;
@@ -30,12 +31,20 @@ public AndroidEnvironmentReporter(Application application) {
     @Override
     @NonNull
     public ApplicationInfo getApplicationInfo() {
-        return new ApplicationInfo(
-                getApplicationID(),
-                getApplicationVersion(),
-                getApplicationName(),
-                getApplicationVersionName()
-        );
+        // use a builder here since it has sanitization and validation built in
+        ApplicationInfoBuilder builder = new ApplicationInfoBuilder();
+        builder.applicationId(getApplicationID());
+        builder.applicationVersion(getApplicationVersion());
+        builder.applicationName(getApplicationName());
+        builder.applicationVersionName(getApplicationVersionName());
+        ApplicationInfo info = builder.createApplicationInfo();
+
+        // defer to super if required property applicationID is missing
+        if (info.getApplicationId() == null) {
+            info = super.getApplicationInfo();
+        }
+
+        return info;
     }
 
     @NonNull
@@ -80,12 +89,10 @@ public String getOSVersion() {
         return Build.VERSION.RELEASE;
     }
 
-    @NonNull
     private String getApplicationID() {
         return application.getPackageName();
     }
 
-    @NonNull
     private String getApplicationName() {
         try {
             PackageManager pm = application.getPackageManager();
@@ -99,7 +106,6 @@ private String getApplicationName() {
         }
     }
 
-    @NonNull
     private String getApplicationVersion() {
         try {
             if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
@@ -109,22 +115,23 @@ private String getApplicationVersion() {
             }
         } catch (PackageManager.NameNotFoundException e) {
             // We don't really expect this to ever happen since we just queried the platform
-            // for the application name and then immediately used it.  Since the code has
-            // this logical path, the best we can do is defer to the next in the chain.
-            return super.getApplicationInfo().getApplicationVersion();
+            // for the application name and then immediately used it.  It is best to set
+            // this to null, if enough properties are missing, we will fallback to the
+            // next source in the chain.
+            return null;
         }
     }
 
-    @NonNull
     private String getApplicationVersionName() {
         try {
             PackageManager pm = application.getPackageManager();
             return pm.getPackageInfo(application.getPackageName(), 0).versionName;
         } catch (PackageManager.NameNotFoundException e) {
             // We don't really expect this to ever happen since we just queried the platform
-            // for the application name and then immediately used it.  Since the code has
-            // this logical path, the best we can do is defer to the next in the chain.
-            return super.getApplicationInfo().getApplicationVersionName();
+            // for the application name and then immediately used it.  It is best to set
+            // this to null, if enough properties are missing, we will fallback to the
+            // next source in the chain.
+            return null;
         }
     }
 }
diff --git a/...dk/src/main/java/com/launchdarkly/sdk/android/env/ApplicationInfoEnvironmentReporter.java b/...dk/src/main/java/com/launchdarkly/sdk/android/env/ApplicationInfoEnvironmentReporter.java
@@ -19,6 +19,10 @@ public ApplicationInfoEnvironmentReporter(ApplicationInfo applicationInfo) {
     @NonNull
     @Override
     public ApplicationInfo getApplicationInfo() {
+        // defer to super if required property applicationID is missing
+        if (applicationInfo.getApplicationId() == null) {
+            return super.getApplicationInfo();
+        }
         return applicationInfo;
     }
 }
diff --git a/...t-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/ApplicationInfoBuilder.java b/...t-sdk/src/main/java/com/launchdarkly/sdk/android/integrations/ApplicationInfoBuilder.java
@@ -1,6 +1,13 @@
 package com.launchdarkly.sdk.android.integrations;
 
+import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
+import androidx.core.util.Consumer;
+
+import com.launchdarkly.logging.LDLogger;
 import com.launchdarkly.sdk.android.Components;
+import com.launchdarkly.sdk.android.LDAndroidLogging;
+import com.launchdarkly.sdk.android.LDUtil;
 import com.launchdarkly.sdk.android.subsystems.ApplicationInfo;
 
 /**
@@ -25,11 +32,18 @@
  * @since 4.1.0
  */
 public final class ApplicationInfoBuilder {
+    @Nullable
     private String applicationId;
+    @Nullable
     private String applicationName;
+    @Nullable
     private String applicationVersion;
+    @Nullable
     private String applicationVersionName;
 
+    @VisibleForTesting
+    LDLogger logger = LDLogger.withAdapter(LDAndroidLogging.adapter(), ApplicationInfoBuilder.class.getSimpleName());
+
     /**
      * Create an empty ApplicationInfoBuilder.
      *
@@ -48,7 +62,7 @@ public ApplicationInfoBuilder() {}
      * @return the builder
      */
     public ApplicationInfoBuilder applicationId(String applicationId) {
-        this.applicationId = applicationId;
+        validatedThenSet("applicationId", s -> this.applicationId = s, applicationId, this.logger);
         return this;
     }
 
@@ -63,7 +77,7 @@ public ApplicationInfoBuilder applicationId(String applicationId) {
      * @return the builder
      */
     public ApplicationInfoBuilder applicationName(String applicationName) {
-        this.applicationName = applicationName;
+        validatedThenSet("applicationName", s -> this.applicationName = s, applicationName, this.logger);
         return this;
     }
 
@@ -79,7 +93,7 @@ public ApplicationInfoBuilder applicationName(String applicationName) {
      * @return the builder
      */
     public ApplicationInfoBuilder applicationVersion(String version) {
-        this.applicationVersion = version;
+        validatedThenSet("applicationVersion", s -> this.applicationVersion = s, version, this.logger);
         return this;
     }
 
@@ -94,7 +108,7 @@ public ApplicationInfoBuilder applicationVersion(String version) {
      * @return the builder
      */
     public ApplicationInfoBuilder applicationVersionName(String versionName) {
-        this.applicationVersionName = versionName;
+        validatedThenSet("applicationVersionName", s -> this.applicationVersionName = s, versionName, this.logger);
         return this;
     }
 
@@ -106,4 +120,27 @@ public ApplicationInfoBuilder applicationVersionName(String versionName) {
     public ApplicationInfo createApplicationInfo() {
         return new ApplicationInfo(applicationId, applicationVersion, applicationName, applicationVersionName);
     }
+
+    /**
+     * @param propertyName   the name of the property being set.  Used for logging.
+     * @param propertySetter lambda for setting the property.  Java is fun and has predefined
+     *                       functional interfaces.
+     * @param input          the string that will be sanitized and validated then applied
+     * @param logger         use for logging.  Can you believe that!?
+     */
+    private void validatedThenSet(String propertyName, Consumer<String> propertySetter, String input, LDLogger logger) {
+        if (input == null) {
+            propertySetter.accept(input);
+            return;
+        }
+
+        String sanitized = LDUtil.sanitizeSpaces(input);
+        String error = LDUtil.validateStringValue(sanitized);
+        if (error != null) {
+            logger.warn("Issue setting {} value '{}'. {}", propertyName, sanitized, error);
+            return;
+        }
+
+        propertySetter.accept(sanitized);
+    }
 }