prepare 5.6.0 release (#241)

src/main/java/com/launchdarkly/sdk/server/FeatureFlagsState.java

@@ -1,5 +1,7 @@
 package com.launchdarkly.sdk.server;
 
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 import com.google.gson.TypeAdapter;
 import com.google.gson.annotations.JsonAdapter;
 import com.google.gson.stream.JsonReader;
@@ -10,7 +12,6 @@
 import com.launchdarkly.sdk.server.interfaces.LDClientInterface;
 
 import java.io.IOException;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -36,19 +37,20 @@
  */
 @JsonAdapter(FeatureFlagsState.JsonSerialization.class)
 public final class FeatureFlagsState implements JsonSerializable {
-  private final Map<String, LDValue> flagValues;
-  private final Map<String, FlagMetadata> flagMetadata;
+  private final ImmutableMap<String, FlagMetadata> flagMetadata;
   private final boolean valid;
 
   static class FlagMetadata {
+    final LDValue value;
     final Integer variation;
     final EvaluationReason reason;
     final Integer version;
     final Boolean trackEvents;
     final Long debugEventsUntilDate;
 
-    FlagMetadata(Integer variation, EvaluationReason reason, Integer version, boolean trackEvents,
-        Long debugEventsUntilDate) {
+    FlagMetadata(LDValue value, Integer variation, EvaluationReason reason, Integer version,
+        boolean trackEvents, Long debugEventsUntilDate) {
+      this.value = LDValue.normalize(value);
       this.variation = variation;
       this.reason = reason;
       this.version = version;
@@ -60,7 +62,8 @@ static class FlagMetadata {
     public boolean equals(Object other) {
       if (other instanceof FlagMetadata) {
         FlagMetadata o = (FlagMetadata)other;
-        return Objects.equals(variation, o.variation) &&
+        return value.equals(o.value) &&
+            Objects.equals(variation, o.variation) &&
             Objects.equals(reason, o.reason) &&
             Objects.equals(version, o.version) &&
             Objects.equals(trackEvents, o.trackEvents) &&
@@ -75,13 +78,27 @@ public int hashCode() {
     }
   }
 
-  private FeatureFlagsState(Map<String, LDValue> flagValues,
-      Map<String, FlagMetadata> flagMetadata, boolean valid) {
-    this.flagValues = Collections.unmodifiableMap(flagValues);
-    this.flagMetadata = Collections.unmodifiableMap(flagMetadata);
+  private FeatureFlagsState(ImmutableMap<String, FlagMetadata> flagMetadata, boolean valid) {
+    this.flagMetadata = flagMetadata;
     this.valid = valid;
   }
 
+  /**
+   * Returns a {@link Builder} for creating instances.
+   * <p>
+   * Application code will not normally use this builder, since the SDK creates its own instances.
+   * However, it may be useful in testing, to simulate values that might be returned by
+   * {@link LDClient#allFlagsState(com.launchdarkly.sdk.LDUser, FlagsStateOption...)}.
+   * 
+   * @param options the same {@link FlagsStateOption}s, if any, that would be passed to
+   *   {@link LDClient#allFlagsState(com.launchdarkly.sdk.LDUser, FlagsStateOption...)}
+   * @return a builder object
+   * @since 5.6.0
+   */
+  public static Builder builder(FlagsStateOption... options) {
+    return new Builder(options);
+  }
+
   /**
    * Returns true if this object contains a valid snapshot of feature flag state, or false if the
    * state could not be computed (for instance, because the client was offline or there was no user).
@@ -98,7 +115,8 @@ public boolean isValid() {
    *   {@code null} if there was no such flag
    */
   public LDValue getFlagValue(String key) {
-    return flagValues.get(key);
+    FlagMetadata data = flagMetadata.get(key);
+    return data == null ? null : data.value;
   }
 
   /**
@@ -115,64 +133,100 @@ public EvaluationReason getFlagReason(String key) {
    * Returns a map of flag keys to flag values. If a flag would have evaluated to the default value,
    * its value will be null.
    * <p>
+   * The returned map is unmodifiable.
+   * <p>
    * Do not use this method if you are passing data to the front end to "bootstrap" the JavaScript client.
    * Instead, serialize the FeatureFlagsState object to JSON using {@code Gson.toJson()} or {@code Gson.toJsonTree()}.
    * @return an immutable map of flag keys to JSON values
    */
   public Map<String, LDValue> toValuesMap() {
-    return flagValues;
+    return Maps.transformValues(flagMetadata, v -> v.value);
   }
 
   @Override
   public boolean equals(Object other) {
     if (other instanceof FeatureFlagsState) {
       FeatureFlagsState o = (FeatureFlagsState)other;
-      return flagValues.equals(o.flagValues) &&
-          flagMetadata.equals(o.flagMetadata) &&
+      return flagMetadata.equals(o.flagMetadata) &&
           valid == o.valid;
     }
     return false;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(flagValues, flagMetadata, valid);
+    return Objects.hash(flagMetadata, valid);
   }
 
-  static class Builder {
-    private Map<String, LDValue> flagValues = new HashMap<>();
-    private Map<String, FlagMetadata> flagMetadata = new HashMap<>();
+  /**
+   * A builder for a {@link FeatureFlagsState} instance.
+   * <p>
+   * Application code will not normally use this builder, since the SDK creates its own instances.
+   * However, it may be useful in testing, to simulate values that might be returned by
+   * {@link LDClient#allFlagsState(com.launchdarkly.sdk.LDUser, FlagsStateOption...)}.
+   *
+   * @since 5.6.0
+   */
+  public static class Builder {
+    private ImmutableMap.Builder<String, FlagMetadata> flagMetadata = ImmutableMap.builder();
     private final boolean saveReasons;
     private final boolean detailsOnlyForTrackedFlags;
     private boolean valid = true;
 
-    Builder(FlagsStateOption... options) {
+    private Builder(FlagsStateOption... options) {
       saveReasons = FlagsStateOption.hasOption(options, FlagsStateOption.WITH_REASONS);
       detailsOnlyForTrackedFlags = FlagsStateOption.hasOption(options, FlagsStateOption.DETAILS_ONLY_FOR_TRACKED_FLAGS);
     }
 
-    Builder valid(boolean valid) {
+    /**
+     * Sets the {@link FeatureFlagsState#isValid()} property. This is true by default.
+     * 
+     * @param valid the new property value
+     * @return the builder
+     */
+    public Builder valid(boolean valid) {
       this.valid = valid;
       return this;
     }
 
-    Builder addFlag(DataModel.FeatureFlag flag, Evaluator.EvalResult eval) {
-      flagValues.put(flag.getKey(), eval.getValue());
-      final boolean flagIsTracked = flag.isTrackEvents() ||
-          (flag.getDebugEventsUntilDate() != null && flag.getDebugEventsUntilDate() > System.currentTimeMillis());
+    public Builder add(
+        String flagKey,
+        LDValue value,
+        Integer variationIndex,
+        EvaluationReason reason,
+        int flagVersion,
+        boolean trackEvents,
+        Long debugEventsUntilDate
+        ) {
+      final boolean flagIsTracked = trackEvents ||
+          (debugEventsUntilDate != null && debugEventsUntilDate > System.currentTimeMillis());
       final boolean wantDetails = !detailsOnlyForTrackedFlags || flagIsTracked;
       FlagMetadata data = new FlagMetadata(
+          value,
+          variationIndex,
+          (saveReasons && wantDetails) ? reason : null,
+          wantDetails ? Integer.valueOf(flagVersion) : null,
+          trackEvents,
+          debugEventsUntilDate
+          );
+      flagMetadata.put(flagKey, data);
+      return this;
+    }
+
+    Builder addFlag(DataModel.FeatureFlag flag, Evaluator.EvalResult eval) {
+      return add(
+          flag.getKey(),
+          eval.getValue(),
           eval.isDefault() ? null : eval.getVariationIndex(),
-          (saveReasons && wantDetails) ? eval.getReason() : null,
-          wantDetails ? flag.getVersion() : null,
+          eval.getReason(),
+          flag.getVersion(),
           flag.isTrackEvents(),
-          flag.getDebugEventsUntilDate());
-      flagMetadata.put(flag.getKey(), data);
-      return this;
+          flag.getDebugEventsUntilDate()
+          );
     }
 
     FeatureFlagsState build() {
-      return new FeatureFlagsState(flagValues, flagMetadata, valid);
+      return new FeatureFlagsState(flagMetadata.build(), valid);
     }
   }
 
@@ -181,9 +235,9 @@ static class JsonSerialization extends TypeAdapter<FeatureFlagsState> {
     public void write(JsonWriter out, FeatureFlagsState state) throws IOException {
       out.beginObject();
 
-      for (Map.Entry<String, LDValue> entry: state.flagValues.entrySet()) {
+      for (Map.Entry<String, FlagMetadata> entry: state.flagMetadata.entrySet()) {
         out.name(entry.getKey());
-        gsonInstance().toJson(entry.getValue(), LDValue.class, out);
+        gsonInstance().toJson(entry.getValue().value, LDValue.class, out);
       }
 
       out.name("$flagsState");
@@ -229,7 +283,7 @@ public void write(JsonWriter out, FeatureFlagsState state) throws IOException {
     @Override
     public FeatureFlagsState read(JsonReader in) throws IOException {
       Map<String, LDValue> flagValues = new HashMap<>();
-      Map<String, FlagMetadata> flagMetadata = new HashMap<>();
+      Map<String, FlagMetadata> flagMetadataWithoutValues = new HashMap<>();
       boolean valid = true;
       in.beginObject();
       while (in.hasNext()) {
@@ -239,7 +293,7 @@ public FeatureFlagsState read(JsonReader in) throws IOException {
           while (in.hasNext()) {
             String metaName = in.nextName();
             FlagMetadata meta = gsonInstance().fromJson(in, FlagMetadata.class);
-            flagMetadata.put(metaName, meta);
+            flagMetadataWithoutValues.put(metaName, meta);
           }
           in.endObject();
         } else if (name.equals("$valid")) {
@@ -250,7 +304,22 @@ public FeatureFlagsState read(JsonReader in) throws IOException {
         }
       }
       in.endObject();
-      return new FeatureFlagsState(flagValues, flagMetadata, valid);
+      ImmutableMap.Builder<String, FlagMetadata> allFlagMetadata = ImmutableMap.builder();
+      for (Map.Entry<String, LDValue> e: flagValues.entrySet()) {
+        FlagMetadata m0 = flagMetadataWithoutValues.get(e.getKey());
+        if (m0 != null) {
+          FlagMetadata m1 = new FlagMetadata(
+              e.getValue(),
+              m0.variation,
+              m0.reason,
+              m0.version,
+              m0.trackEvents != null && m0.trackEvents.booleanValue(),
+              m0.debugEventsUntilDate
+              );
+          allFlagMetadata.put(e.getKey(), m1);
+        }
+      }
+      return new FeatureFlagsState(allFlagMetadata.build(), valid);
     }
   }
 }

src/main/java/com/launchdarkly/sdk/server/LDClient.java

@@ -41,6 +41,7 @@
 import static com.launchdarkly.sdk.EvaluationDetail.NO_VARIATION;
 import static com.launchdarkly.sdk.server.DataModel.FEATURES;
 import static com.launchdarkly.sdk.server.DataModel.SEGMENTS;
+import static com.launchdarkly.sdk.server.Util.isAsciiHeaderValue;
 
 /**
  * A client for the LaunchDarkly API. Client instances are thread-safe. Applications should instantiate
@@ -82,8 +83,15 @@ public final class LDClient implements LDClientInterface {
    * values; it will still continue trying to connect in the background. You can detect whether
    * initialization has succeeded by calling {@link #isInitialized()}. If you prefer to customize
    * this behavior, use {@link LDClient#LDClient(String, LDConfig)} instead.
+   * <p>
+   * For rules regarding the throwing of unchecked exceptions for error conditions, see
+   * {@link LDClient#LDClient(String, LDConfig)}.
    *
    * @param sdkKey the SDK key for your LaunchDarkly environment
+   * @throws IllegalArgumentException if a parameter contained a grossly malformed value;
+   *   for security reasons, in case of an illegal SDK key, the exception message does
+   *   not include the key
+   * @throws NullPointerException if a non-nullable parameter was null
    * @see LDClient#LDClient(String, LDConfig)
    */
   public LDClient(String sdkKey) {
@@ -136,14 +144,32 @@ private static final DataModel.Segment getSegment(DataStore store, String key) {
    *         // do whatever is appropriate if initialization has timed out
    *     }
    * </code></pre>
+   * <p>
+   * This constructor can throw unchecked exceptions if it is immediately apparent that
+   * the SDK cannot work with these parameters. For instance, if the SDK key contains a
+   * non-printable character that cannot be used in an HTTP header, it will throw an
+   * {@link IllegalArgumentException} since the SDK key is normally sent to LaunchDarkly
+   * in an HTTP header and no such value could possibly be valid. Similarly, a null
+   * value for a non-nullable parameter may throw a {@link NullPointerException}. The
+   * constructor will not throw an exception for any error condition that could only be
+   * detected after making a request to LaunchDarkly (such as an SDK key that is simply
+   * wrong despite being valid ASCII, so it is invalid but not illegal); those are logged
+   * and treated as an unsuccessful initialization, as described above. 
    *
    * @param sdkKey the SDK key for your LaunchDarkly environment
    * @param config a client configuration object
+   * @throws IllegalArgumentException if a parameter contained a grossly malformed value;
+   *   for security reasons, in case of an illegal SDK key, the exception message does
+   *   not include the key
+   * @throws NullPointerException if a non-nullable parameter was null
    * @see LDClient#LDClient(String, LDConfig)
    */
   public LDClient(String sdkKey, LDConfig config) {
     checkNotNull(config, "config must not be null");
     this.sdkKey = checkNotNull(sdkKey, "sdkKey must not be null");
+    if (!isAsciiHeaderValue(sdkKey) ) {
+      throw new IllegalArgumentException("SDK key contained an invalid character");
+    }
     this.offline = config.offline;
 
     this.sharedExecutor = createSharedExecutor(config);
@@ -268,7 +294,7 @@ private void sendFlagRequestEvent(Event.FeatureRequest event) {
 
   @Override
   public FeatureFlagsState allFlagsState(LDUser user, FlagsStateOption... options) {
-    FeatureFlagsState.Builder builder = new FeatureFlagsState.Builder(options);
+    FeatureFlagsState.Builder builder = FeatureFlagsState.builder(options);
 
     if (isOffline()) {
       Loggers.EVALUATION.debug("allFlagsState() was called when client is in offline mode.");

src/main/java/com/launchdarkly/sdk/server/Util.java

@@ -37,6 +37,25 @@ static Headers.Builder getHeadersBuilderFor(HttpConfiguration config) {
     return builder;
   }
 
+  // This is specifically testing whether the string would be considered a valid HTTP header value
+  // *by the OkHttp client*. The actual HTTP spec does not prohibit characters >= 127; OkHttp's
+  // check is overly strict, as was pointed out in https://github.com/square/okhttp/issues/2016.
+  // But all OkHttp 3.x and 4.x versions so far have continued to enforce that check. Control
+  // characters other than a tab are always illegal.
+  //
+  // The value we're mainly concerned with is the SDK key (Authorization header). If an SDK key
+  // accidentally has (for instance) a newline added to it, we don't want to end up having OkHttp
+  // throw an exception mentioning the value, which might get logged (https://github.com/square/okhttp/issues/6738).
+  static boolean isAsciiHeaderValue(String value) {
+    for (int i = 0; i < value.length(); i++) {
+      char ch = value.charAt(i);
+      if ((ch < 0x20 || ch > 0x7e) && ch != '\t') {
+        return false;
+      }
+    }
+    return true;
+  }
+
   static void configureHttpClientBuilder(HttpConfiguration config, OkHttpClient.Builder builder) {
     builder.connectionPool(new ConnectionPool(5, 5, TimeUnit.SECONDS))
       .connectTimeout(config.getConnectTimeout())

src/main/java/com/launchdarkly/sdk/server/integrations/FileData.java

@@ -1,5 +1,7 @@
 package com.launchdarkly.sdk.server.integrations;
 
+import com.launchdarkly.sdk.server.LDConfig;
+
 /**
  * Integration between the LaunchDarkly SDK and file data.
  * <p>
@@ -56,8 +58,10 @@ public enum DuplicateKeysHandling {
    * <p>
    * This will cause the client <i>not</i> to connect to LaunchDarkly to get feature flags. The
    * client may still make network connections to send analytics events, unless you have disabled
-   * this with {@link com.launchdarkly.sdk.server.Components#noEvents()} or
-   * {@link com.launchdarkly.sdk.server.LDConfig.Builder#offline(boolean)}.
+   * this with {@link com.launchdarkly.sdk.server.Components#noEvents()}. IMPORTANT: Do <i>not</i>
+   * set {@link LDConfig.Builder#offline(boolean)} to {@code true}; doing so would not just put the
+   * SDK "offline" with regard to LaunchDarkly, but will completely turn off all flag data sources
+   * to the SDK <i>including the file data source</i>.
    * <p>
    * Flag data files can be either JSON or YAML. They contain an object with three possible
    * properties: