fixed checking headers in decorator and updated tests

sanzmauro · Sep 13, 2024 · 24852ea · 24852ea
1 parent 84eeea5
commit 24852ea
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 15 deletions.
diff --git a/client/src/main/java/io/split/client/RequestDecorator.java b/client/src/main/java/io/split/client/RequestDecorator.java
@@ -37,7 +37,7 @@ public RequestContext decorateHeaders(RequestContext request) {
             return new RequestContext(_headerDecorator.getHeaderOverrides(request)
                     .entrySet()
                     .stream()
-                    .filter(e -> !forbiddenHeaders.contains(e.getKey()))
+                    .filter(e -> !forbiddenHeaders.contains(e.getKey().toLowerCase()))
                     .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
         } catch (Exception e) {
             throw new IllegalArgumentException(

diff --git a/client/src/main/java/io/split/client/SplitClientConfig.java b/client/src/main/java/io/split/client/SplitClientConfig.java
@@ -1085,6 +1085,13 @@ private void verifyNetworkParams() {
                 throw new IllegalStateException("_onDemandFetchMaxRetries must be > 0");
             }
         }
+
+        private void verifyAlternativeClient() {
+            if (_alternativeHTTPModule != null && _streamingEnabled) {
+                throw new IllegalArgumentException("Streaming feature is not supported with Alternative HTTP Client");
+            }
+        }
+
         public SplitClientConfig build() {
 
             verifyRates();
@@ -1095,6 +1102,8 @@ public SplitClientConfig build() {
 
             verifyNetworkParams();
 
+            verifyAlternativeClient();
+
             if (_numThreadsForSegmentFetch <= 0) {
                 throw new IllegalArgumentException("Number of threads for fetching segments MUST be greater than zero");
             }

diff --git a/...it/client/ApacheRequestDecoratorTest.java → ...ent/utils/ApacheRequestDecoratorTest.java b/...it/client/ApacheRequestDecoratorTest.java → ...ent/utils/ApacheRequestDecoratorTest.java
@@ -1,18 +1,15 @@
-package io.split.client;
+package io.split.client.utils;
 
-import io.split.client.utils.ApacheRequestDecorator;
+import io.split.client.CustomHeaderDecorator;
+import io.split.client.RequestDecorator;
+import io.split.client.dtos.RequestContext;
 import org.apache.hc.client5.http.classic.methods.HttpGet;
 import org.apache.hc.client5.http.classic.methods.HttpPost;
 import org.apache.hc.core5.http.Header;
 import org.apache.hc.core5.http.ProtocolException;
 import org.junit.Assert;
 import org.junit.Test;
 
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.core.Is.is;
-
-import io.split.client.dtos.RequestContext;
-
 import java.util.List;
 import java.util.Arrays;
 import java.util.Map;
@@ -31,7 +28,7 @@ public void testNoOp() {
         request  = (HttpGet) apacheRequestDecorator.decorate(request, requestDecorator);
         Assert.assertEquals(1, request.getHeaders().length);
     }
-/*
+
     @Test
     public void testAddCustomHeaders() throws ProtocolException {
         class MyCustomHeaders implements CustomHeaderDecorator {
@@ -47,9 +44,11 @@ public Map<String, List<String>> getHeaderOverrides(RequestContext context) {
         }
         MyCustomHeaders myHeaders = new MyCustomHeaders();
         RequestDecorator decorator = new RequestDecorator(myHeaders);
+        ApacheRequestDecorator apacheRequestDecorator = new ApacheRequestDecorator();
+
         HttpGet request = new HttpGet("http://anyhost");
         request.addHeader("first", "myfirstheader");
-        request  = (HttpGet) decorator.decorateHeaders(request);
+        request  = (HttpGet) apacheRequestDecorator.decorate(request, decorator);
 
         Assert.assertEquals(4, request.getHeaders().length);
         Assert.assertEquals("1", request.getHeader("first").getValue());
@@ -61,7 +60,7 @@ public Map<String, List<String>> getHeaderOverrides(RequestContext context) {
 
         HttpPost request2 = new HttpPost("http://anyhost");
         request2.addHeader("myheader", "value");
-        request2  = (HttpPost) decorator.decorateHeaders(request2);
+        request2  = (HttpPost) apacheRequestDecorator.decorate(request2, decorator);
         Assert.assertEquals(5, request2.getHeaders().length);
     }
 
@@ -90,8 +89,9 @@ public Map<String, List<String>> getHeaderOverrides(RequestContext context) {
         }
         MyCustomHeaders myHeaders = new MyCustomHeaders();
         RequestDecorator decorator = new RequestDecorator(myHeaders);
+        ApacheRequestDecorator apacheRequestDecorator = new ApacheRequestDecorator();
         HttpGet request = new HttpGet("http://anyhost");
-        request  = (HttpGet) decorator.decorateHeaders(request);
+        request  = (HttpGet) apacheRequestDecorator.decorate(request, decorator);
         Assert.assertEquals(1, request.getHeaders().length);
         Assert.assertEquals(null, request.getHeader("SplitSDKVersion"));
     }
@@ -107,9 +107,8 @@ public Map<String, List<String>> getHeaderOverrides(RequestContext context) {
         }
         MyCustomHeaders myHeaders = new MyCustomHeaders();
         RequestDecorator decorator = new RequestDecorator(myHeaders);
+        ApacheRequestDecorator apacheRequestDecorator = new ApacheRequestDecorator();
         HttpGet request = new HttpGet("http://anyhost");
-        request  = (HttpGet) decorator.decorateHeaders(request);
+        request  = (HttpGet) apacheRequestDecorator.decorate(request, decorator);
     }
-
- */
 }
diff --git a/okhttp-modules/src/test/java/io/split/httpmodules/okhttp/SplitConfigTests.java b/okhttp-modules/src/test/java/io/split/httpmodules/okhttp/SplitConfigTests.java
@@ -28,4 +28,17 @@ public void checkExpectedAuthScheme() {
         Assert.assertEquals(null, cfg.alternativeHTTPModule());
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void checkStreamingEnabled() {
+        SplitClientConfig cfg = SplitClientConfig.builder()
+                .alternativeHTTPModule(OkHttpModule.builder()
+                        .proxyAuthScheme(ProxyAuthScheme.KERBEROS)
+                        .proxyAuthKerberosPrincipalName("bilal@bilal")
+                        .proxyHost("some-proxy")
+                        .proxyPort(3128)
+                        .debugEnabled()
+                        .build())
+                .streamingEnabled(true)
+                .build();
+    }
 }