Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Java9SslEngine implementation of ApplicationProtocolAccessor and … #7258

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ interface ApplicationProtocolAccessor {
* @return the application-level protocol name or
* {@code null} if the negotiation failed or the client does not have ALPN/NPN extension
*/
String getApplicationProtocol();
String getNegotiatedApplicationProtocol();
}
14 changes: 11 additions & 3 deletions handler/src/main/java/io/netty/handler/ssl/Java9SslEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,25 @@ public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dst, int offset, int
}

@Override
void setApplicationProtocol(String applicationProtocol) {
void setNegotiatedApplicationProtocol(String applicationProtocol) {
// Do nothing as this is handled internally by the Java9 implementation of SSLEngine.
}

@Override
public String getApplicationProtocol() {
return Java9SslUtils.getApplicationProtocol(getWrappedEngine());
public String getNegotiatedApplicationProtocol() {
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
// @Override annotations here.
public String getApplicationProtocol() {
return Java9SslUtils.getApplicationProtocol(getWrappedEngine());
}

public String getHandshakeApplicationProtocol() {
return Java9SslUtils.getHandshakeApplicationProtocol(getWrappedEngine());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,22 +137,22 @@ static class NoFailProtocolSelector implements ProtocolSelector {

@Override
public void unsupported() {
engineWrapper.setApplicationProtocol(null);
engineWrapper.setNegotiatedApplicationProtocol(null);
}

@Override
public String select(List<String> protocols) throws Exception {
for (String p : supportedProtocols) {
if (protocols.contains(p)) {
engineWrapper.setApplicationProtocol(p);
engineWrapper.setNegotiatedApplicationProtocol(p);
return p;
}
}
return noSelectMatchFound();
}

public String noSelectMatchFound() throws Exception {
engineWrapper.setApplicationProtocol(null);
engineWrapper.setNegotiatedApplicationProtocol(null);
return null;
}
}
Expand All @@ -179,13 +179,13 @@ private static class NoFailProtocolSelectionListener implements ProtocolSelectio

@Override
public void unsupported() {
engineWrapper.setApplicationProtocol(null);
engineWrapper.setNegotiatedApplicationProtocol(null);
}

@Override
public void selected(String protocol) throws Exception {
if (supportedProtocols.contains(protocol)) {
engineWrapper.setApplicationProtocol(protocol);
engineWrapper.setNegotiatedApplicationProtocol(protocol);
} else {
noSelectedMatchFound(protocol);
}
Expand Down
4 changes: 2 additions & 2 deletions handler/src/main/java/io/netty/handler/ssl/JdkSslEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ class JdkSslEngine extends SSLEngine implements ApplicationProtocolAccessor {
}

@Override
public String getApplicationProtocol() {
public String getNegotiatedApplicationProtocol() {
return applicationProtocol;
}

void setApplicationProtocol(String applicationProtocol) {
void setNegotiatedApplicationProtocol(String applicationProtocol) {
this.applicationProtocol = applicationProtocol;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1829,7 +1829,7 @@ final boolean checkSniHostnameMatch(String hostname) {
}

@Override
public String getApplicationProtocol() {
public String getNegotiatedApplicationProtocol() {
return applicationProtocol;
}

Expand Down
3 changes: 1 addition & 2 deletions handler/src/main/java/io/netty/handler/ssl/SslHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
import javax.net.ssl.SSLEngineResult.Status;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLSession;

import static io.netty.buffer.ByteBufUtil.ensureWritableSuccess;
import static io.netty.handler.ssl.SslUtils.getEncryptedPacketLength;
Expand Down Expand Up @@ -578,7 +577,7 @@ public String applicationProtocol() {
return null;
}

return ((ApplicationProtocolAccessor) engine).getApplicationProtocol();
return ((ApplicationProtocolAccessor) engine).getNegotiatedApplicationProtocol();
}

/**
Expand Down
15 changes: 11 additions & 4 deletions handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import io.netty.util.concurrent.Promise;
import io.netty.util.internal.EmptyArrays;
import io.netty.util.internal.PlatformDependent;
import io.netty.util.internal.StringUtil;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -996,10 +997,8 @@ protected void runTest(String expectedApplicationProtocol) throws Exception {
try {
writeAndVerifyReceived(clientMessage.retain(), clientChannel, serverLatch, serverReceiver);
writeAndVerifyReceived(serverMessage.retain(), serverConnectedChannel, clientLatch, clientReceiver);
if (expectedApplicationProtocol != null) {
verifyApplicationLevelProtocol(clientChannel, expectedApplicationProtocol);
verifyApplicationLevelProtocol(serverConnectedChannel, expectedApplicationProtocol);
}
verifyApplicationLevelProtocol(clientChannel, expectedApplicationProtocol);
verifyApplicationLevelProtocol(serverConnectedChannel, expectedApplicationProtocol);
} finally {
clientMessage.release();
serverMessage.release();
Expand All @@ -1011,6 +1010,14 @@ private static void verifyApplicationLevelProtocol(Channel channel, String expec
assertNotNull(handler);
String appProto = handler.applicationProtocol();
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,
Expand Down