From c6bf8a6db6b88754121ec60399ae391c2a15d232 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Thu, 22 May 2025 17:18:33 +0100 Subject: [PATCH 1/2] Fix - Ensure redirects are handled by middleware.RedirectHandler --- .../http/okHttp/gradle/dependencies.gradle | 1 + .../kiota/http/KiotaClientFactory.java | 1 + .../http/middleware/RedirectHandler.java | 9 +-- .../kiota/http/KiotaClientFactoryTest.java | 6 ++ .../http/middleware/RedirectHandlerTests.java | 62 +++++++++++++++++++ 5 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java diff --git a/components/http/okHttp/gradle/dependencies.gradle b/components/http/okHttp/gradle/dependencies.gradle index a5535a1b0..321bfbf7d 100644 --- a/components/http/okHttp/gradle/dependencies.gradle +++ b/components/http/okHttp/gradle/dependencies.gradle @@ -5,6 +5,7 @@ dependencies { testImplementation 'org.mockito:mockito-core:5.18.0' testImplementation 'com.squareup.okhttp3:logging-interceptor:4.12.0' + testImplementation 'com.squareup.okhttp3:mockwebserver:4.12.0' // This dependency is used internally, and not exposed to consumers on their own compile classpath. diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java index 7c72a868f..bacf03943 100644 --- a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java +++ b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java @@ -58,6 +58,7 @@ private KiotaClientFactory() {} Objects.requireNonNull(interceptors, "parameter interceptors cannot be null"); final OkHttpClient.Builder builder = new OkHttpClient.Builder() + .followRedirects(false) // Redirects handled by .middleware.RedirectHandler rather than okhttp .connectTimeout(Duration.ofSeconds(100)) .readTimeout(Duration.ofSeconds(100)) .callTimeout( diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java index 52ff30273..dd15c5e45 100644 --- a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java +++ b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java @@ -173,10 +173,11 @@ Request getRedirect(final Request request, final Response userResponse) request, "RedirectHandler_Intercept - redirect " + requestsCount, span); - redirectSpan.setAttribute( - "com.microsoft.kiota.handler.redirect.count", requestsCount); - redirectSpan.setAttribute("http.status_code", response.code()); - redirectSpan.end(); + if(redirectSpan != null) { + redirectSpan.setAttribute("com.microsoft.kiota.handler.redirect.count", requestsCount); + redirectSpan.setAttribute("http.status_code", response.code()); + redirectSpan.end(); + } } } while (shouldRedirect); } finally { diff --git a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/KiotaClientFactoryTest.java b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/KiotaClientFactoryTest.java index a6c300549..37800c4dd 100644 --- a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/KiotaClientFactoryTest.java +++ b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/KiotaClientFactoryTest.java @@ -153,6 +153,12 @@ void testCreateWithAuthProviderAndRequestOptions() throws IOException { } } + @Test + void upstreamRedirectHandlingDisabledByDefault() { + OkHttpClient client = KiotaClientFactory.create().build(); + assertFalse(client.followRedirects()); + } + private static RetryHandler getDisabledRetryHandler() { RetryHandlerOption retryHandlerOption = new RetryHandlerOption((delay, executionCount, request, response) -> false, 0, 0); diff --git a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java new file mode 100644 index 000000000..a85c535bb --- /dev/null +++ b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java @@ -0,0 +1,62 @@ +package com.microsoft.kiota.http.middleware; + +import com.microsoft.kiota.http.KiotaClientFactory; +import com.microsoft.kiota.http.middleware.options.RedirectHandlerOption; +import okhttp3.*; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + + +@SuppressWarnings("resource") +public class RedirectHandlerTests { + + @Test + void redirectsAreFollowedByDefault() throws Exception { + var server = new MockWebServer(); + server.enqueue(new MockResponse() + .setResponseCode(301) + .setHeader("Location", server.url("/bar")) + ); + server.enqueue(new MockResponse() + .setResponseCode(201) + ); + + var interceptors = new Interceptor[] { new RedirectHandler() }; + + final OkHttpClient client = KiotaClientFactory.create(interceptors).build(); + final Request request = new Request.Builder().url(server.url("/foo")).build(); + + // ACT + var response = client.newCall(request).execute(); + + server.takeRequest(); // discard first request + var request2 = server.takeRequest(); + + assertEquals("/bar", request2.getPath()); + assertEquals(201, response.code()); + } + + @Test + void redirectsCanBeDisabled() throws Exception { + var server = new MockWebServer(); + server.enqueue(new MockResponse() + .setResponseCode(301) + .setHeader("Location", server.url("/bar")) + ); + + var ignoreRedirectsOption = new RedirectHandlerOption(0, response -> false); + var redirectHandler = new RedirectHandler(ignoreRedirectsOption); + var interceptors = new Interceptor[] { redirectHandler }; + + final OkHttpClient client = KiotaClientFactory.create(interceptors).build(); + final Request request = new Request.Builder().url(server.url("/foo")).build(); + + // ACT + var response = client.newCall(request).execute(); + + assertEquals(301, response.code()); + } +} From 3f16ee929507b082722661fe7b38a6564ca5a29b Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Fri, 13 Jun 2025 13:48:29 +0100 Subject: [PATCH 2/2] Fix style violations --- .../kiota/http/KiotaClientFactory.java | 4 ++- .../http/middleware/RedirectHandler.java | 5 ++-- .../http/middleware/RedirectHandlerTests.java | 27 ++++++++----------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java index bacf03943..5f36334f2 100644 --- a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java +++ b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/KiotaClientFactory.java @@ -58,7 +58,9 @@ private KiotaClientFactory() {} Objects.requireNonNull(interceptors, "parameter interceptors cannot be null"); final OkHttpClient.Builder builder = new OkHttpClient.Builder() - .followRedirects(false) // Redirects handled by .middleware.RedirectHandler rather than okhttp + .followRedirects( + false) // Redirects handled by .middleware.RedirectHandler rather + // than okhttp .connectTimeout(Duration.ofSeconds(100)) .readTimeout(Duration.ofSeconds(100)) .callTimeout( diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java index dd15c5e45..4cc2d4c9a 100644 --- a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java +++ b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/RedirectHandler.java @@ -173,8 +173,9 @@ Request getRedirect(final Request request, final Response userResponse) request, "RedirectHandler_Intercept - redirect " + requestsCount, span); - if(redirectSpan != null) { - redirectSpan.setAttribute("com.microsoft.kiota.handler.redirect.count", requestsCount); + if (redirectSpan != null) { + redirectSpan.setAttribute( + "com.microsoft.kiota.handler.redirect.count", requestsCount); redirectSpan.setAttribute("http.status_code", response.code()); redirectSpan.end(); } diff --git a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java index a85c535bb..e72a3fafc 100644 --- a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java +++ b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/middleware/RedirectHandlerTests.java @@ -1,14 +1,15 @@ package com.microsoft.kiota.http.middleware; +import static org.junit.jupiter.api.Assertions.assertEquals; + import com.microsoft.kiota.http.KiotaClientFactory; import com.microsoft.kiota.http.middleware.options.RedirectHandlerOption; + import okhttp3.*; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.assertEquals; +import org.junit.jupiter.api.Test; @SuppressWarnings("resource") public class RedirectHandlerTests { @@ -16,15 +17,11 @@ public class RedirectHandlerTests { @Test void redirectsAreFollowedByDefault() throws Exception { var server = new MockWebServer(); - server.enqueue(new MockResponse() - .setResponseCode(301) - .setHeader("Location", server.url("/bar")) - ); - server.enqueue(new MockResponse() - .setResponseCode(201) - ); + server.enqueue( + new MockResponse().setResponseCode(301).setHeader("Location", server.url("/bar"))); + server.enqueue(new MockResponse().setResponseCode(201)); - var interceptors = new Interceptor[] { new RedirectHandler() }; + var interceptors = new Interceptor[] {new RedirectHandler()}; final OkHttpClient client = KiotaClientFactory.create(interceptors).build(); final Request request = new Request.Builder().url(server.url("/foo")).build(); @@ -42,14 +39,12 @@ void redirectsAreFollowedByDefault() throws Exception { @Test void redirectsCanBeDisabled() throws Exception { var server = new MockWebServer(); - server.enqueue(new MockResponse() - .setResponseCode(301) - .setHeader("Location", server.url("/bar")) - ); + server.enqueue( + new MockResponse().setResponseCode(301).setHeader("Location", server.url("/bar"))); var ignoreRedirectsOption = new RedirectHandlerOption(0, response -> false); var redirectHandler = new RedirectHandler(ignoreRedirectsOption); - var interceptors = new Interceptor[] { redirectHandler }; + var interceptors = new Interceptor[] {redirectHandler}; final OkHttpClient client = KiotaClientFactory.create(interceptors).build(); final Request request = new Request.Builder().url(server.url("/foo")).build();