From 24852ea3bbbce426a8ba9d678fae8f6426b3772c Mon Sep 17 00:00:00 2001 From: Bilal Al Date: Fri, 13 Sep 2024 09:26:47 -0700 Subject: [PATCH] fixed checking headers in decorator and updated tests --- .../io/split/client/RequestDecorator.java | 2 +- .../io/split/client/SplitClientConfig.java | 9 +++++++ .../ApacheRequestDecoratorTest.java | 27 +++++++++---------- .../httpmodules/okhttp/SplitConfigTests.java | 13 +++++++++ 4 files changed, 36 insertions(+), 15 deletions(-) rename client/src/test/java/io/split/client/{ => utils}/ApacheRequestDecoratorTest.java (87%) diff --git a/client/src/main/java/io/split/client/RequestDecorator.java b/client/src/main/java/io/split/client/RequestDecorator.java index 8d30e699..33059e61 100644 --- 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 index 2a4a70c3..8787c106 100644 --- 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/client/src/test/java/io/split/client/ApacheRequestDecoratorTest.java b/client/src/test/java/io/split/client/utils/ApacheRequestDecoratorTest.java similarity index 87% rename from client/src/test/java/io/split/client/ApacheRequestDecoratorTest.java rename to client/src/test/java/io/split/client/utils/ApacheRequestDecoratorTest.java index b0f0c350..5d5971bb 100644 --- a/client/src/test/java/io/split/client/ApacheRequestDecoratorTest.java +++ b/client/src/test/java/io/split/client/utils/ApacheRequestDecoratorTest.java @@ -1,6 +1,8 @@ -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; @@ -8,11 +10,6 @@ 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> 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> 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> 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> 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); } - - */ } \ No newline at end of file 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 index d4093464..e7d7f6de 100644 --- 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(); + } }