Skip to content

Commit 5aa3f26

Browse files
z4kn4feinadams85
andauthored
Accept only 200 HTTP status code / check for empty response body (#68)
* Accept only 200 HTTP status code / check for empty response body * Throw exception in case of invalid JSON * Update src/main/java/com/configcat/Utils.java Co-authored-by: adams85 <31276480+adams85@users.noreply.github.com> * Proposed changes * Potential further improvements that may help error diagnostics * Bump versions --------- Co-authored-by: adams85 <31276480+adams85@users.noreply.github.com> Co-authored-by: Adam Simon <adamosimoni@gmail.com>
1 parent ce5ca05 commit 5aa3f26

7 files changed

Lines changed: 49 additions & 9 deletions

File tree

gradle.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
version=9.4.3
1+
version=9.4.4

src/main/java/com/configcat/Config.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public class Config {
1313
@SerializedName(value = "p")
1414
private Preferences preferences;
1515
@SerializedName(value = "f")
16-
private final Map<String, Setting> entries = new HashMap<>();
16+
private Map<String, Setting> entries = new HashMap<>();
1717
@SerializedName(value = "s")
1818
private Segment[] segments;
1919

@@ -35,7 +35,9 @@ public Segment[] getSegments() {
3535
* The map of settings.
3636
*/
3737
public Map<String, Setting> getEntries() {
38-
return entries;
38+
// NOTE: Deserializing a JSON like '{ "f": null }' overwrites entries with null.
39+
// However, we want to treat null as an empty map in that case too.
40+
return entries != null ? entries : (entries = new HashMap<>());
3941
}
4042

4143
boolean isEmpty() {

src/main/java/com/configcat/ConfigFetcher.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ public void onFailure(@NotNull Call call, @NotNull IOException e) {
113113
public void onResponse(@NotNull Call call, @NotNull Response response) {
114114
try (ResponseBody body = response.body()) {
115115
String cfRayId = response.header("CF-RAY");
116-
if (response.isSuccessful() && body != null) {
117-
String content = body.string();
116+
if (response.code() == 200) {
117+
String content = body != null ? body.string() : null;
118118
String eTag = response.header("ETag");
119119
Result<Config> result = deserializeConfig(content, cfRayId);
120120
if (result.error() != null) {
@@ -190,4 +190,3 @@ private Result<Config> deserializeConfig(String json, String cfRayId) {
190190
}
191191
}
192192
}
193-

src/main/java/com/configcat/Constants.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ private Constants() { /* prevent from instantiation*/ }
77
static final long DISTANT_PAST = 0;
88
static final String CONFIG_JSON_NAME = "config_v6.json";
99
static final String SERIALIZATION_FORMAT_VERSION = "v2";
10-
static final String VERSION = "9.4.3";
10+
static final String VERSION = "9.4.4";
1111

1212
static final String SDK_KEY_PROXY_PREFIX = "configcat-proxy/";
1313
static final String SDK_KEY_PREFIX = "configcat-sdk-1";

src/main/java/com/configcat/RolloutEvaluator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public EvaluationResult evaluate(Setting setting, String key, User user, Map<Str
4545
@NotNull
4646
private EvaluationResult evaluateSetting(Setting setting, EvaluateLogger evaluateLogger, EvaluationContext context) {
4747
EvaluationResult evaluationResult = null;
48-
if (setting.getTargetingRules() != null) {
48+
if (setting.getTargetingRules() != null && setting.getTargetingRules().length > 0) {
4949
evaluationResult = evaluateTargetingRules(setting, context, evaluateLogger);
5050
}
5151
if (evaluationResult == null && setting.getPercentageOptions() != null && setting.getPercentageOptions().length > 0) {

src/main/java/com/configcat/Utils.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,17 @@ public static DecimalFormat getDecimalFormat() {
3737
}
3838

3939
public static Config deserializeConfig(String json) {
40+
if (json == null || json.isEmpty()) {
41+
throw new IllegalArgumentException("Config JSON content cannot be null or empty.");
42+
}
43+
4044
Config config = Utils.gson.fromJson(json, Config.class);
41-
String salt = config.getPreferences().getSalt();
45+
46+
if (config == null) {
47+
throw new IllegalArgumentException("Invalid config JSON content: " + json);
48+
}
49+
50+
String salt = config.getPreferences() != null ? config.getPreferences().getSalt() : null;
4251
Segment[] segments = config.getSegments();
4352
if (segments == null) {
4453
segments = new Segment[]{};

src/test/java/com/configcat/ConfigFetcherTest.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,16 @@
88
import org.junit.jupiter.api.AfterEach;
99
import org.junit.jupiter.api.BeforeEach;
1010
import org.junit.jupiter.api.Test;
11+
import org.junit.jupiter.params.ParameterizedTest;
12+
import org.junit.jupiter.params.provider.Arguments;
13+
import org.junit.jupiter.params.provider.MethodSource;
1114
import org.slf4j.Logger;
1215
import org.slf4j.LoggerFactory;
1316

1417
import java.io.IOException;
1518
import java.util.concurrent.ExecutionException;
1619
import java.util.concurrent.TimeUnit;
20+
import java.util.stream.Stream;
1721

1822
import static org.junit.jupiter.api.Assertions.*;
1923
import static org.mockito.ArgumentMatchers.anyString;
@@ -155,6 +159,32 @@ public void fetchSuccess() throws Exception {
155159
fetcher.close();
156160
}
157161

162+
private static Stream<Arguments> emptyFetchTestData() {
163+
return Stream.of(
164+
Arguments.of(""),
165+
Arguments.of("null")
166+
);
167+
}
168+
169+
@ParameterizedTest
170+
@MethodSource("emptyFetchTestData")
171+
public void fetchEmpty(String body) throws Exception {
172+
this.server.enqueue(new MockResponse().setResponseCode(200).setBody(body));
173+
174+
ConfigFetcher fetcher = new ConfigFetcher(new OkHttpClient.Builder().build(),
175+
logger,
176+
"",
177+
this.server.url("/").toString(),
178+
false,
179+
PollingModes.manualPoll().getPollingIdentifier());
180+
181+
FetchResponse response = fetcher.fetchAsync(null).get();
182+
assertFalse(response.isFetched());
183+
assertEquals("Fetching config JSON was successful but the HTTP response content was invalid.", response.error().toString());
184+
185+
fetcher.close();
186+
}
187+
158188
@Test
159189
public void testIntegration() throws IOException, ExecutionException, InterruptedException {
160190
ConfigFetcher fetch = new ConfigFetcher(new OkHttpClient.Builder()

0 commit comments

Comments
 (0)