Skip to content

Commit

Permalink
Fix the repeated TimeOut response.
Browse files Browse the repository at this point in the history
  • Loading branch information
cispr committed Feb 20, 2017
1 parent 55fd04d commit f816f3e
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -516,17 +516,19 @@ protected void connected() {
recordClientConnected();
}

@Override
protected void timedOut() {
boolean clientReadMoreRecentlyThanServer =
currentServerConnection == null
|| this.lastReadTime > currentServerConnection.lastReadTime;
if (clientReadMoreRecentlyThanServer) {
LOG.debug("Server timed out: {}", currentServerConnection);
void timedOut(ProxyToServerConnection serverConnection) {
if (currentServerConnection == serverConnection && this.lastReadTime > currentServerConnection.lastReadTime) {
// the idle timeout fired on the active server connection. send a timeout response to the client.
LOG.warn("Server timed out: {}", currentServerConnection);
currentFilters.serverToProxyResponseTimedOut();
writeGatewayTimeout(currentRequest);
// DO NOT call super.timedOut() if the server timed out, to avoid closing the connection unnecessarily
} else {
}
}

@Override
protected void timedOut() {
// idle timeout fired on the client channel. if we aren't waiting on a response from a server, hang up
if (currentServerConnection == null || this.lastReadTime <= currentServerConnection.lastReadTime) {
super.timedOut();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ protected void becameWritable() {
@Override
protected void timedOut() {
super.timedOut();
clientConnection.timedOut();
clientConnection.timedOut(this);
}

@Override
Expand Down
24 changes: 8 additions & 16 deletions src/test/java/org/littleshoot/proxy/KeepAliveTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void testHttp11DoesNotCloseConnectionByDefault() throws IOException, Inte
this.socket = SocketClientUtil.getSocketToProxyServer(proxyServer);

// construct the basic request: METHOD + URI + HTTP version + CRLF (to indicate the end of the request)
String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\n"
String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\r\n"
+ "\r\n";

// send the same request twice over the same connection
Expand Down Expand Up @@ -129,7 +129,7 @@ public void testProxyKeepsClientConnectionOpenAfterServerDisconnect() throws IOE
this.socket = SocketClientUtil.getSocketToProxyServer(proxyServer);

// construct the basic request: METHOD + URI + HTTP version + CRLF (to indicate the end of the request)
String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\n"
String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\r\n"
+ "\r\n";

// send the same request twice over the same connection
Expand Down Expand Up @@ -172,7 +172,7 @@ public void testBadGatewayDoesNotCloseConnection() throws IOException, Interrupt

socket = SocketClientUtil.getSocketToProxyServer(proxyServer);

String badGatewayGet = "GET http://localhost:0/success HTTP/1.1\n"
String badGatewayGet = "GET http://localhost:0/success HTTP/1.1\r\n"
+ "\r\n";

// send the same request twice over the same connection
Expand Down Expand Up @@ -206,35 +206,27 @@ public void testGatewayTimeoutDoesNotCloseConnection() throws IOException, Inter
.withBody("success"));

this.proxyServer = DefaultHttpProxyServer.bootstrap()
.withIdleConnectionTimeout(3)
.withIdleConnectionTimeout(2)
.withPort(0)
.start();

socket = SocketClientUtil.getSocketToProxyServer(proxyServer);

String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\n"
String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\r\n"
+ "\r\n";

// send the same request twice over the same connection
for (int i = 1; i <= 2; i++) {
SocketClientUtil.writeStringToSocket(successfulGet, socket);

// wait a bit to allow the proxy server to respond
Thread.sleep(3500);

String response = SocketClientUtil.readStringFromSocket(socket);

// match the whole response to make sure that the it is not repeated
assertThat("The response is repeated:", response, is("HTTP/1.1 504 Gateway Timeout\r\n" +
"Content-Length: 15\r\n" +
"Content-Type: text/html; charset=utf-8\r\n" +
"\r\n" +
"Gateway TimeoutHTTP/1.1 504 Gateway Timeout\r\n" +
"Content-Length: 15\r\n" +
"Content-Type: text/html; charset=utf-8\r\n" +
"\r\n" +
"Gateway Timeout"));

assertThat("Expected to receive an HTTP 200 from the server (iteration: " + i + ")", response, startsWith("HTTP/1.1 504 Gateway Timeout"));
}

assertTrue("Expected connection to proxy server to be open and readable", SocketClientUtil.isSocketReadyToRead(socket));
Expand Down Expand Up @@ -279,7 +271,7 @@ public HttpResponse clientToProxyRequest(HttpObject httpObject) {

socket = SocketClientUtil.getSocketToProxyServer(proxyServer);

String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\n"
String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\r\n"
+ "\r\n";

// send the same request twice over the same connection
Expand Down Expand Up @@ -338,7 +330,7 @@ public HttpResponse clientToProxyRequest(HttpObject httpObject) {

socket = SocketClientUtil.getSocketToProxyServer(proxyServer);

String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\n"
String successfulGet = "GET http://localhost:" + mockServerPort + "/success HTTP/1.1\r\n"
+ "\r\n";

// only send this request once, since we expect the short circuit response to close the connection
Expand Down
6 changes: 2 additions & 4 deletions src/test/java/org/littleshoot/proxy/TimeoutTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import java.util.concurrent.TimeUnit;

import static org.hamcrest.Matchers.lessThan;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat;
import static org.mockserver.model.HttpRequest.request;
import static org.mockserver.model.HttpResponse.response;
Expand Down Expand Up @@ -132,9 +132,7 @@ public void testClientIdleBeforeRequestReceived() throws IOException, Interrupte
// wait a bit to allow the proxy server to respond
Thread.sleep(1500);

// the proxy should return an HTTP 504 due to the timeout
String response = SocketClientUtil.readStringFromSocket(socket);
assertThat("Expected to receive an HTTP 504 Gateway Timeout from the server", response, startsWith("HTTP/1.1 504"));
assertFalse("Client to proxy connection should be closed", SocketClientUtil.isSocketReadyToRead(socket));

socket.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ public static String readStringFromSocket(Socket socket) throws IOException {
public static boolean isSocketReadyToWrite(Socket socket) throws IOException {
OutputStream out = socket.getOutputStream();
try {
out.write(0);
out.flush();
out.write(0);
out.flush();
for(int i = 0; i < 500; ++i) {
out.write(0);
out.flush();
}
} catch (SocketException e) {
return false;
}
Expand Down

0 comments on commit f816f3e

Please sign in to comment.