Fix Java9SslEngine implementation of ApplicationProtocolAccessor and so fix ApplicationProtocolNegationHandler
Motivation: Java9SslEngine did not correctly implement ApplicationProtocolAccessor and so returned an empty String when no extension was used while the interface contract is to return null. This lead to the situation that ApplicationProtocolNegationHandler did not correct work. Modifications: - Rename ApplicationProtocolAccessor.getApplicationProtocol() to getNegotiatedApplicationProtocol() which resolves the clash with the method exposed by Java9s SSLEngine. - Correctly implement getNegotiatedApplicationProtocol() for Java9Sslengine - Add delegate in Java9Sslengine.getApplicationProtocol() which is provided by Java9 - Adjust tests to test the correct behaviour. Result: Fixes [#7251].
This commit is contained in:
parent
ad548a6a0a
commit
bb1833f22c
@ -26,5 +26,5 @@ interface ApplicationProtocolAccessor {
|
|||||||
* @return the application-level protocol name or
|
* @return the application-level protocol name or
|
||||||
* {@code null} if the negotiation failed or the client does not have ALPN/NPN extension
|
* {@code null} if the negotiation failed or the client does not have ALPN/NPN extension
|
||||||
*/
|
*/
|
||||||
String getApplicationProtocol();
|
String getNegotiatedApplicationProtocol();
|
||||||
}
|
}
|
||||||
|
@ -150,17 +150,25 @@ final class Java9SslEngine extends JdkSslEngine {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
void setApplicationProtocol(String applicationProtocol) {
|
void setNegotiatedApplicationProtocol(String applicationProtocol) {
|
||||||
// Do nothing as this is handled internally by the Java9 implementation of SSLEngine.
|
// Do nothing as this is handled internally by the Java9 implementation of SSLEngine.
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String getApplicationProtocol() {
|
public String getNegotiatedApplicationProtocol() {
|
||||||
return Java9SslUtils.getApplicationProtocol(getWrappedEngine());
|
String protocol = getApplicationProtocol();
|
||||||
|
if (protocol != null) {
|
||||||
|
return protocol.isEmpty() ? null : protocol;
|
||||||
|
}
|
||||||
|
return protocol;
|
||||||
}
|
}
|
||||||
|
|
||||||
// These methods will override the methods defined by Java 9. As we compile with Java8 we can not add
|
// These methods will override the methods defined by Java 9. As we compile with Java8 we can not add
|
||||||
// @Override annotations here.
|
// @Override annotations here.
|
||||||
|
public String getApplicationProtocol() {
|
||||||
|
return Java9SslUtils.getApplicationProtocol(getWrappedEngine());
|
||||||
|
}
|
||||||
|
|
||||||
public String getHandshakeApplicationProtocol() {
|
public String getHandshakeApplicationProtocol() {
|
||||||
return Java9SslUtils.getHandshakeApplicationProtocol(getWrappedEngine());
|
return Java9SslUtils.getHandshakeApplicationProtocol(getWrappedEngine());
|
||||||
}
|
}
|
||||||
|
@ -137,14 +137,14 @@ class JdkBaseApplicationProtocolNegotiator implements JdkApplicationProtocolNego
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void unsupported() {
|
public void unsupported() {
|
||||||
engineWrapper.setApplicationProtocol(null);
|
engineWrapper.setNegotiatedApplicationProtocol(null);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String select(List<String> protocols) throws Exception {
|
public String select(List<String> protocols) throws Exception {
|
||||||
for (String p : supportedProtocols) {
|
for (String p : supportedProtocols) {
|
||||||
if (protocols.contains(p)) {
|
if (protocols.contains(p)) {
|
||||||
engineWrapper.setApplicationProtocol(p);
|
engineWrapper.setNegotiatedApplicationProtocol(p);
|
||||||
return p;
|
return p;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -152,7 +152,7 @@ class JdkBaseApplicationProtocolNegotiator implements JdkApplicationProtocolNego
|
|||||||
}
|
}
|
||||||
|
|
||||||
public String noSelectMatchFound() throws Exception {
|
public String noSelectMatchFound() throws Exception {
|
||||||
engineWrapper.setApplicationProtocol(null);
|
engineWrapper.setNegotiatedApplicationProtocol(null);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -179,13 +179,13 @@ class JdkBaseApplicationProtocolNegotiator implements JdkApplicationProtocolNego
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void unsupported() {
|
public void unsupported() {
|
||||||
engineWrapper.setApplicationProtocol(null);
|
engineWrapper.setNegotiatedApplicationProtocol(null);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void selected(String protocol) throws Exception {
|
public void selected(String protocol) throws Exception {
|
||||||
if (supportedProtocols.contains(protocol)) {
|
if (supportedProtocols.contains(protocol)) {
|
||||||
engineWrapper.setApplicationProtocol(protocol);
|
engineWrapper.setNegotiatedApplicationProtocol(protocol);
|
||||||
} else {
|
} else {
|
||||||
noSelectedMatchFound(protocol);
|
noSelectedMatchFound(protocol);
|
||||||
}
|
}
|
||||||
|
@ -33,11 +33,11 @@ class JdkSslEngine extends SSLEngine implements ApplicationProtocolAccessor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String getApplicationProtocol() {
|
public String getNegotiatedApplicationProtocol() {
|
||||||
return applicationProtocol;
|
return applicationProtocol;
|
||||||
}
|
}
|
||||||
|
|
||||||
void setApplicationProtocol(String applicationProtocol) {
|
void setNegotiatedApplicationProtocol(String applicationProtocol) {
|
||||||
this.applicationProtocol = applicationProtocol;
|
this.applicationProtocol = applicationProtocol;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1829,7 +1829,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String getApplicationProtocol() {
|
public String getNegotiatedApplicationProtocol() {
|
||||||
return applicationProtocol;
|
return applicationProtocol;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -66,7 +66,6 @@ import javax.net.ssl.SSLEngineResult;
|
|||||||
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
|
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
|
||||||
import javax.net.ssl.SSLEngineResult.Status;
|
import javax.net.ssl.SSLEngineResult.Status;
|
||||||
import javax.net.ssl.SSLException;
|
import javax.net.ssl.SSLException;
|
||||||
import javax.net.ssl.SSLSession;
|
|
||||||
|
|
||||||
import static io.netty.buffer.ByteBufUtil.ensureWritableSuccess;
|
import static io.netty.buffer.ByteBufUtil.ensureWritableSuccess;
|
||||||
import static io.netty.handler.ssl.SslUtils.getEncryptedPacketLength;
|
import static io.netty.handler.ssl.SslUtils.getEncryptedPacketLength;
|
||||||
@ -578,7 +577,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
return ((ApplicationProtocolAccessor) engine).getApplicationProtocol();
|
return ((ApplicationProtocolAccessor) engine).getNegotiatedApplicationProtocol();
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -42,6 +42,7 @@ import io.netty.util.concurrent.Future;
|
|||||||
import io.netty.util.concurrent.Promise;
|
import io.netty.util.concurrent.Promise;
|
||||||
import io.netty.util.internal.EmptyArrays;
|
import io.netty.util.internal.EmptyArrays;
|
||||||
import io.netty.util.internal.PlatformDependent;
|
import io.netty.util.internal.PlatformDependent;
|
||||||
|
import io.netty.util.internal.StringUtil;
|
||||||
import org.junit.After;
|
import org.junit.After;
|
||||||
import org.junit.Before;
|
import org.junit.Before;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
@ -996,10 +997,8 @@ public abstract class SSLEngineTest {
|
|||||||
try {
|
try {
|
||||||
writeAndVerifyReceived(clientMessage.retain(), clientChannel, serverLatch, serverReceiver);
|
writeAndVerifyReceived(clientMessage.retain(), clientChannel, serverLatch, serverReceiver);
|
||||||
writeAndVerifyReceived(serverMessage.retain(), serverConnectedChannel, clientLatch, clientReceiver);
|
writeAndVerifyReceived(serverMessage.retain(), serverConnectedChannel, clientLatch, clientReceiver);
|
||||||
if (expectedApplicationProtocol != null) {
|
|
||||||
verifyApplicationLevelProtocol(clientChannel, expectedApplicationProtocol);
|
verifyApplicationLevelProtocol(clientChannel, expectedApplicationProtocol);
|
||||||
verifyApplicationLevelProtocol(serverConnectedChannel, expectedApplicationProtocol);
|
verifyApplicationLevelProtocol(serverConnectedChannel, expectedApplicationProtocol);
|
||||||
}
|
|
||||||
} finally {
|
} finally {
|
||||||
clientMessage.release();
|
clientMessage.release();
|
||||||
serverMessage.release();
|
serverMessage.release();
|
||||||
@ -1011,6 +1010,14 @@ public abstract class SSLEngineTest {
|
|||||||
assertNotNull(handler);
|
assertNotNull(handler);
|
||||||
String appProto = handler.applicationProtocol();
|
String appProto = handler.applicationProtocol();
|
||||||
assertEquals(appProto, expectedApplicationProtocol);
|
assertEquals(appProto, expectedApplicationProtocol);
|
||||||
|
|
||||||
|
SSLEngine engine = handler.engine();
|
||||||
|
if (engine instanceof Java9SslEngine) {
|
||||||
|
// Also verify the Java9 exposed method.
|
||||||
|
Java9SslEngine java9SslEngine = (Java9SslEngine) engine;
|
||||||
|
assertEquals(expectedApplicationProtocol == null ? StringUtil.EMPTY_STRING : expectedApplicationProtocol,
|
||||||
|
java9SslEngine.getApplicationProtocol());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static void writeAndVerifyReceived(ByteBuf message, Channel sendChannel, CountDownLatch receiverLatch,
|
private static void writeAndVerifyReceived(ByteBuf message, Channel sendChannel, CountDownLatch receiverLatch,
|
||||||
|
Loading…
Reference in New Issue
Block a user