Skip to content

Commit 77ad4a1

Browse files
chillaqchillaq
authored
Merge pull request #488 from splitio/custom-auth-lowercase-check
Converted reserved headers to lower to have case insesitive check, an…
2 parents 43489fe + 4c33372 commit 77ad4a1

File tree

12 files changed

+133
-66
lines changed

12 files changed

+133
-66
lines changed

‎client/src/main/java/io/split/client/HttpSegmentChangeFetcher.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ private HttpSegmentChangeFetcher(SplitHttpClient client, URI uri, TelemetryRunti
5151
public SegmentChange fetch(String segmentName, long since, FetchOptions options) {
5252
long start = System.currentTimeMillis();
5353

54-
SplitHttpResponse response = null;
54+
SplitHttpResponse response;
5555

5656
try {
5757
String path = _target.getPath() + "/" + segmentName;
@@ -65,16 +65,15 @@ public SegmentChange fetch(String segmentName, long since, FetchOptions options)
6565
URI uri = uriBuilder.build();
6666

6767
response = _client.get(uri, options);
68-
int statusCode = response.statusCode;
6968

70-
if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
71-
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.SEGMENT_SYNC, statusCode);
72-
if (statusCode == HttpStatus.SC_FORBIDDEN) {
69+
if (response.statusCode < HttpStatus.SC_OK || response.statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
70+
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.SEGMENT_SYNC, response.statusCode);
71+
if (response.statusCode == HttpStatus.SC_FORBIDDEN) {
7372
_log.error("factory instantiation: you passed a client side type sdkKey, " +
7473
"please grab an sdk key from the Split user interface that is of type server side");
7574
}
7675
throw new IllegalStateException(String.format("Could not retrieve segment changes for %s, since %s; http return code %s",
77-
segmentName, since, statusCode));
76+
segmentName, since, response.statusCode));
7877
}
7978
_telemetryRuntimeProducer.recordSuccessfulSync(LastSynchronizationRecordsEnum.SEGMENTS, System.currentTimeMillis());
8079

@@ -85,8 +84,6 @@ public SegmentChange fetch(String segmentName, long since, FetchOptions options)
8584
} finally {
8685
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.SEGMENTS, System.currentTimeMillis()-start);
8786
}
88-
89-
9087
}
9188

9289
@VisibleForTesting

‎client/src/main/java/io/split/client/HttpSplitChangeFetcher.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,15 @@ public SplitChange fetch(long since, FetchOptions options) {
7070
URI uri = uriBuilder.build();
7171
response = _client.get(uri, options);
7272

73-
int statusCode = response.statusCode;
74-
75-
if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
76-
if (statusCode == HttpStatus.SC_REQUEST_URI_TOO_LONG) {
73+
if (response.statusCode < HttpStatus.SC_OK || response.statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
74+
if (response.statusCode == HttpStatus.SC_REQUEST_URI_TOO_LONG) {
7775
_log.error("The amount of flag sets provided are big causing uri length error.");
78-
throw new UriTooLongException(String.format("Status code: %s. Message: %s", statusCode, response.statusMessage));
76+
throw new UriTooLongException(String.format("Status code: %s. Message: %s", response.statusCode, response.statusMessage));
7977
}
80-
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.SPLIT_SYNC, statusCode);
81-
throw new IllegalStateException(String.format("Could not retrieve splitChanges since %s; http return code %s", since, statusCode));
78+
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.SPLIT_SYNC, response.statusCode);
79+
throw new IllegalStateException(
80+
String.format("Could not retrieve splitChanges since %s; http return code %s", since, response.statusCode)
81+
);
8282
}
8383

8484
return Json.fromJson(response.body, SplitChange.class);

‎client/src/main/java/io/split/client/RequestDecorator.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,18 @@ public final class RequestDecorator {
1919
UserCustomHeaderDecorator _headerDecorator;
2020

2121
private static final Set<String> forbiddenHeaders = new HashSet<>(Arrays.asList(
22-
"SplitSDKVersion",
23-
"SplitMachineIp",
24-
"SplitMachineName",
25-
"SplitImpressionsMode",
26-
"Host",
27-
"Referrer",
28-
"Content-Type",
29-
"Content-Length",
30-
"Content-Encoding",
31-
"Accept",
32-
"Keep-Alive",
33-
"X-Fastly-Debug"
22+
"splitsdkversion",
23+
"splitmachineip",
24+
"splitmachinename",
25+
"splitimpressionsmode",
26+
"host",
27+
"referrer",
28+
"content-type",
29+
"content-length",
30+
"content-encoding",
31+
"accept",
32+
"keep-alive",
33+
"x-fastly-debug"
3434
));
3535

3636
public RequestDecorator(UserCustomHeaderDecorator headerDecorator) {
@@ -55,6 +55,6 @@ public HttpUriRequestBase decorateHeaders(HttpUriRequestBase request) {
5555
}
5656

5757
private boolean isHeaderAllowed(String headerName) {
58-
return !forbiddenHeaders.contains(headerName);
58+
return !forbiddenHeaders.contains(headerName.toLowerCase());
5959
}
6060
}

‎client/src/main/java/io/split/client/SplitFactoryImpl.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ public class SplitFactoryImpl implements SplitFactory {
155155
private final SplitSynchronizationTask _splitSynchronizationTask;
156156
private final EventsTask _eventsTask;
157157
private final SyncManager _syncManager;
158-
private final CloseableHttpClient _httpclient;
159158
private final SplitHttpClient _splitHttpClient;
160159
private final UserStorageWrapper _userStorageWrapper;
161160
private final ImpressionsSender _impressionsSender;
@@ -186,9 +185,7 @@ public SplitFactoryImpl(String apiToken, SplitClientConfig config) throws URISyn
186185
_gates = new SDKReadinessGates();
187186

188187
// HttpClient
189-
_httpclient = buildHttpClient(apiToken, config, _sdkMetadata);
190-
UserCustomHeaderDecorator _userCustomHeaderDecorator = config.userCustomHeaderDecorator();
191-
_splitHttpClient = SplitHttpClientImpl.create(_httpclient, new RequestDecorator(_userCustomHeaderDecorator));
188+
_splitHttpClient = buildSplitHttpClient(apiToken, config, _sdkMetadata);
192189

193190
// Roots
194191
_rootTarget = URI.create(config.endpoint());
@@ -279,7 +276,6 @@ protected SplitFactoryImpl(String apiToken, SplitClientConfig config, CustomStor
279276
_splitFetcher = null;
280277
_splitSynchronizationTask = null;
281278
_eventsTask = null;
282-
_httpclient = null;
283279
_splitHttpClient = null;
284280
_rootTarget = null;
285281
_eventsRootTarget = null;
@@ -361,7 +357,6 @@ protected SplitFactoryImpl(SplitClientConfig config) {
361357
_telemetrySynchronizer = null;
362358
_telemetrySyncTask = null;
363359
_eventsTask = null;
364-
_httpclient = null;
365360
_splitHttpClient = null;
366361
_impressionsSender = null;
367362
_rootTarget = null;
@@ -478,7 +473,8 @@ public boolean isDestroyed() {
478473
return isTerminated;
479474
}
480475

481-
private static CloseableHttpClient buildHttpClient(String apiToken, SplitClientConfig config, SDKMetadata sdkMetadata) {
476+
private static SplitHttpClient buildSplitHttpClient(String apiToken, SplitClientConfig config, SDKMetadata sdkMetadata)
477+
throws URISyntaxException {
482478
SSLConnectionSocketFactory sslSocketFactory = SSLConnectionSocketFactoryBuilder.create()
483479
.setSslContext(SSLContexts.createSystemDefault())
484480
.setTlsVersions(TLS.V_1_1, TLS.V_1_2)
@@ -512,7 +508,7 @@ private static CloseableHttpClient buildHttpClient(String apiToken, SplitClientC
512508
httpClientbuilder = setupProxy(httpClientbuilder, config);
513509
}
514510

515-
return httpClientbuilder.build();
511+
return SplitHttpClientImpl.create(httpClientbuilder.build(), new RequestDecorator(config.userCustomHeaderDecorator()));
516512
}
517513

518514
private static CloseableHttpClient buildSSEdHttpClient(String apiToken, SplitClientConfig config, SDKMetadata sdkMetadata) {

‎client/src/main/java/io/split/client/impressions/HttpImpressionsSender.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,8 @@ public void postImpressionsBulk(List<TestImpressions> impressions) {
7171
additionalHeader.put(IMPRESSIONS_MODE_HEADER, _mode.toString());
7272
response = _client.post(_impressionBulkTarget, entity, additionalHeader);
7373

74-
int status = response.statusCode;
75-
76-
if (status < HttpStatus.SC_OK || status >= HttpStatus.SC_MULTIPLE_CHOICES) {
77-
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.IMPRESSION_SYNC, status);
74+
if (response.statusCode < HttpStatus.SC_OK || response.statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
75+
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.IMPRESSION_SYNC, response.statusCode);
7876
}
7977
_telemetryRuntimeProducer.recordSuccessfulSync(LastSynchronizationRecordsEnum.IMPRESSIONS, System.currentTimeMillis());
8078

@@ -98,9 +96,8 @@ public void postCounters(HashMap<ImpressionCounter.Key, Integer> raw) {
9896
Utils.toJsonEntity(ImpressionCount.fromImpressionCounterData(raw)),
9997
null);
10098

101-
int status = response.statusCode;
102-
if (status < HttpStatus.SC_OK || status >= HttpStatus.SC_MULTIPLE_CHOICES) {
103-
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.IMPRESSION_COUNT_SYNC, status);
99+
if (response.statusCode < HttpStatus.SC_OK || response.statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
100+
_telemetryRuntimeProducer.recordSyncError(ResourceEnum.IMPRESSION_COUNT_SYNC, response.statusCode);
104101
}
105102
_telemetryRuntimeProducer.recordSyncLatency(HTTPLatenciesEnum.IMPRESSIONS_COUNT, System.currentTimeMillis() - initTime);
106103
_telemetryRuntimeProducer.recordSuccessfulSync(LastSynchronizationRecordsEnum.IMPRESSIONS_COUNT, System.currentTimeMillis());

‎client/src/main/java/io/split/service/HttpPostImp.java‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import org.apache.hc.core5.http.HttpStatus;
99
import org.slf4j.Logger;
1010
import org.slf4j.LoggerFactory;
11-
import org.apache.hc.client5.http.classic.methods.HttpPost;
1211

1312
import java.net.URI;
1413

@@ -28,18 +27,10 @@ public void post(URI uri, Object object, String posted, HttpParamsWrapper httpPa
2827
long initTime = System.currentTimeMillis();
2928
HttpEntity entity = Utils.toJsonEntity(object);
3029

31-
HttpPost request = new HttpPost(uri);
32-
request.setEntity(entity);
33-
34-
if (_logger.isDebugEnabled()) {
35-
_logger.debug(String.format("[%s] %s", request.getMethod(), uri));
36-
}
37-
3830
try {
3931
SplitHttpResponse response = _client.post(uri, entity, null);
40-
int status = response.statusCode;
41-
if (status < HttpStatus.SC_OK || status >= HttpStatus.SC_MULTIPLE_CHOICES) {
42-
_telemetryRuntimeProducer.recordSyncError(httpParamsWrapper.getResourceEnum(), status);
32+
if (response.statusCode < HttpStatus.SC_OK || response.statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
33+
_telemetryRuntimeProducer.recordSyncError(httpParamsWrapper.getResourceEnum(), response.statusCode);
4334
return;
4435
}
4536
_telemetryRuntimeProducer.recordSyncLatency(httpParamsWrapper.getHttpLatenciesEnum(), System.currentTimeMillis() - initTime);

‎client/src/main/java/io/split/service/SplitHttpClientImpl.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,17 @@ public SplitHttpResponse get(URI uri, FetchOptions options) {
5252

5353
response = _client.execute(request);
5454

55-
int statusCode = response.getCode();
56-
5755
if (_log.isDebugEnabled()) {
58-
_log.debug(String.format("[%s] %s. Status code: %s", request.getMethod(), uri.toURL(), statusCode));
56+
_log.debug(String.format("[%s] %s. Status code: %s", request.getMethod(), uri.toURL(), response.getCode()));
5957
}
6058

6159
SplitHttpResponse httpResponse = new SplitHttpResponse();
6260
httpResponse.statusMessage = "";
63-
if (statusCode < HttpStatus.SC_OK || statusCode >= HttpStatus.SC_MULTIPLE_CHOICES) {
64-
_log.warn(String.format("Response status was: %s. Reason: %s", statusCode , response.getReasonPhrase()));
61+
if (response.getCode() < HttpStatus.SC_OK || response.getCode() >= HttpStatus.SC_MULTIPLE_CHOICES) {
62+
_log.warn(String.format("Response status was: %s. Reason: %s", response.getCode() , response.getReasonPhrase()));
6563
httpResponse.statusMessage = response.getReasonPhrase();
6664
}
67-
httpResponse.statusCode = statusCode;
65+
httpResponse.statusCode = response.getCode();
6866
httpResponse.body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
6967
return httpResponse;
7068
} catch (Exception e) {

‎client/src/test/java/io/split/client/HttpSegmentChangeFetcherTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,26 @@ public void testFetcherWithCDNBypassOption() throws IOException, URISyntaxExcept
116116
Assert.assertFalse(captured.get(1).getUri().toString().contains("till="));
117117
}
118118

119+
@Test(expected = IllegalStateException.class)
120+
public void testFetcherWithError() throws IOException, URISyntaxException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
121+
URI rootTarget = URI.create("https://api.split.io");
122+
123+
HttpEntity entityMock = Mockito.mock(HttpEntity.class);
124+
when(entityMock.getContent()).thenReturn(new StringBufferInputStream("{\"till\": 1}"));
125+
ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
126+
when(response.getCode()).thenReturn(400);
127+
when(response.getEntity()).thenReturn(entityMock);
128+
when(response.getHeaders()).thenReturn(new Header[0]);
129+
130+
ArgumentCaptor<ClassicHttpRequest> requestCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class);
131+
CloseableHttpClient httpClientMock = Mockito.mock(CloseableHttpClient.class);
132+
SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, new RequestDecorator(null));
133+
134+
when(httpClientMock.execute(requestCaptor.capture())).thenReturn(TestHelper.classicResponseToCloseableMock(response));
135+
136+
Metrics.NoopMetrics metrics = new Metrics.NoopMetrics();
137+
HttpSegmentChangeFetcher fetcher = HttpSegmentChangeFetcher.create(splitHtpClient, rootTarget, Mockito.mock(TelemetryStorage.class));
138+
139+
fetcher.fetch("someSegment", -1, new FetchOptions.Builder().build());
140+
}
119141
}

‎client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,9 @@
2828
import java.net.URI;
2929
import java.net.URISyntaxException;
3030
import java.nio.charset.StandardCharsets;
31-
import java.util.HashSet;
32-
import java.util.List;
33-
import java.util.Map;
34-
import java.util.Set;
31+
import java.sql.Array;
32+
import java.util.*;
33+
import java.util.stream.Collectors;
3534

3635
import static org.mockito.Mockito.when;
3736

@@ -143,4 +142,30 @@ public void testRandomNumberGeneration() throws URISyntaxException {
143142

144143
Assert.assertTrue(seen.size() >= (total * 0.9999));
145144
}
145+
146+
@Test(expected = IllegalStateException.class)
147+
public void testURLTooLong() throws IOException, URISyntaxException, IllegalAccessException, NoSuchMethodException, InvocationTargetException {
148+
URI rootTarget = URI.create("https://api.split.io");
149+
150+
HttpEntity entityMock = Mockito.mock(HttpEntity.class);
151+
when(entityMock.getContent()).thenReturn(new ByteArrayInputStream("{\"till\": 1}".getBytes(StandardCharsets.UTF_8)));
152+
ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);
153+
when(response.getCode()).thenReturn(414);
154+
when(response.getEntity()).thenReturn(entityMock);
155+
when(response.getHeaders()).thenReturn(new Header[0]);
156+
CloseableHttpClient httpClientMock = Mockito.mock(CloseableHttpClient.class);
157+
ArgumentCaptor<ClassicHttpRequest> requestCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class);
158+
when(httpClientMock.execute(requestCaptor.capture())).thenReturn(TestHelper.classicResponseToCloseableMock(response));
159+
160+
SplitHttpClient splitHtpClient = SplitHttpClientImpl.create(httpClientMock, new RequestDecorator(null));
161+
HttpSplitChangeFetcher fetcher = HttpSplitChangeFetcher.create(splitHtpClient, rootTarget, Mockito.mock(TelemetryRuntimeProducer.class));
162+
List<String> sets = new ArrayList<String>();
163+
for (Integer i=0; i<100; i++) {
164+
sets.add("set" + i.toString());
165+
}
166+
String result = sets.stream()
167+
.map(n -> String.valueOf(n))
168+
.collect(Collectors.joining(",", "", ""));
169+
fetcher.fetch(-1, new FetchOptions.Builder().flagSetsFilter(result).cacheControlHeaders(false).build());
170+
}
146171
}

‎client/src/test/java/io/split/client/RequestDecoratorTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,18 @@ public Map<String, String> getHeaderOverrides() {
6161
{{
6262
put("first", "1");
6363
put("SplitSDKVersion", "2.4");
64+
put("SplitMachineip", "xx");
65+
put("splitMachineName", "xx");
66+
put("splitimpressionsmode", "xx");
67+
put("HOST", "xx");
68+
put("referrer", "xx");
69+
put("content-type", "xx");
70+
put("content-length", "xx");
71+
put("content-encoding", "xx");
72+
put("ACCEPT", "xx");
73+
put("keep-alive", "xx");
74+
put("x-fastly-debug", "xx");
75+
6476
}};
6577
}
6678
}

0 commit comments

Comments
 (0)