Skip to content

Commit

Permalink
Merge remote-tracking branch 'nfuller/ChangeAlpnNpnSupport' into nful…
Browse files Browse the repository at this point in the history
…ler_ChangeAlpnNpnSupport

* nfuller/ChangeAlpnNpnSupport:
  Remove NPN support from OkHttp

Conflicts:
	okhttp/src/main/java/com/squareup/okhttp/Protocol.java
	pom.xml
  • Loading branch information
squarejesse committed Dec 26, 2014
2 parents 8cf76d0 + 4d06821 commit 9559348
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 133 deletions.
8 changes: 5 additions & 3 deletions benchmarks/pom.xml
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
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public class Benchmark extends com.google.caliper.Benchmark {
@Param({ "0", "20" })
int headerCount;

/** Which ALPN/NPN protocols are in use. Only useful with TLS. */
/** Which ALPN protocols are in use. Only useful with TLS. */
List<Protocol> protocols = Arrays.asList(Protocol.HTTP_1_1);

public static void main(String[] args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,17 +169,7 @@ public void setBodyLimit(int maxBodyLength) {
}

/**
* Sets whether NPN is used on incoming HTTPS connections to negotiate a
* protocol like HTTP/1.1 or SPDY/3. Call this method to disable NPN and
* SPDY.
* @deprecated Use {@link #setProtocolNegotiationEnabled}.
*/
public void setNpnEnabled(boolean npnEnabled) {
this.protocolNegotiationEnabled = npnEnabled;
}

/**
* Sets whether ALPN or NPN is used on incoming HTTPS connections to
* Sets whether ALPN is used on incoming HTTPS connections to
* negotiate a protocol like HTTP/1.1 or HTTP/2. Call this method to disable
* negotiation and restrict connections to HTTP/1.1.
*/
Expand All @@ -188,19 +178,7 @@ public void setProtocolNegotiationEnabled(boolean protocolNegotiationEnabled) {
}

/**
* Indicates the protocols supported by NPN on incoming HTTPS connections.
* This list is ignored when npn is disabled.
*
* @param protocols the protocols to use, in order of preference. The list
* must contain "http/1.1". It must not contain null.
* @deprecated Use {@link #setProtocols(java.util.List)}.
*/
public void setNpnProtocols(List<Protocol> protocols) {
setProtocols(protocols);
}

/**
* Indicates the protocols supported by NPN or ALPN on incoming HTTPS
* Indicates the protocols supported by ALPN on incoming HTTPS
* connections. This list is ignored when
* {@link #setProtocolNegotiationEnabled negotiation is disabled}.
*
Expand Down
4 changes: 0 additions & 4 deletions okcurl/pom.xml
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static void main(String[] args) throws Exception {
int responseCode = connection.getResponseCode();
System.out.println(responseCode);
List<String> protocolValues = connection.getHeaderFields().get(SELECTED_PROTOCOL);
// If null, probably you didn't add jetty's npn jar to your boot classpath!
// If null, probably you didn't add jetty's alpn jar to your boot classpath!
if (protocolValues != null && !protocolValues.isEmpty()) {
System.out.println("PROTOCOL " + protocolValues.get(0));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static void main(String[] args) throws Exception {
int responseCode = connection.getResponseCode();
System.out.println(responseCode);
List<String> protocolValues = connection.getHeaderFields().get(SELECTED_PROTOCOL);
// If null, probably you didn't add jetty's npn jar to your boot classpath!
// If null, probably you didn't add jetty's alpn jar to your boot classpath!
if (protocolValues != null && !protocolValues.isEmpty()) {
System.out.println("PROTOCOL " + protocolValues.get(0));
}
Expand Down
9 changes: 4 additions & 5 deletions okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,7 @@ public final Dispatcher getDispatcher() {
* successors (h2). The http/1.1 transport will never be dropped.
*
* <p>If multiple protocols are specified, <a
* href="https://technotes.googlecode.com/git/nextprotoneg.html">NPN</a> or
* <a href="http://tools.ietf.org/html/draft-ietf-tls-applayerprotoneg">ALPN</a>
* href="http://tools.ietf.org/html/draft-ietf-tls-applayerprotoneg">ALPN</a>
* will be used to negotiate a transport.
*
* <p>{@link Protocol#HTTP_1_0} is not supported in this set. Requests are
Expand Down Expand Up @@ -605,9 +604,9 @@ final OkHttpClient copyWithDefaults() {
/**
* Java and Android programs default to using a single global SSL context,
* accessible to HTTP clients as {@link SSLSocketFactory#getDefault()}. If we
* used the shared SSL context, when OkHttp enables NPN for its SPDY-related
* stuff, it would also enable NPN for other usages, which might crash them
* because NPN is enabled when it isn't expected to be.
* used the shared SSL context, when OkHttp enables ALPN for its SPDY-related
* stuff, it would also enable ALPN for other usages, which might crash them
* because ALPN is enabled when it isn't expected to be.
*
* <p>This code avoids that by defaulting to an OkHttp-created SSL context.
* The drawback of this approach is that apps that customize the global SSL
Expand Down
5 changes: 2 additions & 3 deletions okhttp/src/main/java/com/squareup/okhttp/Protocol.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@

/**
* Protocols that OkHttp implements for <a
* href="http://tools.ietf.org/html/draft-agl-tls-nextprotoneg-04">NPN</a> and
* <a href="http://tools.ietf.org/html/draft-ietf-tls-applayerprotoneg">ALPN</a>
* href="http://tools.ietf.org/html/draft-ietf-tls-applayerprotoneg">ALPN</a>
* selection.
*
* <h3>Protocol vs Scheme</h3>
Expand Down Expand Up @@ -96,7 +95,7 @@ public static Protocol get(String protocol) throws IOException {
}

/**
* Returns the string used to identify this protocol for ALPN and NPN, like
* Returns the string used to identify this protocol for ALPN, like
* "http/1.1", "spdy/3.1" or "h2-16".
*/
@Override public String toString() {
Expand Down
102 changes: 30 additions & 72 deletions okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java
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 All @@ -299,7 +257,7 @@ public JdkWithJettyBootPlatform(Method putMethod, Method getMethod,
List<String> names = new ArrayList<>(protocols.size());
for (int i = 0, size = protocols.size(); i < size; i++) {
Protocol protocol = protocols.get(i);
if (protocol == Protocol.HTTP_1_0) continue; // No HTTP/1.0 for NPN or ALPN.
if (protocol == Protocol.HTTP_1_0) continue; // No HTTP/1.0 for ALPN.
names.add(protocol.toString());
}
try {
Expand All @@ -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 All @@ -388,7 +346,7 @@ static byte[] concatLengthPrefixed(List<Protocol> protocols) {
Buffer result = new Buffer();
for (int i = 0, size = protocols.size(); i < size; i++) {
Protocol protocol = protocols.get(i);
if (protocol == Protocol.HTTP_1_0) continue; // No HTTP/1.0 for NPN.
if (protocol == Protocol.HTTP_1_0) continue; // No HTTP/1.0 for ALPN.
result.writeByte(protocol.toString().length());
result.writeUtf8(protocol.toString());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ private SpdyConnection(Builder builder) throws IOException {
new Thread(readerRunnable).start(); // Not a daemon thread.
}

/** The protocol as selected using NPN or ALPN. */
/** The protocol as selected using ALPN. */
public Protocol getProtocol() {
return protocol;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
/** A version and dialect of the framed socket protocol. */
public interface Variant {

/** The protocol as selected using NPN or ALPN. */
/** The protocol as selected using ALPN. */
Protocol getProtocol();

/**
Expand Down
Loading

0 comments on commit 9559348

Please sign in to comment.