prepare 5.6.0 release (#241)

Author: LaunchDarklyCI

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: