Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SDKS-7516] Create FlagSetSanitizer to validate flag sets #435

Merged
merged 9 commits into from
Sep 14, 2023
16 changes: 8 additions & 8 deletions client/src/main/java/io/split/client/SplitClientConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
import pluggable.CustomStorageWrapper;

import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.io.InputStream;
import java.util.Properties;
import java.util.concurrent.ThreadFactory;

import static io.split.inputValidation.FlagSetsValidator.cleanup;

/**
* Configurations for the SplitClient.
*
Expand Down Expand Up @@ -86,7 +88,7 @@ public class SplitClientConfig {
// To be set during startup
public static String splitSdkVersion;
private final long _lastSeenCacheSize;
private final List<String> _flagSetsFilter;
private final HashSet<String> _flagSetsFilter;

public static Builder builder() {
return new Builder();
Expand Down Expand Up @@ -142,7 +144,7 @@ private SplitClientConfig(String endpoint,
int filterUniqueKeysRefreshRate,
long lastSeenCacheSize,
ThreadFactory threadFactory,
List<String> flagSetsFilter) {
HashSet<String> flagSetsFilter) {
_endpoint = endpoint;
_eventsEndpoint = eventsEndpoint;
_featuresRefreshRate = pollForFeatureChangesEveryNSeconds;
Expand Down Expand Up @@ -195,7 +197,6 @@ private SplitClientConfig(String endpoint,
_threadFactory = threadFactory;
_flagSetsFilter = flagSetsFilter;


Properties props = new Properties();
try {
props.load(this.getClass().getClassLoader().getResourceAsStream("splitversion.properties"));
Expand Down Expand Up @@ -383,7 +384,7 @@ public long getLastSeenCacheSize() {
public ThreadFactory getThreadFactory() {
return _threadFactory;
}
public List<String> getSetsFilter() {
public HashSet<String> getSetsFilter() {
return _flagSetsFilter;
}

Expand Down Expand Up @@ -442,7 +443,7 @@ public static final class Builder {
private StorageMode _storageMode = StorageMode.MEMORY;
private final long _lastSeenCacheSize = 500000;
private ThreadFactory _threadFactory;
private List<String> _flagSetsFilter = Collections.emptyList();
private HashSet<String> _flagSetsFilter = new HashSet<>();

public Builder() {
}
Expand Down Expand Up @@ -921,8 +922,7 @@ public Builder customStorageWrapper(CustomStorageWrapper customStorageWrapper) {
* @return this builder
*/
public Builder flagSetsFilter(List<String> flagSetsFilter) {
_flagSetsFilter = flagSetsFilter;
//TODO Apply validations
_flagSetsFilter = cleanup(flagSetsFilter);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.split.inputValidation;

import java.util.HashSet;

public class FlagSetsValidResult {
private final Boolean _valid;
private final HashSet<String> _flagSets;

public FlagSetsValidResult(Boolean valid, HashSet<String> flagSets) {
_valid = valid;
_flagSets = flagSets;
}

public Boolean getValid() {
return _valid;
}

public HashSet<String> getFlagSets() {
return _flagSets;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package io.split.inputValidation;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.HashSet;
import java.util.List;
import java.util.regex.Pattern;

public final class FlagSetsValidator {

private static final String FLAG_SET_REGEX = "^[a-z0-9][_a-z0-9]{0,49}$";
private static final Logger _log = LoggerFactory.getLogger(FlagSetsValidator.class);

private FlagSetsValidator() {
throw new IllegalStateException("Utility class");
}

public static HashSet<String> cleanup(List<String> flagSets) {
if (flagSets == null || flagSets.isEmpty()) {
return new HashSet<>();
}
HashSet<String> cleanFlagSets = new HashSet<>();
for (String flagSet: flagSets) {
if(flagSet != flagSet.toLowerCase()) {
_log.warn(String.format("Flag Set name %s should be all lowercase - converting string to lowercase", flagSet));
flagSet = flagSet.toLowerCase();
}
if (flagSet.trim() != flagSet) {
_log.warn(String.format("Flag Set name %s has extra whitespace, trimming", flagSet));
flagSet = flagSet.trim();
}
if (!Pattern.matches(FLAG_SET_REGEX, flagSet)) {
_log.warn(String.format("you passed %s, Flag Set must adhere to the regular expressions %s. This means an Flag Set must be " +
"start with a letter, be in lowercase, alphanumeric and have a max length of 50 characters. %s was discarded.",
flagSet, FLAG_SET_REGEX, flagSet));
continue;
}
cleanFlagSets.add(flagSet);
}
return cleanFlagSets;
}

public static FlagSetsValidResult areValid(List<String> flagSets) {
HashSet<String> cleanFlagSets = cleanup(flagSets);

return new FlagSetsValidResult(cleanFlagSets != null && cleanFlagSets.size() != 0, cleanFlagSets);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ public void checkDefaultRateForFeatureAndSegment() {

@Test
public void checkSetFlagSetsFilter() {
List<String> sets = Stream.of("test1", "test2").collect(Collectors.toList());
List<String> sets = Stream.of("test1", "test2", "TEST3", "test-4").collect(Collectors.toList());
SplitClientConfig config = SplitClientConfig.builder().flagSetsFilter(sets).build();
Assert.assertNotNull(config.getSetsFilter());
Assert.assertEquals(2, config.getSetsFilter().size());
Assert.assertEquals(3, config.getSetsFilter().size());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package io.split.client.utils;

import org.junit.Assert;
import org.junit.Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;

import static io.split.inputValidation.FlagSetsValidator.cleanup;
import static io.split.inputValidation.FlagSetsValidator.areValid;

public class FlagSetsValidatorTest {

@Test
public void testEmptyFlagSets() {
List<String> flagSets = new ArrayList<>();
HashSet<String> cleanFlagSets = cleanup(flagSets);
Assert.assertTrue(cleanFlagSets.isEmpty());
}

@Test
public void testUpperFlagSets() {
List<String> flagSets = new ArrayList<>();
flagSets.add("Test1");
flagSets.add("TEST2");
HashSet<String> cleanFlagSets = cleanup(flagSets);
Assert.assertTrue(cleanFlagSets.contains("test1"));
Assert.assertTrue(cleanFlagSets.contains("test2"));
}

@Test
public void testTrimFlagSets() {
List<String> flagSets = new ArrayList<>();
flagSets.add(" test1");
flagSets.add(" test2 ");
HashSet<String> cleanFlagSets = cleanup(flagSets);
Assert.assertTrue(cleanFlagSets.contains("test1"));
Assert.assertTrue(cleanFlagSets.contains("test2"));
}

@Test
public void testRegexFlagSets() {
List<String> flagSets = new ArrayList<>();
flagSets.add(" test1");
flagSets.add(" test-2 ");
HashSet<String> cleanFlagSets = cleanup(flagSets);
Assert.assertEquals(1, cleanFlagSets.size());
Assert.assertTrue(cleanFlagSets.contains("test1"));
Assert.assertFalse(cleanFlagSets.contains("test-2"));
}

@Test
public void testDuplicateFlagSets() {
List<String> flagSets = new ArrayList<>();
flagSets.add(" test1");
flagSets.add(" test1 ");
HashSet<String> cleanFlagSets = cleanup(flagSets);
Assert.assertEquals(1, cleanFlagSets.size());
Assert.assertTrue(cleanFlagSets.contains("test1"));
}

@Test
public void testIsValid(){
Assert.assertTrue(areValid(Arrays.asList(" test1 ")).getValid());
Assert.assertTrue(areValid(Arrays.asList("Test1 ")).getValid());
}

@Test
public void testIsNotValid(){
Assert.assertFalse(areValid(Arrays.asList(" test 1 ")).getValid());
Assert.assertFalse(areValid(Arrays.asList("")).getValid());
Assert.assertFalse(areValid(null).getValid());
}
}