From dba09163debee5c3d4e3455d23cdff01fdc5d57e Mon Sep 17 00:00:00 2001 From: sanjaypujare Date: Thu, 9 Jan 2020 16:41:20 -0800 Subject: [PATCH] netty: remove 'grpc-exp' from the list of next-protocol-versions in ALPN (#6592) --- .../java/io/grpc/netty/GrpcSslContexts.java | 15 ++------ .../io/grpc/netty/GrpcSslContextsTest.java | 34 ------------------- .../grpc/netty/ProtocolNegotiatorsTest.java | 23 ------------- .../io/grpc/okhttp/OkHttpTlsUpgrader.java | 2 +- .../okhttp/OkHttpProtocolNegotiatorTest.java | 20 ----------- .../io/grpc/okhttp/OkHttpTlsUpgraderTest.java | 8 ----- .../io/grpc/okhttp/internal/Protocol.java | 12 +------ 7 files changed, 4 insertions(+), 110 deletions(-) delete mode 100644 netty/src/test/java/io/grpc/netty/GrpcSslContextsTest.java diff --git a/netty/src/main/java/io/grpc/netty/GrpcSslContexts.java b/netty/src/main/java/io/grpc/netty/GrpcSslContexts.java index 1b1820d8c1..72701cabc5 100644 --- a/netty/src/main/java/io/grpc/netty/GrpcSslContexts.java +++ b/netty/src/main/java/io/grpc/netty/GrpcSslContexts.java @@ -50,25 +50,14 @@ public class GrpcSslContexts { private GrpcSslContexts() {} - /* - * The experimental "grpc-exp" string identifies gRPC (and by implication - * HTTP/2) when used over TLS. This indicates to the server that the client - * will only send gRPC traffic on the h2 connection and is negotiated in - * preference to h2 when the client and server support it, but is not - * standardized. Support for this may be removed at any time. - */ - private static final String GRPC_EXP_VERSION = "grpc-exp"; - // The "h2" string identifies HTTP/2 when used over TLS private static final String HTTP2_VERSION = "h2"; /* - * List of ALPN/NPN protocols in order of preference. GRPC_EXP_VERSION - * requires that HTTP2_VERSION be present and that GRPC_EXP_VERSION should be - * preferenced over HTTP2_VERSION. + * List of ALPN/NPN protocols in order of preference. */ static final List NEXT_PROTOCOL_VERSIONS = - Collections.unmodifiableList(Arrays.asList(GRPC_EXP_VERSION, HTTP2_VERSION)); + Collections.unmodifiableList(Arrays.asList(HTTP2_VERSION)); /* * These configs use ACCEPT due to limited support in OpenSSL. Actual protocol enforcement is diff --git a/netty/src/test/java/io/grpc/netty/GrpcSslContextsTest.java b/netty/src/test/java/io/grpc/netty/GrpcSslContextsTest.java deleted file mode 100644 index c74fead78a..0000000000 --- a/netty/src/test/java/io/grpc/netty/GrpcSslContextsTest.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2016 The gRPC Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.grpc.netty; - -import static org.junit.Assert.assertTrue; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Unit tests for {@link io.grpc.netty.GrpcSslContexts}. */ -@RunWith(JUnit4.class) -public class GrpcSslContextsTest { - @Test public void selectApplicationProtocolConfig_grpcExp() { - assertTrue( - GrpcSslContexts.NEXT_PROTOCOL_VERSIONS.indexOf("grpc-exp") == -1 - || GrpcSslContexts.NEXT_PROTOCOL_VERSIONS.indexOf("grpc-exp") - < GrpcSslContexts.NEXT_PROTOCOL_VERSIONS.indexOf("h2")); - } -} diff --git a/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java b/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java index ffa45f4816..5747e74e2f 100644 --- a/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java +++ b/netty/src/test/java/io/grpc/netty/ProtocolNegotiatorsTest.java @@ -349,29 +349,6 @@ public class ProtocolNegotiatorsTest { assertNotNull(grpcHandlerCtx); } - @Test - public void tlsHandler_userEventTriggeredSslEvent_supportedProtocolGrpcExp() throws Exception { - SslHandler goodSslHandler = new SslHandler(engine, false) { - @Override - public String applicationProtocol() { - return "grpc-exp"; - } - }; - - ChannelHandler handler = new ServerTlsHandler(grpcHandler, sslContext, null); - pipeline.addLast(handler); - - pipeline.replace(SslHandler.class, null, goodSslHandler); - channelHandlerCtx = pipeline.context(handler); - Object sslEvent = SslHandshakeCompletionEvent.SUCCESS; - - pipeline.fireUserEventTriggered(sslEvent); - - assertTrue(channel.isOpen()); - ChannelHandlerContext grpcHandlerCtx = pipeline.context(grpcHandler); - assertNotNull(grpcHandlerCtx); - } - @Test public void engineLog() { ChannelHandler handler = new ServerTlsHandler(grpcHandler, sslContext, null); diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java index 937758a538..d5cf69bba1 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpTlsUpgrader.java @@ -43,7 +43,7 @@ final class OkHttpTlsUpgrader { */ @VisibleForTesting static final List TLS_PROTOCOLS = - Collections.unmodifiableList(Arrays.asList(Protocol.GRPC_EXP, Protocol.HTTP_2)); + Collections.unmodifiableList(Arrays.asList(Protocol.HTTP_2)); /** * Upgrades given Socket to be an SSLSocket. diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpProtocolNegotiatorTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpProtocolNegotiatorTest.java index 370b0f6add..61214f169d 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpProtocolNegotiatorTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpProtocolNegotiatorTest.java @@ -146,26 +146,6 @@ public class OkHttpProtocolNegotiatorTest { verify(platform).afterHandshake(sock); } - @Test - public void negotiate_preferGrpcExp() throws Exception { - // This test doesn't actually verify that grpc-exp is preferred, since the - // mocking of getSelectedProtocol() causes the protocol list to be ignored. - // The main usefulness of the test is for future changes to - // OkHttpProtocolNegotiator, where we can catch any change that would affect - // grpc-exp preference. - when(platform.getSelectedProtocol(ArgumentMatchers.any())).thenReturn("grpc-exp"); - OkHttpProtocolNegotiator negotiator = new OkHttpProtocolNegotiator(platform); - - String actual = - negotiator.negotiate(sock, "hostname", - ImmutableList.of(Protocol.GRPC_EXP, Protocol.HTTP_2)); - - assertEquals("grpc-exp", actual); - verify(sock).startHandshake(); - verify(platform).getSelectedProtocol(sock); - verify(platform).afterHandshake(sock); - } - // Checks that the super class is properly invoked. @Test public void negotiate_android_handshakeFails() throws Exception { diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java index 05c649a265..6346ab8e8f 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTlsUpgraderTest.java @@ -18,9 +18,7 @@ package io.grpc.okhttp; import static io.grpc.okhttp.OkHttpTlsUpgrader.canonicalizeHost; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import io.grpc.okhttp.internal.Protocol; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -28,12 +26,6 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link io.grpc.okhttp.OkHttpTlsUpgrader}. */ @RunWith(JUnit4.class) public class OkHttpTlsUpgraderTest { - @Test public void upgrade_grpcExp() { - assertTrue( - OkHttpTlsUpgrader.TLS_PROTOCOLS.indexOf(Protocol.GRPC_EXP) == -1 - || OkHttpTlsUpgrader.TLS_PROTOCOLS.indexOf(Protocol.GRPC_EXP) - < OkHttpTlsUpgrader.TLS_PROTOCOLS.indexOf(Protocol.HTTP_2)); - } @Test public void canonicalizeHosts() { assertEquals("::1", canonicalizeHost("::1")); diff --git a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Protocol.java b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Protocol.java index 7b5a3ae76a..0c9244607f 100644 --- a/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Protocol.java +++ b/okhttp/third_party/okhttp/main/java/io/grpc/okhttp/internal/Protocol.java @@ -70,16 +70,7 @@ public enum Protocol { * , present in Java 8+ and Android 5+. Servers that enforce this may send an * exception message including the string {@code INADEQUATE_SECURITY}. */ - HTTP_2("h2"), - - /** - * The experimental "grpc-exp" string identifies gRPC (and by implication - * HTTP/2) when used over TLS. This indicates to the server that the client - * will only send gRPC traffic on the h2 connection and is negotiated in - * preference to h2 when the client and server support it, but is not - * standardized. Support for this may be removed at any time. - */ - GRPC_EXP("grpc-exp"); + HTTP_2("h2"); private final String protocol; @@ -96,7 +87,6 @@ public enum Protocol { if (protocol.equals(HTTP_1_0.protocol)) return HTTP_1_0; if (protocol.equals(HTTP_1_1.protocol)) return HTTP_1_1; if (protocol.equals(HTTP_2.protocol)) return HTTP_2; - if (protocol.equals(GRPC_EXP.protocol)) return GRPC_EXP; if (protocol.equals(SPDY_3.protocol)) return SPDY_3; throw new IOException("Unexpected protocol: " + protocol); }