From ed319bb48e075fac2261de08256d4c05f61a304d Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 21 Aug 2024 15:08:52 -0400 Subject: [PATCH 1/5] request body is one shot Signed-off-by: Vincent Biret --- .../java/com/microsoft/kiota/http/OkHttpRequestAdapter.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java index 602f64567..aecfd6682 100644 --- a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java +++ b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java @@ -899,6 +899,11 @@ public MediaType contentType() { } } + @Override + public boolean isOneShot() { + return true; + } + @Override public long contentLength() throws IOException { final Set contentLength = From e4dd84e3f6fb08b5ff149530b1f20bde90584b37 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Wed, 21 Aug 2024 22:30:02 +0300 Subject: [PATCH 2/5] Update isOneShot check and reset stream if possible --- .../java/com/microsoft/kiota/http/OkHttpRequestAdapter.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java index aecfd6682..028afe64c 100644 --- a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java +++ b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java @@ -901,7 +901,7 @@ public MediaType contentType() { @Override public boolean isOneShot() { - return true; + return !requestInfo.content.markSupported(); } @Override @@ -928,6 +928,9 @@ public long contentLength() throws IOException { @Override public void writeTo(@Nonnull BufferedSink sink) throws IOException { sink.writeAll(Okio.source(requestInfo.content)); + if (!isOneShot()) { + requestInfo.content.reset(); + } } }; From 6c2d17010f004fd8a22842780e8e457352b349dd Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 22 Aug 2024 14:37:52 +0300 Subject: [PATCH 3/5] fix: Ensure interceptors don't drain request body stream before network call --- .../http/okHttp/gradle/dependencies.gradle | 1 + .../kiota/http/OkHttpRequestAdapterTest.java | 163 ++++++++++++++++++ 2 files changed, 164 insertions(+) diff --git a/components/http/okHttp/gradle/dependencies.gradle b/components/http/okHttp/gradle/dependencies.gradle index 8c385fe48..11e93b50c 100644 --- a/components/http/okHttp/gradle/dependencies.gradle +++ b/components/http/okHttp/gradle/dependencies.gradle @@ -6,6 +6,7 @@ dependencies { // Use JUnit Jupiter Engine for testing. testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine' testImplementation 'org.mockito:mockito-inline:5.2.0' + testImplementation 'com.squareup.okhttp3:logging-interceptor: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/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java index 0c11ade28..393e24625 100644 --- a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java +++ b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java @@ -6,6 +6,7 @@ import com.microsoft.kiota.ApiException; import com.microsoft.kiota.HttpMethod; +import com.microsoft.kiota.NativeResponseHandler; import com.microsoft.kiota.RequestInformation; import com.microsoft.kiota.authentication.AuthenticationProvider; import com.microsoft.kiota.serialization.Parsable; @@ -19,13 +20,17 @@ import okhttp3.Call; import okhttp3.Callback; import okhttp3.Dispatcher; +import okhttp3.Interceptor; import okhttp3.MediaType; import okhttp3.OkHttpClient; import okhttp3.Protocol; import okhttp3.Request; import okhttp3.Response; import okhttp3.ResponseBody; +import okhttp3.logging.HttpLoggingInterceptor; +import okhttp3.logging.HttpLoggingInterceptor.Level; +import okio.Buffer; import okio.Okio; import org.junit.jupiter.api.Test; @@ -401,6 +406,135 @@ void getRequestFromRequestInformationWithoutContentLengthOverrideWithEmptyPayloa assertEquals(0, request.body().contentLength()); } + @Test + void buildsNativeRequestSupportingMultipleWrites() throws Exception { + final var authenticationProviderMock = mock(AuthenticationProvider.class); + final var requestInformation = new RequestInformation(); + requestInformation.setUri(new URI("https://localhost")); + var requestBodyJson = "{\"name\":\"value\",\"array\":[\"1\",\"2\",\"3\"]}"; + ByteArrayInputStream content = + new ByteArrayInputStream(requestBodyJson.getBytes(StandardCharsets.UTF_8)); + requestInformation.setStreamContent(content, "application/json"); + requestInformation.httpMethod = HttpMethod.PUT; + + final var adapter = new OkHttpRequestAdapter(authenticationProviderMock); + final var request = + adapter.getRequestFromRequestInformation( + requestInformation, mock(Span.class), mock(Span.class)); + + final var requestBody = request.body(); + assertNotNull(requestBody); + var buffer = new Buffer(); + requestBody.writeTo(buffer); + assertEquals(requestBodyJson, buffer.readUtf8()); + + // Second write to the buffer to ensure the body is not consumed + buffer = new Buffer(); + requestBody.writeTo(buffer); + assertEquals(requestBodyJson, buffer.readUtf8()); + } + + @Test + void buildsNativeRequestSupportingOneShotWrite() throws Exception { + final var authenticationProviderMock = mock(AuthenticationProvider.class); + final var testFile = new File("./src/test/resources/helloWorld.txt"); + final var requestInformation = new RequestInformation(); + + requestInformation.setUri(new URI("https://localhost")); + requestInformation.httpMethod = HttpMethod.PUT; + final var contentLength = testFile.length(); + requestInformation.headers.add("Content-Length", String.valueOf(contentLength)); + try (FileInputStream content = new FileInputStream(testFile)) { + requestInformation.setStreamContent(content, "application/octet-stream"); + + final var adapter = new OkHttpRequestAdapter(authenticationProviderMock); + final var request = + adapter.getRequestFromRequestInformation( + requestInformation, mock(Span.class), mock(Span.class)); + + final var requestBody = request.body(); + assertNotNull(requestBody); + var buffer = new Buffer(); + requestBody.writeTo(buffer); + assertEquals(contentLength, buffer.size()); + + // Second write to the buffer to ensure the body is not consumed + buffer = new Buffer(); + requestBody.writeTo(buffer); + assertEquals(0, buffer.size()); + } + } + + @Test + void loggingInterceptorDoesNotDrainRequestBodyForMarkableStreams() throws Exception { + var loggingInterceptor = new HttpLoggingInterceptor(); + loggingInterceptor.setLevel(Level.BODY); + + var okHttpClient = + KiotaClientFactory.create() + .addInterceptor(loggingInterceptor) + .addInterceptor(new MockResponseHandler()) + .build(); + + final var authenticationProviderMock = mock(AuthenticationProvider.class); + authenticationProviderMock.authenticateRequest( + any(RequestInformation.class), any(Map.class)); + var requestAdapter = + new OkHttpRequestAdapter(authenticationProviderMock, null, null, okHttpClient); + + final var requestInformation = new RequestInformation(); + requestInformation.setUri(new URI("https://localhost")); + var requestBodyJson = "{\"name\":\"value\",\"array\":[\"1\",\"2\",\"3\"]}"; + ByteArrayInputStream content = + new ByteArrayInputStream(requestBodyJson.getBytes(StandardCharsets.UTF_8)); + requestInformation.setStreamContent(content, "application/json"); + requestInformation.httpMethod = HttpMethod.PUT; + var nativeResponseHandler = new NativeResponseHandler(); + requestInformation.setResponseHandler(nativeResponseHandler); + + var mockEntity = creatMockEntity(); + requestAdapter.send(requestInformation, null, (node) -> mockEntity); + var nativeResponse = (Response) nativeResponseHandler.getValue(); + assertNotNull(nativeResponse); + assertEquals(requestBodyJson, nativeResponse.body().source().readUtf8()); + } + + @Test + void loggingInterceptorDoesNotDrainRequestBodyForNonMarkableStreams() throws Exception { + var loggingInterceptor = new HttpLoggingInterceptor(); + loggingInterceptor.setLevel(Level.BODY); + + var okHttpClient = + KiotaClientFactory.create() + .addInterceptor(loggingInterceptor) + .addInterceptor(new MockResponseHandler()) + .build(); + + final var authenticationProviderMock = mock(AuthenticationProvider.class); + authenticationProviderMock.authenticateRequest( + any(RequestInformation.class), any(Map.class)); + var requestAdapter = + new OkHttpRequestAdapter(authenticationProviderMock, null, null, okHttpClient); + + final var requestInformation = new RequestInformation(); + requestInformation.setUri(new URI("https://localhost")); + requestInformation.httpMethod = HttpMethod.PUT; + var nativeResponseHandler = new NativeResponseHandler(); + requestInformation.setResponseHandler(nativeResponseHandler); + + final var testFile = new File("./src/test/resources/helloWorld.txt"); + final var contentLength = testFile.length(); + + try (FileInputStream content = new FileInputStream(testFile)) { + requestInformation.setStreamContent(content, "application/octet-stream"); + var mockEntity = creatMockEntity(); + requestAdapter.send(requestInformation, null, (node) -> mockEntity); + var nativeResponse = (Response) nativeResponseHandler.getValue(); + assertNotNull(nativeResponse); + assertEquals(contentLength, nativeResponse.body().source().readByteArray().length); + } + } + public static OkHttpClient getMockClient(final Response response) throws IOException { final OkHttpClient mockClient = mock(OkHttpClient.class); final Call remoteCall = mock(Call.class); @@ -440,4 +574,33 @@ public ParseNodeFactory creatMockParseNodeFactory( when(mockFactory.getValidContentType()).thenReturn(validContentType); return mockFactory; } + + // Returns request body as response body + static class MockResponseHandler implements Interceptor { + @Override + public Response intercept(Chain chain) throws IOException { + final var request = chain.request(); + final var requestBody = request.body(); + if (request != null && requestBody != null) { + final var buffer = new Buffer(); + requestBody.writeTo(buffer); + return new Response.Builder() + .code(200) + .message("OK") + .protocol(Protocol.HTTP_1_1) + .request(request) + .body( + ResponseBody.create( + buffer.readByteArray(), + MediaType.parse("application/json"))) + .build(); + } + return new Response.Builder() + .code(200) + .message("OK") + .protocol(Protocol.HTTP_1_1) + .request(request) + .build(); + } + } } From 89e921a058b83bb8d00119867d63413cc8f07d12 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 22 Aug 2024 08:12:32 -0400 Subject: [PATCH 4/5] Apply suggestions from code review --- .../com/microsoft/kiota/http/OkHttpRequestAdapterTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java index 393e24625..636a97111 100644 --- a/components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java +++ b/components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java @@ -493,7 +493,7 @@ void loggingInterceptorDoesNotDrainRequestBodyForMarkableStreams() throws Except requestInformation.setResponseHandler(nativeResponseHandler); var mockEntity = creatMockEntity(); - requestAdapter.send(requestInformation, null, (node) -> mockEntity); + requestAdapter.send(requestInformation, null, node -> mockEntity); var nativeResponse = (Response) nativeResponseHandler.getValue(); assertNotNull(nativeResponse); assertEquals(requestBodyJson, nativeResponse.body().source().readUtf8()); @@ -528,7 +528,7 @@ void loggingInterceptorDoesNotDrainRequestBodyForNonMarkableStreams() throws Exc try (FileInputStream content = new FileInputStream(testFile)) { requestInformation.setStreamContent(content, "application/octet-stream"); var mockEntity = creatMockEntity(); - requestAdapter.send(requestInformation, null, (node) -> mockEntity); + requestAdapter.send(requestInformation, null, node -> mockEntity); var nativeResponse = (Response) nativeResponseHandler.getValue(); assertNotNull(nativeResponse); assertEquals(contentLength, nativeResponse.body().source().readByteArray().length); From 2c3191fa60fe53a186a7c7a6293096c404c4759c Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 22 Aug 2024 15:26:43 +0300 Subject: [PATCH 5/5] Bump version and update CHANGELOG --- CHANGELOG.md | 6 ++++ README.md | 30 +++++++++---------- .../options/UserAgentHandlerOption.java | 2 +- gradle.properties | 2 +- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff54adb10..e126c46ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.3.0] - 2024-08-22 + +### Changed + +- Ensure interceptors don't drain request body stream before network call [#2037](https://github.com/microsoftgraph/msgraph-sdk-java/issues/2037) + ## [1.2.0] - 2024-08-09 ### Changed diff --git a/README.md b/README.md index ed03121d6..960a94716 100644 --- a/README.md +++ b/README.md @@ -21,14 +21,14 @@ Read more about Kiota [here](https://github.com/microsoft/kiota/blob/main/README In `build.gradle` in the `dependencies` section: ```Groovy -implementation 'com.microsoft.kiota:microsoft-kiota-abstractions:1.2.0' -implementation 'com.microsoft.kiota:microsoft-kiota-authentication-azure:1.2.0' -implementation 'com.microsoft.kiota:microsoft-kiota-http-okHttp:1.2.0' -implementation 'com.microsoft.kiota:microsoft-kiota-serialization-json:1.2.0' -implementation 'com.microsoft.kiota:microsoft-kiota-serialization-text:1.2.0' -implementation 'com.microsoft.kiota:microsoft-kiota-serialization-form:1.2.0' -implementation 'com.microsoft.kiota:microsoft-kiota-serialization-multipart:1.2.0' -implementation 'com.microsoft.kiota:microsoft-kiota-bundle:1.2.0' +implementation 'com.microsoft.kiota:microsoft-kiota-abstractions:1.3.0' +implementation 'com.microsoft.kiota:microsoft-kiota-authentication-azure:1.3.0' +implementation 'com.microsoft.kiota:microsoft-kiota-http-okHttp:1.3.0' +implementation 'com.microsoft.kiota:microsoft-kiota-serialization-json:1.3.0' +implementation 'com.microsoft.kiota:microsoft-kiota-serialization-text:1.3.0' +implementation 'com.microsoft.kiota:microsoft-kiota-serialization-form:1.3.0' +implementation 'com.microsoft.kiota:microsoft-kiota-serialization-multipart:1.3.0' +implementation 'com.microsoft.kiota:microsoft-kiota-bundle:1.3.0' implementation 'jakarta.annotation:jakarta.annotation-api:2.1.1' ``` @@ -40,37 +40,37 @@ In `pom.xml` in the `dependencies` section: com.microsoft.kiota microsoft-kiota-abstractions - 1.2.0 + 1.3.0 com.microsoft.kiota microsoft-kiota-authentication-azure - 1.2.0 + 1.3.0 com.microsoft.kiota microsoft-kiota-http-okHttp - 1.2.0 + 1.3.0 com.microsoft.kiota microsoft-kiota-serialization-json - 1.2.0 + 1.3.0 com.microsoft.kiota microsoft-kiota-serialization-text - 1.2.0 + 1.3.0 com.microsoft.kiota microsoft-kiota-serialization-form - 1.2.0 + 1.3.0 com.microsoft.kiota microsoft-kiota-serialization-multipart - 1.2.0 + 1.3.0 jakarta.annotation diff --git a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/options/UserAgentHandlerOption.java b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/options/UserAgentHandlerOption.java index 221f75d17..2eb271beb 100644 --- a/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/options/UserAgentHandlerOption.java +++ b/components/http/okHttp/src/main/java/com/microsoft/kiota/http/middleware/options/UserAgentHandlerOption.java @@ -13,7 +13,7 @@ public UserAgentHandlerOption() {} private boolean enabled = true; @Nonnull private String productName = "kiota-java"; - @Nonnull private String productVersion = "1.2.0"; + @Nonnull private String productVersion = "1.3.0"; /** * Gets the product name to be used in the user agent header diff --git a/gradle.properties b/gradle.properties index f933e70c8..ff6dd0baf 100644 --- a/gradle.properties +++ b/gradle.properties @@ -25,7 +25,7 @@ org.gradle.caching=true mavenGroupId = com.microsoft.kiota mavenMajorVersion = 1 -mavenMinorVersion = 2 +mavenMinorVersion = 3 mavenPatchVersion = 0 mavenArtifactSuffix =