Skip to content

Commit a45bd7e

Browse files
authored
okhttp: fix header scheme does not match transport type.
* okhttp: fix header scheme does not match transport type. (#6260) okhttp: fix header scheme does not match transport type. (cherry picked from commit ba17682) * remove the extra argument.
1 parent faff74f commit a45bd7e

File tree

6 files changed

+56
-8
lines changed

6 files changed

+56
-8
lines changed

okhttp/src/main/java/io/grpc/okhttp/Headers.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@
3434
*/
3535
class Headers {
3636

37-
public static final Header SCHEME_HEADER = new Header(Header.TARGET_SCHEME, "https");
37+
public static final Header HTTPS_SCHEME_HEADER = new Header(Header.TARGET_SCHEME, "https");
38+
public static final Header HTTP_SCHEME_HEADER = new Header(Header.TARGET_SCHEME, "http");
3839
public static final Header METHOD_HEADER = new Header(Header.TARGET_METHOD, GrpcUtil.HTTP_METHOD);
3940
public static final Header METHOD_GET_HEADER = new Header(Header.TARGET_METHOD, "GET");
4041
public static final Header CONTENT_TYPE_HEADER =
@@ -47,7 +48,12 @@ class Headers {
4748
* application thread context.
4849
*/
4950
public static List<Header> createRequestHeaders(
50-
Metadata headers, String defaultPath, String authority, String userAgent, boolean useGet) {
51+
Metadata headers,
52+
String defaultPath,
53+
String authority,
54+
String userAgent,
55+
boolean useGet,
56+
boolean usePlaintext) {
5157
Preconditions.checkNotNull(headers, "headers");
5258
Preconditions.checkNotNull(defaultPath, "defaultPath");
5359
Preconditions.checkNotNull(authority, "authority");
@@ -61,7 +67,11 @@ public static List<Header> createRequestHeaders(
6167
List<Header> okhttpHeaders = new ArrayList<>(7 + InternalMetadata.headerCount(headers));
6268

6369
// Set GRPC-specific headers.
64-
okhttpHeaders.add(SCHEME_HEADER);
70+
if (usePlaintext) {
71+
okhttpHeaders.add(HTTP_SCHEME_HEADER);
72+
} else {
73+
okhttpHeaders.add(HTTPS_SCHEME_HEADER);
74+
}
6575
if (useGet) {
6676
okhttpHeaders.add(METHOD_GET_HEADER);
6777
} else {

okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,14 @@ private void sendBuffer(Buffer buffer, boolean endOfStream, boolean flush) {
381381

382382
@GuardedBy("lock")
383383
private void streamReady(Metadata metadata, String path) {
384-
requestHeaders = Headers.createRequestHeaders(metadata, path, authority, userAgent, useGet);
384+
requestHeaders =
385+
Headers.createRequestHeaders(
386+
metadata,
387+
path,
388+
authority,
389+
userAgent,
390+
useGet,
391+
transport.isUsingPlaintext());
385392
transport.streamReadyToStart(OkHttpClientStream.this);
386393
}
387394
}

okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,11 @@ protected void handleNotInUse() {
310310
initTransportTracer();
311311
}
312312

313+
// sslSocketFactory is set to null when use plaintext.
314+
boolean isUsingPlaintext() {
315+
return sslSocketFactory == null;
316+
}
317+
313318
private void initTransportTracer() {
314319
synchronized (lock) { // to make @GuardedBy linter happy
315320
transportTracer.setFlowControlWindowReader(new TransportTracer.FlowControlReader() {

okhttp/src/test/java/io/grpc/okhttp/HeadersTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public void createRequestHeaders_sanitizes() {
5353
path,
5454
authority,
5555
userAgent,
56+
false,
5657
false);
5758

5859
// 7 reserved headers, 1 user header

okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import static org.mockito.Mockito.times;
2626
import static org.mockito.Mockito.verify;
2727
import static org.mockito.Mockito.verifyNoMoreInteractions;
28+
import static org.mockito.Mockito.when;
2829

2930
import com.google.common.io.BaseEncoding;
3031
import io.grpc.CallOptions;
@@ -181,7 +182,7 @@ public void start_headerFieldOrder() throws IOException {
181182
verify(mockedFrameWriter)
182183
.synStream(eq(false), eq(false), eq(3), eq(0), headersCaptor.capture());
183184
assertThat(headersCaptor.getValue()).containsExactly(
184-
Headers.SCHEME_HEADER,
185+
Headers.HTTPS_SCHEME_HEADER,
185186
Headers.METHOD_HEADER,
186187
new Header(Header.TARGET_AUTHORITY, "localhost"),
187188
new Header(Header.TARGET_PATH, "/" + methodDescriptor.getFullMethodName()),
@@ -191,6 +192,30 @@ public void start_headerFieldOrder() throws IOException {
191192
.inOrder();
192193
}
193194

195+
@Test
196+
public void start_headerPlaintext() throws IOException {
197+
Metadata metaData = new Metadata();
198+
metaData.put(GrpcUtil.USER_AGENT_KEY, "misbehaving-application");
199+
when(transport.isUsingPlaintext()).thenReturn(true);
200+
stream = new OkHttpClientStream(methodDescriptor, metaData, frameWriter, transport,
201+
flowController, lock, MAX_MESSAGE_SIZE, INITIAL_WINDOW_SIZE, "localhost",
202+
"good-application", StatsTraceContext.NOOP, transportTracer, CallOptions.DEFAULT);
203+
stream.start(new BaseClientStreamListener());
204+
stream.transportState().start(3);
205+
206+
verify(mockedFrameWriter)
207+
.synStream(eq(false), eq(false), eq(3), eq(0), headersCaptor.capture());
208+
assertThat(headersCaptor.getValue()).containsExactly(
209+
Headers.HTTP_SCHEME_HEADER,
210+
Headers.METHOD_HEADER,
211+
new Header(Header.TARGET_AUTHORITY, "localhost"),
212+
new Header(Header.TARGET_PATH, "/" + methodDescriptor.getFullMethodName()),
213+
new Header(GrpcUtil.USER_AGENT_KEY.name(), "good-application"),
214+
Headers.CONTENT_TYPE_HEADER,
215+
Headers.TE_HEADER)
216+
.inOrder();
217+
}
218+
194219
@Test
195220
public void getUnaryRequest() throws IOException {
196221
MethodDescriptor<?, ?> getMethod = MethodDescriptor.<Void, Void>newBuilder()

okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
import static io.grpc.internal.ClientStreamListener.RpcProgress.REFUSED;
2323
import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE;
2424
import static io.grpc.okhttp.Headers.CONTENT_TYPE_HEADER;
25+
import static io.grpc.okhttp.Headers.HTTP_SCHEME_HEADER;
2526
import static io.grpc.okhttp.Headers.METHOD_HEADER;
26-
import static io.grpc.okhttp.Headers.SCHEME_HEADER;
2727
import static io.grpc.okhttp.Headers.TE_HEADER;
2828
import static org.junit.Assert.assertEquals;
2929
import static org.junit.Assert.assertFalse;
@@ -649,7 +649,7 @@ public void addDefaultUserAgent() throws Exception {
649649
stream.start(listener);
650650
Header userAgentHeader = new Header(GrpcUtil.USER_AGENT_KEY.name(),
651651
GrpcUtil.getGrpcUserAgent("okhttp", null));
652-
List<Header> expectedHeaders = Arrays.asList(SCHEME_HEADER, METHOD_HEADER,
652+
List<Header> expectedHeaders = Arrays.asList(HTTP_SCHEME_HEADER, METHOD_HEADER,
653653
new Header(Header.TARGET_AUTHORITY, "notarealauthority:80"),
654654
new Header(Header.TARGET_PATH, "/" + method.getFullMethodName()),
655655
userAgentHeader, CONTENT_TYPE_HEADER, TE_HEADER);
@@ -666,7 +666,7 @@ public void overrideDefaultUserAgent() throws Exception {
666666
OkHttpClientStream stream =
667667
clientTransport.newStream(method, new Metadata(), CallOptions.DEFAULT);
668668
stream.start(listener);
669-
List<Header> expectedHeaders = Arrays.asList(SCHEME_HEADER, METHOD_HEADER,
669+
List<Header> expectedHeaders = Arrays.asList(HTTP_SCHEME_HEADER, METHOD_HEADER,
670670
new Header(Header.TARGET_AUTHORITY, "notarealauthority:80"),
671671
new Header(Header.TARGET_PATH, "/" + method.getFullMethodName()),
672672
new Header(GrpcUtil.USER_AGENT_KEY.name(),

0 commit comments

Comments
 (0)