Merge pull request #238 from hierynomus/issue-236

Fix for race condition in global request response handling (fixes #236)
This commit is contained in:
Jeroen van Erp
2016-03-18 11:29:01 +01:00
5 changed files with 115 additions and 6 deletions

View File

@@ -57,6 +57,8 @@ dependencies {
testCompile "org.mockito:mockito-core:1.9.5"
testCompile "org.apache.sshd:sshd-core:1.0.0"
testRuntime "ch.qos.logback:logback-classic:1.1.2"
testCompile 'org.glassfish.grizzly:grizzly-http-server:2.3.17'
}
jar {

View File

@@ -216,13 +216,15 @@ public class ConnectionImpl
throws ConnectionException {
synchronized (globalReqPromises) {
Promise<SSHPacket, ConnectionException> gr = globalReqPromises.poll();
if (gr == null)
if (gr == null) {
throw new ConnectionException(DisconnectReason.PROTOCOL_ERROR,
"Got a global request response when none was requested");
else if (response == null)
} else if (response == null) {
gr.deliverError(new ConnectionException("Global request [" + gr + "] failed"));
else
gr.deliver(response);
} else {
// To prevent a race condition, copy the packet before delivering, as it will be handled in a different thread.
gr.deliver(new SSHPacket(response));
}
}
}

View File

@@ -0,0 +1,44 @@
package com.hierynomus.sshj.connection.channel.forwarded;
import com.hierynomus.sshj.test.HttpServer;
import com.hierynomus.sshj.test.SshFixture;
import com.hierynomus.sshj.test.util.FileUtil;
import net.schmizz.sshj.SSHClient;
import net.schmizz.sshj.connection.channel.forwarded.RemotePortForwarder;
import net.schmizz.sshj.connection.channel.forwarded.SocketForwardingConnectListener;
import org.apache.sshd.server.forward.AcceptAllForwardingFilter;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.File;
import java.io.IOException;
import java.net.InetSocketAddress;
import static org.junit.Assert.*;
public class RemotePortForwarderTest {
@Rule
public SshFixture fixture = new SshFixture();
@Rule
public HttpServer httpServer = new HttpServer();
@Test
public void shouldDynamicallyForwardPort() throws IOException {
fixture.getServer().setTcpipForwardingFilter(new AcceptAllForwardingFilter());
File file = httpServer.getDocRoot().newFile("index.html");
FileUtil.writeToFile(file, "<html><head/><body><h1>Hi!</h1></body></html>");
SSHClient sshClient = fixture.setupConnectedDefaultClient();
sshClient.authPassword("jeroen", "jeroen");
sshClient.getRemotePortForwarder().bind(
// where the server should listen
new RemotePortForwarder.Forward(0),
// what we do with incoming connections that are forwarded to us
new SocketForwardingConnectListener(new InetSocketAddress("127.0.0.1", 8080)));
}
}

View File

@@ -0,0 +1,45 @@
package com.hierynomus.sshj.test;
import org.junit.rules.ExternalResource;
import org.junit.rules.TemporaryFolder;
import java.io.File;
/**
* Can be used to setup a test HTTP server
*/
public class HttpServer extends ExternalResource {
private org.glassfish.grizzly.http.server.HttpServer httpServer;
private TemporaryFolder docRoot = new TemporaryFolder();
public HttpServer() {
}
@Override
protected void before() throws Throwable {
docRoot.create();
httpServer = org.glassfish.grizzly.http.server.HttpServer.createSimpleServer(docRoot.getRoot().getAbsolutePath());
httpServer.start();
}
@Override
protected void after() {
try {
httpServer.shutdownNow();
} catch (Exception e) {}
try {
docRoot.delete();
} catch (Exception e) {}
}
public org.glassfish.grizzly.http.server.HttpServer getHttpServer() {
return httpServer;
}
public TemporaryFolder getDocRoot() {
return docRoot;
}
}

View File

@@ -0,0 +1,16 @@
<configuration>
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<encoder>
<pattern>%d{yyyy-MM-dd HH:mm:ss.SSS} [%.-20thread] %-5level %logger{36} - %msg%n</pattern>
</encoder>
</appender>
<root level="info">
<appender-ref ref="STDOUT"/>
</root>
<logger name="net.schmizz.sshj" level="debug"/>
<logger name="net.schmizz.sshj.transport" level="trace" />
</configuration>