From 77c90a1d3fffa525a3f840146cc12524e87074ee Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Fri, 12 Dec 2014 11:03:57 +0000 Subject: [PATCH] Remove NPN support from OkHttp Work in progress. I see a new failure from a certificate pinning test but I don't know why. Help solving this would be appreciated. I also see failures before & after: 1) A failure from DisconnectTest (https://github.com/square/okhttp/pull/1197 may fix that). 2) A failure from CallTest.doesNotFollow21Redirects_Async (timeout) The maven changes should be treated with the contempt they deserve. I mostly removed things I didn't understand and stroked maven until it stopped squealing. The benchmarks / okcurl changes are a particular mystery. Tried with arbitrary versions of openjdk7 and openjdk8 I had lying around. Behavior seems the same. --- benchmarks/pom.xml | 8 +- okcurl/pom.xml | 4 - .../squareup/okhttp/internal/Platform.java | 98 ++++++------------- pom.xml | 34 +++---- 4 files changed, 50 insertions(+), 94 deletions(-) diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index a78f5778af5e..ae3a70a3d7ba 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -97,14 +97,15 @@ - npn-when-jdk7 + alpn-when-jdk7 1.7 - org.mortbay.jetty.npn - npn-boot + org.mortbay.jetty.alpn + alpn-boot + ${alpn.jdk7.version} provided @@ -118,6 +119,7 @@ org.mortbay.jetty.alpn alpn-boot + ${alpn.jdk8.version} provided diff --git a/okcurl/pom.xml b/okcurl/pom.xml index 8f8394467855..205e9e23edb3 100644 --- a/okcurl/pom.xml +++ b/okcurl/pom.xml @@ -22,10 +22,6 @@ org.bouncycastle bcprov-jdk15on - - org.mortbay.jetty.npn - npn-boot - io.airlift airline diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java b/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java index b1182d7e86e7..10630220a942 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java @@ -40,24 +40,12 @@ * Access to Platform-specific features necessary for SPDY and advanced TLS. * This includes Server Name Indication (SNI) and session tickets. * - *

ALPN and NPN

- * This class uses TLS extensions ALPN and NPN to negotiate the upgrade from - * HTTP/1.1 (the default protocol to use with TLS on port 443) to either SPDY - * or HTTP/2. + *

ALPN

+ * This class uses TLS extension ALPN to negotiate the upgrade from HTTP/1.1 + * (the default protocol to use with TLS on port 443) to either SPDY or HTTP/2. * - *

NPN (Next Protocol Negotiation) was developed for SPDY. It is widely - * available and we support it on both Android (4.1+) and OpenJDK 7 (via the - * Jetty Alpn-boot library). NPN is not yet available on OpenJDK 8. - * - *

ALPN (Application Layer Protocol Negotiation) is the successor to NPN. It - * has some technical advantages over NPN. ALPN first arrived in Android 4.4, - * but that release suffers a concurrency bug - * so we don't use it. ALPN is supported on OpenJDK 7 and 8 (via the Jetty - * ALPN-boot library). - * - *

On platforms that support both extensions, OkHttp will use both, - * preferring ALPN's result. Future versions of OkHttp will drop support for - * NPN. + *

ALPN (Application Layer Protocol Negotiation) first arrived in Android 4.4, + * ALPN is supported on OpenJDK 7 and 8 (via the Jetty ALPN-boot library). */ public class Platform { private static final Platform PLATFORM = findPlatform(); @@ -132,15 +120,9 @@ private static Platform findPlatform() { // This isn't an Android runtime. } - try { // to find the Jetty's ALPN or NPN extension for OpenJDK. + try { // to find the Jetty's ALPN extension for OpenJDK. String negoClassName = "org.eclipse.jetty.alpn.ALPN"; - Class negoClass; - try { - negoClass = Class.forName(negoClassName); - } catch (ClassNotFoundException ignored) { // ALPN isn't on the classpath. - negoClassName = "org.eclipse.jetty.npn.NextProtoNego"; - negoClass = Class.forName(negoClassName); - } + Class negoClass = Class.forName(negoClassName); Class providerClass = Class.forName(negoClassName + "$Provider"); Class clientProviderClass = Class.forName(negoClassName + "$ClientProvider"); Class serverProviderClass = Class.forName(negoClassName + "$ServerProvider"); @@ -148,8 +130,8 @@ private static Platform findPlatform() { Method getMethod = negoClass.getMethod("get", SSLSocket.class); return new JdkWithJettyBootPlatform( putMethod, getMethod, clientProviderClass, serverProviderClass); - } catch (ClassNotFoundException ignored) { // NPN isn't on the classpath. - } catch (NoSuchMethodException ignored) { // The ALPN or NPN version isn't what we expect. + } catch (ClassNotFoundException ignored) { + } catch (NoSuchMethodException ignored) { // The ALPN version isn't what we expect. } return new Platform(); @@ -157,7 +139,7 @@ private static Platform findPlatform() { /** * Android 2.3 or better. Version 2.3 supports TLS session tickets and server - * name indication (SNI). Versions 4.1 supports NPN. + * name indication (SNI). Versions 4.4 supports ALPN. */ private static class Android extends Platform { @@ -173,12 +155,6 @@ private static class Android extends Platform { // setAlpnSelectedProtocol(byte[]) private static final OptionalMethod SET_ALPN_PROTOCOLS = new OptionalMethod(null, "setAlpnProtocols", byte[].class); - // byte[] getNpnSelectedProtocol() - private static final OptionalMethod GET_NPN_SELECTED_PROTOCOL = - new OptionalMethod(byte[].class, "getNpnSelectedProtocol"); - // setNpnSelectedProtocol(byte[]) - private static final OptionalMethod SET_NPN_PROTOCOLS = - new OptionalMethod(null, "setNpnProtocols", byte[].class); // Non-null on Android 4.0+. private final Method trafficStatsTagSocket; @@ -210,43 +186,26 @@ private Android(Method trafficStatsTagSocket, Method trafficStatsUntagSocket) { SET_HOSTNAME.invokeOptionalWithoutCheckedException(sslSocket, hostname); } - // Enable NPN / ALPN. + // Enable ALPN. boolean alpnSupported = SET_ALPN_PROTOCOLS.isSupported(sslSocket); - boolean npnSupported = SET_NPN_PROTOCOLS.isSupported(sslSocket); - if (!(alpnSupported || npnSupported)) { + if (!alpnSupported) { return; } Object[] parameters = { concatLengthPrefixed(protocols) }; - if (alpnSupported) { - SET_ALPN_PROTOCOLS.invokeWithoutCheckedException(sslSocket, parameters); - } - if (npnSupported) { - SET_NPN_PROTOCOLS.invokeWithoutCheckedException(sslSocket, parameters); - } + SET_ALPN_PROTOCOLS.invokeWithoutCheckedException(sslSocket, parameters); } @Override public String getSelectedProtocol(SSLSocket socket) { boolean alpnSupported = GET_ALPN_SELECTED_PROTOCOL.isSupported(socket); - boolean npnSupported = GET_NPN_SELECTED_PROTOCOL.isSupported(socket); - if (!(alpnSupported || npnSupported)) { + if (!alpnSupported) { return null; } - // Prefer ALPN's result if it is present. - if (alpnSupported) { - byte[] alpnResult = - (byte[]) GET_ALPN_SELECTED_PROTOCOL.invokeWithoutCheckedException(socket); - if (alpnResult != null) { - return new String(alpnResult, Util.UTF_8); - } - } - if (npnSupported) { - byte[] npnResult = - (byte[]) GET_NPN_SELECTED_PROTOCOL.invokeWithoutCheckedException(socket); - if (npnResult != null) { - return new String(npnResult, Util.UTF_8); - } + byte[] alpnResult = + (byte[]) GET_ALPN_SELECTED_PROTOCOL.invokeWithoutCheckedException(socket); + if (alpnResult != null) { + return new String(alpnResult, Util.UTF_8); } return null; } @@ -277,8 +236,7 @@ private Android(Method trafficStatsTagSocket, Method trafficStatsUntagSocket) { } /** - * OpenJDK 7+ with {@code org.mortbay.jetty.npn/npn-boot} or - * {@code org.mortbay.jetty.alpn/alpn-boot} in the boot class path. + * OpenJDK 7+ with {@code org.mortbay.jetty.alpn/alpn-boot} in the boot class path. */ private static class JdkWithJettyBootPlatform extends Platform { private final Method getMethod; @@ -318,8 +276,8 @@ public JdkWithJettyBootPlatform(Method putMethod, Method getMethod, JettyNegoProvider provider = (JettyNegoProvider) Proxy.getInvocationHandler(getMethod.invoke(null, socket)); if (!provider.unsupported && provider.selected == null) { - logger.log(Level.INFO, "NPN/ALPN callback dropped: SPDY and HTTP/2 are disabled. " - + "Is npn-boot or alpn-boot on the boot class path?"); + logger.log(Level.INFO, "ALPN callback dropped: SPDY and HTTP/2 are disabled. " + + "Is alpn-boot on the boot class path?"); return null; } return provider.unsupported ? null : provider.selected; @@ -332,15 +290,15 @@ public JdkWithJettyBootPlatform(Method putMethod, Method getMethod, } /** - * Handle the methods of NPN or ALPN's ClientProvider and ServerProvider + * Handle the methods of ALPN's ClientProvider and ServerProvider * without a compile-time dependency on those interfaces. */ private static class JettyNegoProvider implements InvocationHandler { /** This peer's supported protocols. */ private final List protocols; - /** Set when remote peer notifies NPN or ALPN is unsupported. */ + /** Set when remote peer notifies ALPN is unsupported. */ private boolean unsupported; - /** The protocol the client (NPN) or server (ALPN) selected. */ + /** The protocol the server selected. */ private String selected; public JettyNegoProvider(List protocols) { @@ -354,12 +312,12 @@ public JettyNegoProvider(List protocols) { args = Util.EMPTY_STRING_ARRAY; } if (methodName.equals("supports") && boolean.class == returnType) { - return true; // NPN or ALPN is supported. + return true; // ALPN is supported. } else if (methodName.equals("unsupported") && void.class == returnType) { - this.unsupported = true; // Peer doesn't support NPN or ALPN. + this.unsupported = true; // Peer doesn't support ALPN. return null; } else if (methodName.equals("protocols") && args.length == 0) { - return protocols; // Server (NPN) or Client (ALPN) advertises these protocols. + return protocols; // Client advertises these protocols. } else if ((methodName.equals("selectProtocol") || methodName.equals("select")) && String.class == returnType && args.length == 1 && args[0] instanceof List) { List peerProtocols = (List) args[0]; @@ -372,7 +330,7 @@ public JettyNegoProvider(List protocols) { return selected = protocols.get(0); // On no intersection, try peer's first protocol. } else if ((methodName.equals("protocolSelected") || methodName.equals("selected")) && args.length == 1) { - this.selected = (String) args[0]; // Client (NPN) or Server (ALPN) selected this protocol. + this.selected = (String) args[0]; // Server selected this protocol. return null; } else { return method.invoke(this, args); diff --git a/pom.xml b/pom.xml index 7275159ef590..0d303f01ab57 100644 --- a/pom.xml +++ b/pom.xml @@ -35,10 +35,10 @@ 1.7 1.0.1 - - 1.1.7.v20140316 - - 8.0.0.v20140317 + + 7.0.0.v20140317 + + 8.0.0.v20140317 1.50 2.2.3 4.2.2 @@ -80,15 +80,15 @@ junit ${junit.version} - - org.mortbay.jetty.npn - npn-boot - ${npn.version} + + org.mortbay.jetty.alpn + alpn-jdk7-boot + ${alpn.jdk7.version} org.mortbay.jetty.alpn - alpn-boot - ${alpn.version} + alpn-jdk8-boot + ${alpn.jdk8.version} org.bouncycastle @@ -213,12 +213,12 @@ - npn-when-jdk7 + alpn-when-jdk7 1.7 - ${settings.localRepository}/org/mortbay/jetty/npn/npn-boot/${npn.version}/npn-boot-${npn.version}.jar + ${settings.localRepository}/org/mortbay/jetty/alpn/alpn-boot/${alpn.jdk7.version}/alpn-boot-${alpn.jdk7.version}.jar @@ -231,9 +231,9 @@ - org.mortbay.jetty.npn - npn-boot - ${npn.version} + org.mortbay.jetty.alpn + alpn-boot + ${alpn.jdk7.version} @@ -247,7 +247,7 @@ 1.8 - ${settings.localRepository}/org/mortbay/jetty/alpn/alpn-boot/${alpn.version}/alpn-boot-${alpn.version}.jar + ${settings.localRepository}/org/mortbay/jetty/alpn/alpn-boot/${alpn.jdk8.version}/alpn-boot-${alpn.jdk8.version}.jar @@ -262,7 +262,7 @@ org.mortbay.jetty.alpn alpn-boot - ${alpn.version} + ${alpn.jdk8.version}