Skip to content

Commit 636296d

Browse files
alzimmermsftalzimmermsft
authored
Fix OkHttp Header Case Change (Azure#18917)
Fixed issue where HTTP header cases was modified during return
1 parent dbabc71 commit 636296d

File tree

5 files changed

+158
-15
lines changed

5 files changed

+158
-15
lines changed

‎sdk/core/azure-core-http-netty/src/test/java/com/azure/core/http/netty/ReactorNettyClientResponseTransformer.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import static com.azure.core.http.netty.ReactorNettyClientTests.EXPECTED_HEADER;
1515
import static com.azure.core.http.netty.ReactorNettyClientTests.NO_DOUBLE_UA_PATH;
16+
import static com.azure.core.http.netty.ReactorNettyClientTests.RETURN_HEADERS_AS_IS_PATH;
1617

1718
/**
1819
* Mock response transformer used to test {@link NettyAsyncHttpClient}.
@@ -35,6 +36,11 @@ public Response transform(Request request, Response response, FileSource fileSou
3536
.status(400)
3637
.build();
3738
}
39+
} else if (RETURN_HEADERS_AS_IS_PATH.equalsIgnoreCase(url)) {
40+
return Response.response()
41+
.status(200)
42+
.headers(request.getHeaders())
43+
.build();
3844
}
3945

4046
return response;

‎sdk/core/azure-core-http-netty/src/test/java/com/azure/core/http/netty/ReactorNettyClientTests.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package com.azure.core.http.netty;
55

66
import com.azure.core.http.HttpClient;
7+
import com.azure.core.http.HttpHeader;
78
import com.azure.core.http.HttpHeaders;
89
import com.azure.core.http.HttpMethod;
910
import com.azure.core.http.HttpRequest;
@@ -35,7 +36,9 @@
3536
import java.net.URL;
3637
import java.nio.ByteBuffer;
3738
import java.nio.charset.StandardCharsets;
39+
import java.time.Duration;
3840
import java.util.ArrayList;
41+
import java.util.Arrays;
3942
import java.util.List;
4043
import java.util.concurrent.CountDownLatch;
4144
import java.util.stream.Stream;
@@ -45,6 +48,7 @@
4548
import static com.github.tomakehurst.wiremock.client.WireMock.post;
4649
import static java.time.Duration.ofMillis;
4750
import static org.junit.jupiter.api.Assertions.assertEquals;
51+
import static org.junit.jupiter.api.Assertions.assertLinesMatch;
4852
import static org.junit.jupiter.api.Assertions.assertNotEquals;
4953
import static org.junit.jupiter.api.Assertions.assertNotNull;
5054
import static org.junit.jupiter.api.Assertions.assertTimeout;
@@ -55,6 +59,7 @@ public class ReactorNettyClientTests {
5559

5660
static final String NO_DOUBLE_UA_PATH = "/noDoubleUA";
5761
static final String EXPECTED_HEADER = "userAgent";
62+
static final String RETURN_HEADERS_AS_IS_PATH = "/returnHeadersAsIs";
5863

5964
private static final String SHORT_BODY = "hi there";
6065
private static final String LONG_BODY = createLongBody();
@@ -79,6 +84,8 @@ public static void beforeClass() {
7984
.withTransformers(ReactorNettyClientResponseTransformer.NAME)));
8085
server.stubFor(get(NO_DOUBLE_UA_PATH).willReturn(aResponse()
8186
.withTransformers(ReactorNettyClientResponseTransformer.NAME)));
87+
server.stubFor(get(RETURN_HEADERS_AS_IS_PATH).willReturn(aResponse()
88+
.withTransformers(ReactorNettyClientResponseTransformer.NAME)));
8289
server.start();
8390
// ResourceLeakDetector.setLevel(Level.PARANOID);
8491
}
@@ -388,6 +395,38 @@ public void validateRequestHasOneUserAgentHeader() {
388395
.verifyComplete();
389396
}
390397

398+
@Test
399+
public void validateHeadersReturnAsIs() {
400+
HttpClient client = new ReactorNettyClientProvider().createInstance();
401+
402+
final String singleValueHeaderName = "singleValue";
403+
final String singleValueHeaderValue = "value";
404+
405+
final String multiValueHeaderName = "Multi-value";
406+
final List<String> multiValueHeaderValue = Arrays.asList("value1", "value2");
407+
408+
HttpHeaders headers = new HttpHeaders()
409+
.set(singleValueHeaderName, singleValueHeaderValue)
410+
.set(multiValueHeaderName, multiValueHeaderValue);
411+
412+
StepVerifier.create(client.send(new HttpRequest(HttpMethod.GET, url(server, RETURN_HEADERS_AS_IS_PATH),
413+
headers, Flux.empty())))
414+
.assertNext(response -> {
415+
assertEquals(200, response.getStatusCode());
416+
417+
HttpHeaders responseHeaders = response.getHeaders();
418+
HttpHeader singleValueHeader = responseHeaders.get(singleValueHeaderName);
419+
assertEquals(singleValueHeaderName, singleValueHeader.getName());
420+
assertEquals(singleValueHeaderValue, singleValueHeader.getValue());
421+
422+
HttpHeader multiValueHeader = responseHeaders.get("Multi-value");
423+
assertEquals(multiValueHeaderName, multiValueHeader.getName());
424+
assertLinesMatch(multiValueHeaderValue, multiValueHeader.getValuesList());
425+
})
426+
.expectComplete()
427+
.verify(Duration.ofSeconds(10));
428+
}
429+
391430
private static Stream<Arguments> requestHeaderSupplier() {
392431
return Stream.of(
393432
Arguments.of(null, ReactorNettyClientResponseTransformer.NULL_REPLACEMENT),

‎sdk/core/azure-core-http-okhttp/src/main/java/com/azure/core/http/okhttp/OkHttpResponseBase.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,17 @@ public final Mono<String> getBodyAsString(Charset charset) {
5858
* @return azure-core HttpHeaders
5959
*/
6060
private static HttpHeaders fromOkHttpHeaders(Headers okHttpHeaders) {
61-
return new HttpHeaders().setAll(okHttpHeaders.toMultimap());
61+
/*
62+
* While OkHttp's Headers class offers a method which converts the headers into a Map<String, List<String>>,
63+
* which matches one of the setters in our HttpHeaders, the method implicitly lower cases header names while
64+
* doing the conversion. This is fine when working purely with HTTPs request-response structure as headers are
65+
* case-insensitive per their definition RFC but this could cause issues when/if the headers are used in
66+
* serialization or deserialization as casing may matter.
67+
*/
68+
HttpHeaders azureHeaders = new HttpHeaders();
69+
70+
okHttpHeaders.names().forEach(name -> azureHeaders.set(name, okHttpHeaders.values(name)));
71+
72+
return azureHeaders;
6273
}
6374
}

‎sdk/core/azure-core-http-okhttp/src/test/java/com/azure/core/http/okhttp/OkHttpAsyncHttpClientResponseTransformer.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package com.azure.core.http.okhttp;
5+
6+
import com.github.tomakehurst.wiremock.common.FileSource;
7+
import com.github.tomakehurst.wiremock.extension.Parameters;
8+
import com.github.tomakehurst.wiremock.extension.ResponseTransformer;
9+
import com.github.tomakehurst.wiremock.http.Request;
10+
import com.github.tomakehurst.wiremock.http.Response;
11+
12+
import static com.azure.core.http.okhttp.OkHttpClientTests.RETURN_HEADERS_AS_IS_PATH;
13+
14+
/**
15+
* Mock response transformer used to test {@link OkHttpAsyncHttpClient}.
16+
*/
17+
public class OkHttpAsyncHttpClientResponseTransformer extends ResponseTransformer {
18+
public static final String NAME = "okhttp-async-http-client-response-transformer";
19+
public static final String NULL_REPLACEMENT = "null";
20+
21+
@Override
22+
public Response transform(Request request, Response response, FileSource fileSource, Parameters parameters) {
23+
String url = request.getUrl();
24+
25+
if (RETURN_HEADERS_AS_IS_PATH.equalsIgnoreCase(url)) {
26+
return Response.response()
27+
.status(200)
28+
.headers(request.getHeaders())
29+
.build();
30+
}
31+
32+
return response;
33+
}
34+
35+
@Override
36+
public String getName() {
37+
return NAME;
38+
}
39+
40+
@Override
41+
public boolean applyGlobally() {
42+
return false;
43+
}
44+
}

‎sdk/core/azure-core-http-okhttp/src/test/java/com/azure/core/http/okhttp/OkHttpClientTests.java‎

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,12 @@
44
package com.azure.core.http.okhttp;
55

66
import com.azure.core.http.HttpClient;
7+
import com.azure.core.http.HttpHeader;
8+
import com.azure.core.http.HttpHeaders;
79
import com.azure.core.http.HttpMethod;
810
import com.azure.core.http.HttpRequest;
911
import com.azure.core.http.HttpResponse;
1012
import com.github.tomakehurst.wiremock.WireMockServer;
11-
import com.github.tomakehurst.wiremock.client.WireMock;
1213
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
1314
import org.junit.jupiter.api.AfterAll;
1415
import org.junit.jupiter.api.Assertions;
@@ -32,12 +33,19 @@
3233
import java.security.MessageDigest;
3334
import java.security.NoSuchAlgorithmException;
3435
import java.time.Duration;
36+
import java.util.Arrays;
37+
import java.util.List;
3538
import java.util.concurrent.CountDownLatch;
3639

40+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
41+
import static com.github.tomakehurst.wiremock.client.WireMock.get;
42+
import static com.github.tomakehurst.wiremock.client.WireMock.post;
3743
import static org.junit.jupiter.api.Assertions.assertEquals;
44+
import static org.junit.jupiter.api.Assertions.assertLinesMatch;
3845
import static org.junit.jupiter.api.Assertions.assertNotNull;
3946

4047
public class OkHttpClientTests {
48+
static final String RETURN_HEADERS_AS_IS_PATH = "/returnHeadersAsIs";
4149

4250
private static final String SHORT_BODY = "hi there";
4351
private static final String LONG_BODY = createLongBody();
@@ -46,14 +54,18 @@ public class OkHttpClientTests {
4654

4755
@BeforeAll
4856
public static void beforeClass() {
49-
server = new WireMockServer(WireMockConfiguration.options().dynamicPort().disableRequestJournal());
50-
server.stubFor(
51-
WireMock.get("/short").willReturn(WireMock.aResponse().withBody(SHORT_BODY)));
52-
server.stubFor(WireMock.get("/long").willReturn(WireMock.aResponse().withBody(LONG_BODY)));
53-
server.stubFor(WireMock.get("/error")
54-
.willReturn(WireMock.aResponse().withBody("error").withStatus(500)));
55-
server.stubFor(
56-
WireMock.post("/shortPost").willReturn(WireMock.aResponse().withBody(SHORT_BODY)));
57+
server = new WireMockServer(WireMockConfiguration.options()
58+
.extensions(new OkHttpAsyncHttpClientResponseTransformer())
59+
.dynamicPort()
60+
.disableRequestJournal()
61+
.gzipDisabled(true));
62+
63+
server.stubFor(get("/short").willReturn(aResponse().withBody(SHORT_BODY)));
64+
server.stubFor(get("/long").willReturn(aResponse().withBody(LONG_BODY)));
65+
server.stubFor(get("/error").willReturn(aResponse().withBody("error").withStatus(500)));
66+
server.stubFor(post("/shortPost").willReturn(aResponse().withBody(SHORT_BODY)));
67+
server.stubFor(get(RETURN_HEADERS_AS_IS_PATH).willReturn(aResponse()
68+
.withTransformers(OkHttpAsyncHttpClientResponseTransformer.NAME)));
5769
server.start();
5870
}
5971

@@ -96,22 +108,21 @@ public void testFlowableWhenServerReturnsBodyAndNoErrorsWhenHttp500Returned() {
96108
assertEquals(500, response.getStatusCode());
97109
}
98110

99-
@Disabled("Not working accurately at present")
100111
@Test
101112
public void testFlowableBackpressure() {
102113
HttpResponse response = getResponse("/long");
103-
//
114+
104115
StepVerifierOptions stepVerifierOptions = StepVerifierOptions.create();
105116
stepVerifierOptions.initialRequest(0);
106-
//
117+
107118
StepVerifier.create(response.getBody(), stepVerifierOptions)
108119
.expectNextCount(0)
109120
.thenRequest(1)
110121
.expectNextCount(1)
111122
.thenRequest(3)
112123
.expectNextCount(3)
113-
.thenRequest(Long.MAX_VALUE)// TODO: Check with smaldini, what is the verifier operator to ignore all next emissions
114-
.expectNextCount(1507)
124+
.thenRequest(Long.MAX_VALUE)
125+
.thenConsumeWhile(ByteBuffer::hasRemaining)
115126
.verifyComplete();
116127
}
117128

@@ -228,6 +239,38 @@ public void testConcurrentRequests() throws NoSuchAlgorithmException {
228239
// assertEquals(numRequests * LONG_BODY.getBytes(StandardCharsets.UTF_8).length, numBytes);
229240
}
230241

242+
@Test
243+
public void validateHeadersReturnAsIs() {
244+
HttpClient client = new OkHttpClientProvider().createInstance();
245+
246+
final String singleValueHeaderName = "singleValue";
247+
final String singleValueHeaderValue = "value";
248+
249+
final String multiValueHeaderName = "Multi-value";
250+
final List<String> multiValueHeaderValue = Arrays.asList("value1", "value2");
251+
252+
HttpHeaders headers = new HttpHeaders()
253+
.set(singleValueHeaderName, singleValueHeaderValue)
254+
.set(multiValueHeaderName, multiValueHeaderValue);
255+
256+
StepVerifier.create(client.send(new HttpRequest(HttpMethod.GET, url(server, RETURN_HEADERS_AS_IS_PATH),
257+
headers, Flux.empty())))
258+
.assertNext(response -> {
259+
assertEquals(200, response.getStatusCode());
260+
261+
HttpHeaders responseHeaders = response.getHeaders();
262+
HttpHeader singleValueHeader = responseHeaders.get(singleValueHeaderName);
263+
assertEquals(singleValueHeaderName, singleValueHeader.getName());
264+
assertEquals(singleValueHeaderValue, singleValueHeader.getValue());
265+
266+
HttpHeader multiValueHeader = responseHeaders.get("Multi-value");
267+
assertEquals(multiValueHeaderName, multiValueHeader.getName());
268+
assertLinesMatch(multiValueHeaderValue, multiValueHeader.getValuesList());
269+
})
270+
.expectComplete()
271+
.verify(Duration.ofSeconds(10));
272+
}
273+
231274
private static MessageDigest md5Digest() {
232275
try {
233276
return MessageDigest.getInstance("MD5");

0 commit comments

Comments
 (0)