Skip to content

Commit b63ac96

Browse files
timayabi2020timayabi2020
authored
Merge pull request #1775 from microsoft/tm/fix-3xx-location-header
Ensures 3XX responses without location header do not throw
2 parents 723ce4e + 214a624 commit b63ac96

File tree

2 files changed

+130
-23
lines changed

2 files changed

+130
-23
lines changed

‎components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java‎

Lines changed: 56 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -70,17 +70,24 @@ public void setBaseUrl(@Nonnull final String baseUrl) {
7070
}
7171

7272
/**
73-
* Instantiates a new OkHttp request adapter with the provided authentication provider.
74-
* @param authenticationProvider the authentication provider to use for authenticating requests.
73+
* Instantiates a new OkHttp request adapter with the provided authentication
74+
* provider.
75+
*
76+
* @param authenticationProvider the authentication provider to use for
77+
* authenticating requests.
7578
*/
7679
public OkHttpRequestAdapter(@Nonnull final AuthenticationProvider authenticationProvider) {
7780
this(authenticationProvider, null, null, null, null);
7881
}
7982

8083
/**
81-
* Instantiates a new OkHttp request adapter with the provided authentication provider, and the parse node factory.
82-
* @param authenticationProvider the authentication provider to use for authenticating requests.
83-
* @param parseNodeFactory the parse node factory to use for parsing responses.
84+
* Instantiates a new OkHttp request adapter with the provided authentication
85+
* provider, and the parse node factory.
86+
*
87+
* @param authenticationProvider the authentication provider to use for
88+
* authenticating requests.
89+
* @param parseNodeFactory the parse node factory to use for parsing
90+
* responses.
8491
*/
8592
@SuppressWarnings("LambdaLast")
8693
public OkHttpRequestAdapter(
@@ -90,10 +97,15 @@ public OkHttpRequestAdapter(
9097
}
9198

9299
/**
93-
* Instantiates a new OkHttp request adapter with the provided authentication provider, parse node factory, and the serialization writer factory.
94-
* @param authenticationProvider the authentication provider to use for authenticating requests.
95-
* @param parseNodeFactory the parse node factory to use for parsing responses.
96-
* @param serializationWriterFactory the serialization writer factory to use for serializing requests.
100+
* Instantiates a new OkHttp request adapter with the provided authentication
101+
* provider, parse node factory, and the serialization writer factory.
102+
*
103+
* @param authenticationProvider the authentication provider to use for
104+
* authenticating requests.
105+
* @param parseNodeFactory the parse node factory to use for parsing
106+
* responses.
107+
* @param serializationWriterFactory the serialization writer factory to use for
108+
* serializing requests.
97109
*/
98110
@SuppressWarnings("LambdaLast")
99111
public OkHttpRequestAdapter(
@@ -104,11 +116,18 @@ public OkHttpRequestAdapter(
104116
}
105117

106118
/**
107-
* Instantiates a new OkHttp request adapter with the provided authentication provider, parse node factory, serialization writer factory, and the http client.
108-
* @param authenticationProvider the authentication provider to use for authenticating requests.
109-
* @param parseNodeFactory the parse node factory to use for parsing responses.
110-
* @param serializationWriterFactory the serialization writer factory to use for serializing requests.
111-
* @param client the http client to use for sending requests.
119+
* Instantiates a new OkHttp request adapter with the provided authentication
120+
* provider, parse node factory, serialization writer factory, and the http
121+
* client.
122+
*
123+
* @param authenticationProvider the authentication provider to use for
124+
* authenticating requests.
125+
* @param parseNodeFactory the parse node factory to use for parsing
126+
* responses.
127+
* @param serializationWriterFactory the serialization writer factory to use for
128+
* serializing requests.
129+
* @param client the http client to use for sending
130+
* requests.
112131
*/
113132
@SuppressWarnings("LambdaLast")
114133
public OkHttpRequestAdapter(
@@ -120,12 +139,20 @@ public OkHttpRequestAdapter(
120139
}
121140

122141
/**
123-
* Instantiates a new OkHttp request adapter with the provided authentication provider, parse node factory, serialization writer factory, http client and observability options.
124-
* @param authenticationProvider the authentication provider to use for authenticating requests.
125-
* @param parseNodeFactory the parse node factory to use for parsing responses.
126-
* @param serializationWriterFactory the serialization writer factory to use for serializing requests.
127-
* @param client the http client to use for sending requests.
128-
* @param observabilityOptions the observability options to use for sending requests.
142+
* Instantiates a new OkHttp request adapter with the provided authentication
143+
* provider, parse node factory, serialization writer factory, http client and
144+
* observability options.
145+
*
146+
* @param authenticationProvider the authentication provider to use for
147+
* authenticating requests.
148+
* @param parseNodeFactory the parse node factory to use for parsing
149+
* responses.
150+
* @param serializationWriterFactory the serialization writer factory to use for
151+
* serializing requests.
152+
* @param client the http client to use for sending
153+
* requests.
154+
* @param observabilityOptions the observability options to use for
155+
* sending requests.
129156
*/
130157
@SuppressWarnings("LambdaLast")
131158
public OkHttpRequestAdapter(
@@ -590,7 +617,10 @@ private boolean shouldReturnNull(final Response response) {
590617
return statusCode == 204;
591618
}
592619

593-
/** key used for the attribute when the error response has models mappings provided */
620+
/**
621+
* key used for the attribute when the error response has models mappings
622+
* provided
623+
*/
594624
@Nonnull public static final String errorMappingFoundAttributeName =
595625
"com.microsoft.kiota.error_mapping_found";
596626

@@ -614,6 +644,7 @@ private Response throwIfFailedResponse(
614644
final int statusCode = response.code();
615645
final ResponseHeaders responseHeaders =
616646
HeadersCompatibility.getResponseHeaders(response.headers());
647+
if (statusCode >= 300 && statusCode < 400) return response;
617648
if (errorMappings == null
618649
|| !errorMappings.containsKey(statusCodeAsString)
619650
&& !(statusCode >= 400
@@ -751,7 +782,9 @@ private String getHeaderValue(final Response response, String key) {
751782
return null;
752783
}
753784

754-
/** Key used for events when an authentication challenge is returned by the API */
785+
/**
786+
* Key used for events when an authentication challenge is returned by the API
787+
*/
755788
@Nonnull public static final String authenticateChallengedEventKey =
756789
"com.microsoft.kiota.authenticate_challenge_received";
757790

@@ -831,7 +864,7 @@ private void setBaseUrlForRequestInformation(@Nonnull final RequestInformation r
831864
* @param spanForAttributes the span for the attributes.
832865
* @return the created request instance.
833866
* @throws URISyntaxException if the URI is invalid.
834-
* @throws IOException if the URL is invalid.
867+
* @throws IOException if the URL is invalid.
835868
*/
836869
protected @Nonnull Request getRequestFromRequestInformation(
837870
@Nonnull final RequestInformation requestInfo,

‎components/http/okHttp/src/test/java/com/microsoft/kiota/http/OkHttpRequestAdapterTest.java‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,80 @@ void loggingInterceptorDoesNotDrainRequestBodyForNonMarkableStreams() throws Exc
535535
}
536536
}
537537

538+
@Test
539+
void testHandle3XXResponseWithoutLocationHeader() throws Exception {
540+
final var authenticationProviderMock = mock(AuthenticationProvider.class);
541+
authenticationProviderMock.authenticateRequest(
542+
any(RequestInformation.class), any(Map.class));
543+
final var client =
544+
getMockClient(
545+
new Response.Builder()
546+
.code(301)
547+
.message("Moved Permanently")
548+
.protocol(Protocol.HTTP_1_1)
549+
.request(new Request.Builder().url("http://localhost").build())
550+
.body(
551+
ResponseBody.create(
552+
"test".getBytes("UTF-8"),
553+
MediaType.parse("application/json")))
554+
.build());
555+
final var requestInformation =
556+
new RequestInformation() {
557+
{
558+
setUri(new URI("https://localhost"));
559+
httpMethod = HttpMethod.GET;
560+
}
561+
};
562+
final var mockEntity = creatMockEntity();
563+
final var mockParseNode = creatMockParseNode(mockEntity);
564+
final var mockFactory = creatMockParseNodeFactory(mockParseNode, "application/json");
565+
566+
final var requestAdapter =
567+
new OkHttpRequestAdapter(authenticationProviderMock, mockFactory, null, client);
568+
var nativeResponseHandler = new NativeResponseHandler();
569+
requestAdapter.send(requestInformation, null, node -> mockEntity);
570+
var nativeResponse = (Response) nativeResponseHandler.getValue();
571+
assertNull(nativeResponse);
572+
}
573+
574+
@Test
575+
void handle3XXResponseWithLocationHeader() throws Exception {
576+
final var authenticationProviderMock = mock(AuthenticationProvider.class);
577+
authenticationProviderMock.authenticateRequest(
578+
any(RequestInformation.class), any(Map.class));
579+
final var client =
580+
getMockClient(
581+
new Response.Builder()
582+
.code(301)
583+
.message("Moved Permanently")
584+
.protocol(Protocol.HTTP_1_1)
585+
.request(new Request.Builder().url("http://localhost").build())
586+
.header("Location", "https://newlocation")
587+
.body(
588+
ResponseBody.create(
589+
"test".getBytes("UTF-8"),
590+
MediaType.parse("application/json")))
591+
.build());
592+
final var requestInformation =
593+
new RequestInformation() {
594+
{
595+
setUri(new URI("https://localhost"));
596+
httpMethod = HttpMethod.GET;
597+
}
598+
};
599+
final var mockEntity = creatMockEntity();
600+
final var mockParseNode = creatMockParseNode(mockEntity);
601+
final var mockFactory = creatMockParseNodeFactory(mockParseNode, "application/json");
602+
603+
final var requestAdapter =
604+
new OkHttpRequestAdapter(authenticationProviderMock, mockFactory, null, client);
605+
var nativeResponseHandler = new NativeResponseHandler();
606+
requestAdapter.send(requestInformation, null, node -> mockEntity);
607+
// Should throw an exception
608+
var nativeResponse = (Response) nativeResponseHandler.getValue();
609+
assertNull(nativeResponse);
610+
}
611+
538612
public static OkHttpClient getMockClient(final Response response) throws IOException {
539613
final OkHttpClient mockClient = mock(OkHttpClient.class);
540614
final Call remoteCall = mock(Call.class);

0 commit comments

Comments
 (0)