From 214a6245cc80d1d3593d1f1098f9d37d6e700f6e Mon Sep 17 00:00:00 2001 From: Microsoft Graph DevX Tooling Date: Wed, 19 Feb 2025 00:29:39 +0300 Subject: [PATCH] fix: Ensures 3XX responses without location header do not throw Fixed violations as per PR gate recommendations Sonar recommendations Addressing sonar issues Gradle apply Updated test to cover multiple values Added another test Updated value sources Updated test Removed unnecessary test Updates Updated tests fix: Ensure 3XX responses without location header do not throw fix: Ensure 3XX responses without location header do not throw --- .../kiota/http/OkHttpRequestAdapter.java | 79 +++++++++++++------ .../kiota/http/OkHttpRequestAdapterTest.java | 74 +++++++++++++++++ 2 files changed, 130 insertions(+), 23 deletions(-) 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 8b6a2f281..43eb3761b 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 @@ -70,17 +70,24 @@ public void setBaseUrl(@Nonnull final String baseUrl) { } /** - * Instantiates a new OkHttp request adapter with the provided authentication provider. - * @param authenticationProvider the authentication provider to use for authenticating requests. + * Instantiates a new OkHttp request adapter with the provided authentication + * provider. + * + * @param authenticationProvider the authentication provider to use for + * authenticating requests. */ public OkHttpRequestAdapter(@Nonnull final AuthenticationProvider authenticationProvider) { this(authenticationProvider, null, null, null, null); } /** - * Instantiates a new OkHttp request adapter with the provided authentication provider, and the parse node factory. - * @param authenticationProvider the authentication provider to use for authenticating requests. - * @param parseNodeFactory the parse node factory to use for parsing responses. + * Instantiates a new OkHttp request adapter with the provided authentication + * provider, and the parse node factory. + * + * @param authenticationProvider the authentication provider to use for + * authenticating requests. + * @param parseNodeFactory the parse node factory to use for parsing + * responses. */ @SuppressWarnings("LambdaLast") public OkHttpRequestAdapter( @@ -90,10 +97,15 @@ public OkHttpRequestAdapter( } /** - * Instantiates a new OkHttp request adapter with the provided authentication provider, parse node factory, and the serialization writer factory. - * @param authenticationProvider the authentication provider to use for authenticating requests. - * @param parseNodeFactory the parse node factory to use for parsing responses. - * @param serializationWriterFactory the serialization writer factory to use for serializing requests. + * Instantiates a new OkHttp request adapter with the provided authentication + * provider, parse node factory, and the serialization writer factory. + * + * @param authenticationProvider the authentication provider to use for + * authenticating requests. + * @param parseNodeFactory the parse node factory to use for parsing + * responses. + * @param serializationWriterFactory the serialization writer factory to use for + * serializing requests. */ @SuppressWarnings("LambdaLast") public OkHttpRequestAdapter( @@ -104,11 +116,18 @@ public OkHttpRequestAdapter( } /** - * Instantiates a new OkHttp request adapter with the provided authentication provider, parse node factory, serialization writer factory, and the http client. - * @param authenticationProvider the authentication provider to use for authenticating requests. - * @param parseNodeFactory the parse node factory to use for parsing responses. - * @param serializationWriterFactory the serialization writer factory to use for serializing requests. - * @param client the http client to use for sending requests. + * Instantiates a new OkHttp request adapter with the provided authentication + * provider, parse node factory, serialization writer factory, and the http + * client. + * + * @param authenticationProvider the authentication provider to use for + * authenticating requests. + * @param parseNodeFactory the parse node factory to use for parsing + * responses. + * @param serializationWriterFactory the serialization writer factory to use for + * serializing requests. + * @param client the http client to use for sending + * requests. */ @SuppressWarnings("LambdaLast") public OkHttpRequestAdapter( @@ -120,12 +139,20 @@ public OkHttpRequestAdapter( } /** - * Instantiates a new OkHttp request adapter with the provided authentication provider, parse node factory, serialization writer factory, http client and observability options. - * @param authenticationProvider the authentication provider to use for authenticating requests. - * @param parseNodeFactory the parse node factory to use for parsing responses. - * @param serializationWriterFactory the serialization writer factory to use for serializing requests. - * @param client the http client to use for sending requests. - * @param observabilityOptions the observability options to use for sending requests. + * Instantiates a new OkHttp request adapter with the provided authentication + * provider, parse node factory, serialization writer factory, http client and + * observability options. + * + * @param authenticationProvider the authentication provider to use for + * authenticating requests. + * @param parseNodeFactory the parse node factory to use for parsing + * responses. + * @param serializationWriterFactory the serialization writer factory to use for + * serializing requests. + * @param client the http client to use for sending + * requests. + * @param observabilityOptions the observability options to use for + * sending requests. */ @SuppressWarnings("LambdaLast") public OkHttpRequestAdapter( @@ -590,7 +617,10 @@ private boolean shouldReturnNull(final Response response) { return statusCode == 204; } - /** key used for the attribute when the error response has models mappings provided */ + /** + * key used for the attribute when the error response has models mappings + * provided + */ @Nonnull public static final String errorMappingFoundAttributeName = "com.microsoft.kiota.error_mapping_found"; @@ -614,6 +644,7 @@ private Response throwIfFailedResponse( final int statusCode = response.code(); final ResponseHeaders responseHeaders = HeadersCompatibility.getResponseHeaders(response.headers()); + if (statusCode >= 300 && statusCode < 400) return response; if (errorMappings == null || !errorMappings.containsKey(statusCodeAsString) && !(statusCode >= 400 @@ -751,7 +782,9 @@ private String getHeaderValue(final Response response, String key) { return null; } - /** Key used for events when an authentication challenge is returned by the API */ + /** + * Key used for events when an authentication challenge is returned by the API + */ @Nonnull public static final String authenticateChallengedEventKey = "com.microsoft.kiota.authenticate_challenge_received"; @@ -831,7 +864,7 @@ private void setBaseUrlForRequestInformation(@Nonnull final RequestInformation r * @param spanForAttributes the span for the attributes. * @return the created request instance. * @throws URISyntaxException if the URI is invalid. - * @throws IOException if the URL is invalid. + * @throws IOException if the URL is invalid. */ protected @Nonnull Request getRequestFromRequestInformation( @Nonnull final RequestInformation requestInfo, 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 b14d32c84..2d9c0bd11 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 @@ -535,6 +535,80 @@ void loggingInterceptorDoesNotDrainRequestBodyForNonMarkableStreams() throws Exc } } + @Test + void testHandle3XXResponseWithoutLocationHeader() throws Exception { + final var authenticationProviderMock = mock(AuthenticationProvider.class); + authenticationProviderMock.authenticateRequest( + any(RequestInformation.class), any(Map.class)); + final var client = + getMockClient( + new Response.Builder() + .code(301) + .message("Moved Permanently") + .protocol(Protocol.HTTP_1_1) + .request(new Request.Builder().url("http://localhost").build()) + .body( + ResponseBody.create( + "test".getBytes("UTF-8"), + MediaType.parse("application/json"))) + .build()); + final var requestInformation = + new RequestInformation() { + { + setUri(new URI("https://localhost")); + httpMethod = HttpMethod.GET; + } + }; + final var mockEntity = creatMockEntity(); + final var mockParseNode = creatMockParseNode(mockEntity); + final var mockFactory = creatMockParseNodeFactory(mockParseNode, "application/json"); + + final var requestAdapter = + new OkHttpRequestAdapter(authenticationProviderMock, mockFactory, null, client); + var nativeResponseHandler = new NativeResponseHandler(); + requestAdapter.send(requestInformation, null, node -> mockEntity); + var nativeResponse = (Response) nativeResponseHandler.getValue(); + assertNull(nativeResponse); + } + + @Test + void handle3XXResponseWithLocationHeader() throws Exception { + final var authenticationProviderMock = mock(AuthenticationProvider.class); + authenticationProviderMock.authenticateRequest( + any(RequestInformation.class), any(Map.class)); + final var client = + getMockClient( + new Response.Builder() + .code(301) + .message("Moved Permanently") + .protocol(Protocol.HTTP_1_1) + .request(new Request.Builder().url("http://localhost").build()) + .header("Location", "https://newlocation") + .body( + ResponseBody.create( + "test".getBytes("UTF-8"), + MediaType.parse("application/json"))) + .build()); + final var requestInformation = + new RequestInformation() { + { + setUri(new URI("https://localhost")); + httpMethod = HttpMethod.GET; + } + }; + final var mockEntity = creatMockEntity(); + final var mockParseNode = creatMockParseNode(mockEntity); + final var mockFactory = creatMockParseNodeFactory(mockParseNode, "application/json"); + + final var requestAdapter = + new OkHttpRequestAdapter(authenticationProviderMock, mockFactory, null, client); + var nativeResponseHandler = new NativeResponseHandler(); + requestAdapter.send(requestInformation, null, node -> mockEntity); + // Should throw an exception + var nativeResponse = (Response) nativeResponseHandler.getValue(); + assertNull(nativeResponse); + } + public static OkHttpClient getMockClient(final Response response) throws IOException { final OkHttpClient mockClient = mock(OkHttpClient.class); final Call remoteCall = mock(Call.class);