From 54c989afe05b1082a2e600ea5f10c40cfe55fbff Mon Sep 17 00:00:00 2001 From: Ran Date: Thu, 10 Oct 2019 09:47:32 -0700 Subject: [PATCH] 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 ba17682eb28ae9a026b55cf80cad3c73d1f9c1e4) * remove the extra argument. --- .../src/main/java/io/grpc/okhttp/Headers.java | 16 ++++++++--- .../io/grpc/okhttp/OkHttpClientStream.java | 9 ++++++- .../io/grpc/okhttp/OkHttpClientTransport.java | 5 ++++ .../test/java/io/grpc/okhttp/HeadersTest.java | 1 + .../grpc/okhttp/OkHttpClientStreamTest.java | 27 ++++++++++++++++++- .../okhttp/OkHttpClientTransportTest.java | 6 ++--- 6 files changed, 56 insertions(+), 8 deletions(-) diff --git a/okhttp/src/main/java/io/grpc/okhttp/Headers.java b/okhttp/src/main/java/io/grpc/okhttp/Headers.java index cb71846eb00..15008f8040f 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/Headers.java +++ b/okhttp/src/main/java/io/grpc/okhttp/Headers.java @@ -34,7 +34,8 @@ */ class Headers { - public static final Header SCHEME_HEADER = new Header(Header.TARGET_SCHEME, "https"); + public static final Header HTTPS_SCHEME_HEADER = new Header(Header.TARGET_SCHEME, "https"); + public static final Header HTTP_SCHEME_HEADER = new Header(Header.TARGET_SCHEME, "http"); public static final Header METHOD_HEADER = new Header(Header.TARGET_METHOD, GrpcUtil.HTTP_METHOD); public static final Header METHOD_GET_HEADER = new Header(Header.TARGET_METHOD, "GET"); public static final Header CONTENT_TYPE_HEADER = @@ -47,7 +48,12 @@ class Headers { * application thread context. */ public static List
createRequestHeaders( - Metadata headers, String defaultPath, String authority, String userAgent, boolean useGet) { + Metadata headers, + String defaultPath, + String authority, + String userAgent, + boolean useGet, + boolean usePlaintext) { Preconditions.checkNotNull(headers, "headers"); Preconditions.checkNotNull(defaultPath, "defaultPath"); Preconditions.checkNotNull(authority, "authority"); @@ -61,7 +67,11 @@ public static List
createRequestHeaders( List
okhttpHeaders = new ArrayList<>(7 + InternalMetadata.headerCount(headers)); // Set GRPC-specific headers. - okhttpHeaders.add(SCHEME_HEADER); + if (usePlaintext) { + okhttpHeaders.add(HTTP_SCHEME_HEADER); + } else { + okhttpHeaders.add(HTTPS_SCHEME_HEADER); + } if (useGet) { okhttpHeaders.add(METHOD_GET_HEADER); } else { diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java index a54feeca149..59208ea9482 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientStream.java @@ -381,7 +381,14 @@ private void sendBuffer(Buffer buffer, boolean endOfStream, boolean flush) { @GuardedBy("lock") private void streamReady(Metadata metadata, String path) { - requestHeaders = Headers.createRequestHeaders(metadata, path, authority, userAgent, useGet); + requestHeaders = + Headers.createRequestHeaders( + metadata, + path, + authority, + userAgent, + useGet, + transport.isUsingPlaintext()); transport.streamReadyToStart(OkHttpClientStream.this); } } diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java index 31de24032e6..1130594f4dd 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java @@ -310,6 +310,11 @@ protected void handleNotInUse() { initTransportTracer(); } + // sslSocketFactory is set to null when use plaintext. + boolean isUsingPlaintext() { + return sslSocketFactory == null; + } + private void initTransportTracer() { synchronized (lock) { // to make @GuardedBy linter happy transportTracer.setFlowControlWindowReader(new TransportTracer.FlowControlReader() { diff --git a/okhttp/src/test/java/io/grpc/okhttp/HeadersTest.java b/okhttp/src/test/java/io/grpc/okhttp/HeadersTest.java index bb84a4186bb..e0662b16117 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/HeadersTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/HeadersTest.java @@ -53,6 +53,7 @@ public void createRequestHeaders_sanitizes() { path, authority, userAgent, + false, false); // 7 reserved headers, 1 user header diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java index ac8bf46fc63..827b87d0f44 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientStreamTest.java @@ -25,6 +25,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import com.google.common.io.BaseEncoding; import io.grpc.CallOptions; @@ -181,7 +182,7 @@ public void start_headerFieldOrder() throws IOException { verify(mockedFrameWriter) .synStream(eq(false), eq(false), eq(3), eq(0), headersCaptor.capture()); assertThat(headersCaptor.getValue()).containsExactly( - Headers.SCHEME_HEADER, + Headers.HTTPS_SCHEME_HEADER, Headers.METHOD_HEADER, new Header(Header.TARGET_AUTHORITY, "localhost"), new Header(Header.TARGET_PATH, "/" + methodDescriptor.getFullMethodName()), @@ -191,6 +192,30 @@ public void start_headerFieldOrder() throws IOException { .inOrder(); } + @Test + public void start_headerPlaintext() throws IOException { + Metadata metaData = new Metadata(); + metaData.put(GrpcUtil.USER_AGENT_KEY, "misbehaving-application"); + when(transport.isUsingPlaintext()).thenReturn(true); + stream = new OkHttpClientStream(methodDescriptor, metaData, frameWriter, transport, + flowController, lock, MAX_MESSAGE_SIZE, INITIAL_WINDOW_SIZE, "localhost", + "good-application", StatsTraceContext.NOOP, transportTracer, CallOptions.DEFAULT); + stream.start(new BaseClientStreamListener()); + stream.transportState().start(3); + + verify(mockedFrameWriter) + .synStream(eq(false), eq(false), eq(3), eq(0), headersCaptor.capture()); + assertThat(headersCaptor.getValue()).containsExactly( + Headers.HTTP_SCHEME_HEADER, + Headers.METHOD_HEADER, + new Header(Header.TARGET_AUTHORITY, "localhost"), + new Header(Header.TARGET_PATH, "/" + methodDescriptor.getFullMethodName()), + new Header(GrpcUtil.USER_AGENT_KEY.name(), "good-application"), + Headers.CONTENT_TYPE_HEADER, + Headers.TE_HEADER) + .inOrder(); + } + @Test public void getUnaryRequest() throws IOException { MethodDescriptor getMethod = MethodDescriptor.newBuilder() diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java index 4c5fb21ff73..d52bd6821a7 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java @@ -22,8 +22,8 @@ import static io.grpc.internal.ClientStreamListener.RpcProgress.REFUSED; import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE; import static io.grpc.okhttp.Headers.CONTENT_TYPE_HEADER; +import static io.grpc.okhttp.Headers.HTTP_SCHEME_HEADER; import static io.grpc.okhttp.Headers.METHOD_HEADER; -import static io.grpc.okhttp.Headers.SCHEME_HEADER; import static io.grpc.okhttp.Headers.TE_HEADER; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -649,7 +649,7 @@ public void addDefaultUserAgent() throws Exception { stream.start(listener); Header userAgentHeader = new Header(GrpcUtil.USER_AGENT_KEY.name(), GrpcUtil.getGrpcUserAgent("okhttp", null)); - List
expectedHeaders = Arrays.asList(SCHEME_HEADER, METHOD_HEADER, + List
expectedHeaders = Arrays.asList(HTTP_SCHEME_HEADER, METHOD_HEADER, new Header(Header.TARGET_AUTHORITY, "notarealauthority:80"), new Header(Header.TARGET_PATH, "/" + method.getFullMethodName()), userAgentHeader, CONTENT_TYPE_HEADER, TE_HEADER); @@ -666,7 +666,7 @@ public void overrideDefaultUserAgent() throws Exception { OkHttpClientStream stream = clientTransport.newStream(method, new Metadata(), CallOptions.DEFAULT); stream.start(listener); - List
expectedHeaders = Arrays.asList(SCHEME_HEADER, METHOD_HEADER, + List
expectedHeaders = Arrays.asList(HTTP_SCHEME_HEADER, METHOD_HEADER, new Header(Header.TARGET_AUTHORITY, "notarealauthority:80"), new Header(Header.TARGET_PATH, "/" + method.getFullMethodName()), new Header(GrpcUtil.USER_AGENT_KEY.name(),