Skip to content

Commit

Permalink
Remove NPN support from OkHttp
Browse files Browse the repository at this point in the history 
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
(square#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.
  • Loading branch information
@nfuller
nfuller committed Dec 15, 2014
1 parent 09637b6 commit 77c90a1
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 94 deletions.
8 changes: 5 additions & 3 deletions benchmarks/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,15 @@
</build>
<profiles>
<profile>
<id>npn-when-jdk7</id>
<id>alpn-when-jdk7</id>
<activation>
<jdk>1.7</jdk>
</activation>
<dependencies>
<dependency>
<groupId>org.mortbay.jetty.npn</groupId>
<artifactId>npn-boot</artifactId>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-boot</artifactId>
<version>${alpn.jdk7.version}</version>
<scope>provided</scope>
</dependency>
</dependencies>
Expand All @@ -118,6 +119,7 @@
<dependency>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-boot</artifactId>
<version>${alpn.jdk8.version}</version>
<scope>provided</scope>
</dependency>
</dependencies>
Expand Down
4 changes: 0 additions & 4 deletions okcurl/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@
<groupId>org.bouncycastle</groupId>
<artifactId>bcprov-jdk15on</artifactId>
</dependency>
<dependency>
<groupId>org.mortbay.jetty.npn</groupId>
<artifactId>npn-boot</artifactId>
</dependency>
<dependency>
<groupId>io.airlift</groupId>
<artifactId>airline</artifactId>
Expand Down
98 changes: 28 additions & 70 deletions okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,12 @@
* Access to Platform-specific features necessary for SPDY and advanced TLS.
* This includes Server Name Indication (SNI) and session tickets.
*
* <h3>ALPN and NPN</h3>
* 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.
* <h3>ALPN</h3>
* 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.
*
* <p>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.
*
* <p>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 <a href="http://goo.gl/y5izPP">concurrency bug</a>
* so we don't use it. ALPN is supported on OpenJDK 7 and 8 (via the Jetty
* ALPN-boot library).
*
* <p>On platforms that support both extensions, OkHttp will use both,
* preferring ALPN's result. Future versions of OkHttp will drop support for
* NPN.
* <p>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();
Expand Down Expand Up @@ -132,32 +120,26 @@ 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");
Method putMethod = negoClass.getMethod("put", SSLSocket.class, providerClass);
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();
}

/**
* 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 {

Expand All @@ -173,12 +155,6 @@ private static class Android extends Platform {
// setAlpnSelectedProtocol(byte[])
private static final OptionalMethod<Socket> SET_ALPN_PROTOCOLS =
new OptionalMethod<Socket>(null, "setAlpnProtocols", byte[].class);
// byte[] getNpnSelectedProtocol()
private static final OptionalMethod<Socket> GET_NPN_SELECTED_PROTOCOL =
new OptionalMethod<Socket>(byte[].class, "getNpnSelectedProtocol");
// setNpnSelectedProtocol(byte[])
private static final OptionalMethod<Socket> SET_NPN_PROTOCOLS =
new OptionalMethod<Socket>(null, "setNpnProtocols", byte[].class);

// Non-null on Android 4.0+.
private final Method trafficStatsTagSocket;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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<String> 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<String> protocols) {
Expand All @@ -354,12 +312,12 @@ public JettyNegoProvider(List<String> 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<String> peerProtocols = (List) args[0];
Expand All @@ -372,7 +330,7 @@ public JettyNegoProvider(List<String> 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);
Expand Down
34 changes: 17 additions & 17 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
<!-- Compilation -->
<java.version>1.7</java.version>
<okio.version>1.0.1</okio.version>
<!-- Targetted to jdk7u60-b13; Oracle jdk7u55-b13. -->
<npn.version>1.1.7.v20140316</npn.version>
<!-- Targetted to OpenJDK 8 b132 -->
<alpn.version>8.0.0.v20140317</alpn.version>
<!-- ALPN library targeted to OpenJDK 7 -->
<alpn.jdk7.version>7.0.0.v20140317</alpn.jdk7.version>
<!-- ALPN library targeted to OpenJDK 8 b132 -->
<alpn.jdk8.version>8.0.0.v20140317</alpn.jdk8.version>
<bouncycastle.version>1.50</bouncycastle.version>
<gson.version>2.2.3</gson.version>
<apache.http.version>4.2.2</apache.http.version>
Expand Down Expand Up @@ -80,15 +80,15 @@
<artifactId>junit</artifactId>
<version>${junit.version}</version>
</dependency>
<dependency>
<groupId>org.mortbay.jetty.npn</groupId>
<artifactId>npn-boot</artifactId>
<version>${npn.version}</version>
<dependency>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-jdk7-boot</artifactId>
<version>${alpn.jdk7.version}</version>
</dependency>
<dependency>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-boot</artifactId>
<version>${alpn.version}</version>
<artifactId>alpn-jdk8-boot</artifactId>
<version>${alpn.jdk8.version}</version>
</dependency>
<dependency>
<groupId>org.bouncycastle</groupId>
Expand Down Expand Up @@ -213,12 +213,12 @@

<profiles>
<profile>
<id>npn-when-jdk7</id>
<id>alpn-when-jdk7</id>
<activation>
<jdk>1.7</jdk>
</activation>
<properties>
<bootclasspathPrefix>${settings.localRepository}/org/mortbay/jetty/npn/npn-boot/${npn.version}/npn-boot-${npn.version}.jar</bootclasspathPrefix>
<bootclasspathPrefix>${settings.localRepository}/org/mortbay/jetty/alpn/alpn-boot/${alpn.jdk7.version}/alpn-boot-${alpn.jdk7.version}.jar</bootclasspathPrefix>
</properties>
<build>
<pluginManagement>
Expand All @@ -231,9 +231,9 @@
</configuration>
<dependencies>
<dependency>
<groupId>org.mortbay.jetty.npn</groupId>
<artifactId>npn-boot</artifactId>
<version>${npn.version}</version>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-boot</artifactId>
<version>${alpn.jdk7.version}</version>
</dependency>
</dependencies>
</plugin>
Expand All @@ -247,7 +247,7 @@
<jdk>1.8</jdk>
</activation>
<properties>
<bootclasspathPrefix>${settings.localRepository}/org/mortbay/jetty/alpn/alpn-boot/${alpn.version}/alpn-boot-${alpn.version}.jar</bootclasspathPrefix>
<bootclasspathPrefix>${settings.localRepository}/org/mortbay/jetty/alpn/alpn-boot/${alpn.jdk8.version}/alpn-boot-${alpn.jdk8.version}.jar</bootclasspathPrefix>
</properties>
<build>
<pluginManagement>
Expand All @@ -262,7 +262,7 @@
<dependency>
<groupId>org.mortbay.jetty.alpn</groupId>
<artifactId>alpn-boot</artifactId>
<version>${alpn.version}</version>
<version>${alpn.jdk8.version}</version>
</dependency>
</dependencies>
</plugin>
Expand Down

0 comments on commit 77c90a1

Please sign in to comment.