Cleaned up test case and implementation for #360665 (Proxying HTTPS request to HTTP port causes exception loop).
Class SelectConnector.ProxySelectChannelEndPoint did not follow the latest changes to interface EndPoint, and was returning wrong values for isInputShutdown() and other few methods. Also suppressed expected exceptions in the test case.
This commit is contained in:
parent
9aee1affe5
commit
0a40c3d750
|
@ -15,7 +15,7 @@ package org.eclipse.jetty.client;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.lang.reflect.Constructor;
|
||||
import java.net.ConnectException;
|
||||
import java.net.ProtocolException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedList;
|
||||
import java.util.List;
|
||||
|
@ -692,12 +692,13 @@ public class HttpDestination implements Dumpable
|
|||
{
|
||||
proxyEndPoint.upgrade();
|
||||
}
|
||||
else if(responseStatus == HttpStatus.GATEWAY_TIMEOUT_504){
|
||||
else if(responseStatus == HttpStatus.GATEWAY_TIMEOUT_504)
|
||||
{
|
||||
onExpire();
|
||||
}
|
||||
else
|
||||
{
|
||||
onException(new ConnectException("Proxy: " + proxyEndPoint.getRemoteAddr() +":" + proxyEndPoint.getRemotePort() + " didn't return http return code 200, but " + responseStatus + " while trying to request: " + exchange.getAddress().toString()));
|
||||
onException(new ProtocolException("Proxy: " + proxyEndPoint.getRemoteAddr() +":" + proxyEndPoint.getRemotePort() + " didn't return http return code 200, but " + responseStatus + " while trying to request: " + exchange.getAddress().toString()));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -15,14 +15,11 @@ package org.eclipse.jetty.client;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.net.SocketTimeoutException;
|
||||
import java.net.UnknownHostException;
|
||||
import java.nio.channels.SelectionKey;
|
||||
import java.nio.channels.SocketChannel;
|
||||
import java.nio.channels.UnresolvedAddressException;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
|
||||
import javax.net.ssl.SSLContext;
|
||||
import javax.net.ssl.SSLEngine;
|
||||
import javax.net.ssl.SSLSession;
|
||||
|
||||
|
@ -280,12 +277,14 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public static class ProxySelectChannelEndPoint extends SslSelectChannelEndPoint
|
||||
{
|
||||
private final SelectChannelEndPoint plainEndPoint;
|
||||
private final EnforceOverrideEndPointMethods enforcer;
|
||||
private volatile boolean upgraded = false;
|
||||
|
||||
public ProxySelectChannelEndPoint(SocketChannel channel, SelectorManager.SelectSet selectSet, SelectionKey key, Buffers sslBuffers, SSLEngine engine, int maxIdleTimeout) throws IOException
|
||||
{
|
||||
super(sslBuffers, channel, selectSet, key, engine, maxIdleTimeout);
|
||||
this.plainEndPoint = new SelectChannelEndPoint(channel, selectSet, key, maxIdleTimeout);
|
||||
this.enforcer = new EnforceOverrideEndPointMethods();
|
||||
}
|
||||
|
||||
public void upgrade()
|
||||
|
@ -293,18 +292,176 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
upgraded = true;
|
||||
}
|
||||
|
||||
public void shutdownOutput() throws IOException
|
||||
{
|
||||
enforcer.shutdownOutput();
|
||||
}
|
||||
|
||||
public boolean isOutputShutdown()
|
||||
{
|
||||
return enforcer.isOutputShutdown();
|
||||
}
|
||||
|
||||
public void shutdownInput() throws IOException
|
||||
{
|
||||
enforcer.shutdownInput();
|
||||
}
|
||||
|
||||
public boolean isInputShutdown()
|
||||
{
|
||||
return enforcer.isInputShutdown();
|
||||
}
|
||||
|
||||
public void close() throws IOException
|
||||
{
|
||||
enforcer.close();
|
||||
}
|
||||
|
||||
public int fill(Buffer buffer) throws IOException
|
||||
{
|
||||
return enforcer.fill(buffer);
|
||||
}
|
||||
|
||||
public int flush(Buffer buffer) throws IOException
|
||||
{
|
||||
return enforcer.flush(buffer);
|
||||
}
|
||||
|
||||
public int flush(Buffer header, Buffer buffer, Buffer trailer) throws IOException
|
||||
{
|
||||
return enforcer.flush(header, buffer, trailer);
|
||||
}
|
||||
|
||||
public String getLocalAddr()
|
||||
{
|
||||
return enforcer.getLocalAddr();
|
||||
}
|
||||
|
||||
public String getLocalHost()
|
||||
{
|
||||
return enforcer.getLocalHost();
|
||||
}
|
||||
|
||||
public int getLocalPort()
|
||||
{
|
||||
return enforcer.getLocalPort();
|
||||
}
|
||||
|
||||
public String getRemoteAddr()
|
||||
{
|
||||
return enforcer.getRemoteAddr();
|
||||
}
|
||||
|
||||
public String getRemoteHost()
|
||||
{
|
||||
return enforcer.getRemoteHost();
|
||||
}
|
||||
|
||||
public int getRemotePort()
|
||||
{
|
||||
return enforcer.getRemotePort();
|
||||
}
|
||||
|
||||
public boolean isBlocking()
|
||||
{
|
||||
return enforcer.isBlocking();
|
||||
}
|
||||
|
||||
public boolean isBufferred()
|
||||
{
|
||||
return enforcer.isBufferred();
|
||||
}
|
||||
|
||||
public boolean blockReadable(long millisecs) throws IOException
|
||||
{
|
||||
return enforcer.blockReadable(millisecs);
|
||||
}
|
||||
|
||||
public boolean blockWritable(long millisecs) throws IOException
|
||||
{
|
||||
return enforcer.blockWritable(millisecs);
|
||||
}
|
||||
|
||||
public boolean isOpen()
|
||||
{
|
||||
return enforcer.isOpen();
|
||||
}
|
||||
|
||||
public Object getTransport()
|
||||
{
|
||||
return enforcer.getTransport();
|
||||
}
|
||||
|
||||
public boolean isBufferingInput()
|
||||
{
|
||||
return enforcer.isBufferingInput();
|
||||
}
|
||||
|
||||
public boolean isBufferingOutput()
|
||||
{
|
||||
return enforcer.isBufferingOutput();
|
||||
}
|
||||
|
||||
public void flush() throws IOException
|
||||
{
|
||||
enforcer.flush();
|
||||
}
|
||||
|
||||
public int getMaxIdleTime()
|
||||
{
|
||||
return enforcer.getMaxIdleTime();
|
||||
}
|
||||
|
||||
public void setMaxIdleTime(int timeMs) throws IOException
|
||||
{
|
||||
enforcer.setMaxIdleTime(timeMs);
|
||||
}
|
||||
|
||||
/**
|
||||
* The only reason this class exist is to enforce that
|
||||
* {@link ProxySelectChannelEndPoint} overrides all methods of {@link EndPoint}.
|
||||
* Therefore, if a method is added to {@link EndPoint}, this class
|
||||
* won't compile anymore, will need an implementation, and one must remember
|
||||
* to override the correspondent method in {@link ProxySelectChannelEndPoint}.
|
||||
*/
|
||||
private class EnforceOverrideEndPointMethods implements EndPoint
|
||||
{
|
||||
public void shutdownOutput() throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
super.shutdownOutput();
|
||||
ProxySelectChannelEndPoint.super.shutdownOutput();
|
||||
else
|
||||
plainEndPoint.shutdownOutput();
|
||||
}
|
||||
|
||||
public boolean isOutputShutdown()
|
||||
{
|
||||
if (upgraded)
|
||||
return ProxySelectChannelEndPoint.super.isOutputShutdown();
|
||||
else
|
||||
return plainEndPoint.isOutputShutdown();
|
||||
}
|
||||
|
||||
public void shutdownInput() throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
ProxySelectChannelEndPoint.super.shutdownInput();
|
||||
else
|
||||
plainEndPoint.shutdownInput();
|
||||
}
|
||||
|
||||
public boolean isInputShutdown()
|
||||
{
|
||||
if (upgraded)
|
||||
return ProxySelectChannelEndPoint.super.isInputShutdown();
|
||||
else
|
||||
return plainEndPoint.isInputShutdown();
|
||||
}
|
||||
|
||||
public void close() throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
super.close();
|
||||
ProxySelectChannelEndPoint.super.close();
|
||||
else
|
||||
plainEndPoint.close();
|
||||
}
|
||||
|
@ -312,7 +469,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public int fill(Buffer buffer) throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
return super.fill(buffer);
|
||||
return ProxySelectChannelEndPoint.super.fill(buffer);
|
||||
else
|
||||
return plainEndPoint.fill(buffer);
|
||||
}
|
||||
|
@ -320,7 +477,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public int flush(Buffer buffer) throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
return super.flush(buffer);
|
||||
return ProxySelectChannelEndPoint.super.flush(buffer);
|
||||
else
|
||||
return plainEndPoint.flush(buffer);
|
||||
}
|
||||
|
@ -328,7 +485,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public int flush(Buffer header, Buffer buffer, Buffer trailer) throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
return super.flush(header, buffer, trailer);
|
||||
return ProxySelectChannelEndPoint.super.flush(header, buffer, trailer);
|
||||
else
|
||||
return plainEndPoint.flush(header, buffer, trailer);
|
||||
}
|
||||
|
@ -336,7 +493,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public String getLocalAddr()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getLocalAddr();
|
||||
return ProxySelectChannelEndPoint.super.getLocalAddr();
|
||||
else
|
||||
return plainEndPoint.getLocalAddr();
|
||||
}
|
||||
|
@ -344,7 +501,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public String getLocalHost()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getLocalHost();
|
||||
return ProxySelectChannelEndPoint.super.getLocalHost();
|
||||
else
|
||||
return plainEndPoint.getLocalHost();
|
||||
}
|
||||
|
@ -352,7 +509,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public int getLocalPort()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getLocalPort();
|
||||
return ProxySelectChannelEndPoint.super.getLocalPort();
|
||||
else
|
||||
return plainEndPoint.getLocalPort();
|
||||
}
|
||||
|
@ -360,7 +517,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public String getRemoteAddr()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getRemoteAddr();
|
||||
return ProxySelectChannelEndPoint.super.getRemoteAddr();
|
||||
else
|
||||
return plainEndPoint.getRemoteAddr();
|
||||
}
|
||||
|
@ -368,7 +525,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public String getRemoteHost()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getRemoteHost();
|
||||
return ProxySelectChannelEndPoint.super.getRemoteHost();
|
||||
else
|
||||
return plainEndPoint.getRemoteHost();
|
||||
}
|
||||
|
@ -376,7 +533,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public int getRemotePort()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getRemotePort();
|
||||
return ProxySelectChannelEndPoint.super.getRemotePort();
|
||||
else
|
||||
return plainEndPoint.getRemotePort();
|
||||
}
|
||||
|
@ -384,7 +541,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public boolean isBlocking()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.isBlocking();
|
||||
return ProxySelectChannelEndPoint.super.isBlocking();
|
||||
else
|
||||
return plainEndPoint.isBlocking();
|
||||
}
|
||||
|
@ -392,7 +549,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public boolean isBufferred()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.isBufferred();
|
||||
return ProxySelectChannelEndPoint.super.isBufferred();
|
||||
else
|
||||
return plainEndPoint.isBufferred();
|
||||
}
|
||||
|
@ -400,7 +557,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public boolean blockReadable(long millisecs) throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
return super.blockReadable(millisecs);
|
||||
return ProxySelectChannelEndPoint.super.blockReadable(millisecs);
|
||||
else
|
||||
return plainEndPoint.blockReadable(millisecs);
|
||||
}
|
||||
|
@ -408,7 +565,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public boolean blockWritable(long millisecs) throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
return super.blockWritable(millisecs);
|
||||
return ProxySelectChannelEndPoint.super.blockWritable(millisecs);
|
||||
else
|
||||
return plainEndPoint.blockWritable(millisecs);
|
||||
}
|
||||
|
@ -416,7 +573,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public boolean isOpen()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.isOpen();
|
||||
return ProxySelectChannelEndPoint.super.isOpen();
|
||||
else
|
||||
return plainEndPoint.isOpen();
|
||||
}
|
||||
|
@ -424,7 +581,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public Object getTransport()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getTransport();
|
||||
return ProxySelectChannelEndPoint.super.getTransport();
|
||||
else
|
||||
return plainEndPoint.getTransport();
|
||||
}
|
||||
|
@ -432,7 +589,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public boolean isBufferingInput()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.isBufferingInput();
|
||||
return ProxySelectChannelEndPoint.super.isBufferingInput();
|
||||
else
|
||||
return plainEndPoint.isBufferingInput();
|
||||
}
|
||||
|
@ -440,7 +597,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public boolean isBufferingOutput()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.isBufferingOutput();
|
||||
return ProxySelectChannelEndPoint.super.isBufferingOutput();
|
||||
else
|
||||
return plainEndPoint.isBufferingOutput();
|
||||
}
|
||||
|
@ -448,7 +605,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public void flush() throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
super.flush();
|
||||
ProxySelectChannelEndPoint.super.flush();
|
||||
else
|
||||
plainEndPoint.flush();
|
||||
|
||||
|
@ -457,7 +614,7 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public int getMaxIdleTime()
|
||||
{
|
||||
if (upgraded)
|
||||
return super.getMaxIdleTime();
|
||||
return ProxySelectChannelEndPoint.super.getMaxIdleTime();
|
||||
else
|
||||
return plainEndPoint.getMaxIdleTime();
|
||||
}
|
||||
|
@ -465,9 +622,10 @@ class SelectConnector extends AbstractLifeCycle implements HttpClient.Connector
|
|||
public void setMaxIdleTime(int timeMs) throws IOException
|
||||
{
|
||||
if (upgraded)
|
||||
super.setMaxIdleTime(timeMs);
|
||||
ProxySelectChannelEndPoint.super.setMaxIdleTime(timeMs);
|
||||
else
|
||||
plainEndPoint.setMaxIdleTime(timeMs);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -13,10 +13,8 @@
|
|||
|
||||
package org.eclipse.jetty.client;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
import java.net.ProtocolException;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
|
@ -31,27 +29,24 @@ import org.junit.After;
|
|||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
|
||||
/* ------------------------------------------------------------ */
|
||||
/**
|
||||
* This UnitTest class executes two tests. Both will send a http request to https://google.com through a misbehaving proxy server.
|
||||
*
|
||||
* <p/>
|
||||
* The first test runs against a proxy which simply closes the connection (as nginx does) for a connect request. The second proxy server always responds with a
|
||||
* 500 error.
|
||||
*
|
||||
* <p/>
|
||||
* The expected result for both tests is an exception and the HttpExchange should have status HttpExchange.STATUS_EXCEPTED.
|
||||
*/
|
||||
public class HttpsViaBrokenHttpProxyTest
|
||||
{
|
||||
|
||||
private Server _proxy = new Server();
|
||||
private HttpClient _client = new HttpClient();
|
||||
|
||||
/* ------------------------------------------------------------ */
|
||||
/**
|
||||
* @throws java.lang.Exception
|
||||
*/
|
||||
@Before
|
||||
public void setUpBeforeClass() throws Exception
|
||||
public void init() throws Exception
|
||||
{
|
||||
// setup proxies with different behaviour
|
||||
_proxy.addConnector(new SelectChannelConnector());
|
||||
|
@ -59,21 +54,54 @@ public class HttpsViaBrokenHttpProxyTest
|
|||
_proxy.start();
|
||||
int proxyClosingConnectionPort = _proxy.getConnectors()[0].getLocalPort();
|
||||
|
||||
_client.setProxy(new Address("localhost",proxyClosingConnectionPort));
|
||||
_client.setProxy(new Address("localhost", proxyClosingConnectionPort));
|
||||
_client.start();
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------ */
|
||||
/**
|
||||
* @throws java.lang.Exception
|
||||
*/
|
||||
@After
|
||||
public void tearDownAfterClass() throws Exception
|
||||
public void destroy() throws Exception
|
||||
{
|
||||
_client.stop();
|
||||
_proxy.stop();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void httpsViaProxyThatClosesConnectionOnConnectRequestTest() throws Exception
|
||||
{
|
||||
sendRequestThroughProxy(new ContentExchange(), "close", 9);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void httpsViaProxyThatReturns500ErrorTest() throws Exception
|
||||
{
|
||||
HttpExchange exchange = new ContentExchange()
|
||||
{
|
||||
@Override
|
||||
protected void onException(Throwable x)
|
||||
{
|
||||
// Suppress logging for expected exception
|
||||
if (!(x instanceof ProtocolException))
|
||||
super.onException(x);
|
||||
}
|
||||
};
|
||||
sendRequestThroughProxy(exchange, "error500", 9);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void httpsViaProxyThatReturns504ErrorTest() throws Exception
|
||||
{
|
||||
sendRequestThroughProxy(new ContentExchange(), "error504", 8);
|
||||
}
|
||||
|
||||
private void sendRequestThroughProxy(HttpExchange exchange, String desiredBehaviour, int exptectedStatus) throws Exception
|
||||
{
|
||||
String url = "https://" + desiredBehaviour + ".com/";
|
||||
exchange.setURL(url);
|
||||
exchange.addRequestHeader("behaviour", desiredBehaviour);
|
||||
_client.send(exchange);
|
||||
assertEquals(HttpExchange.toState(exptectedStatus) + " status awaited", exptectedStatus, exchange.waitForDone());
|
||||
}
|
||||
|
||||
private class BadBehavingConnectHandler extends ConnectHandler
|
||||
{
|
||||
@Override
|
||||
|
@ -81,7 +109,9 @@ public class HttpsViaBrokenHttpProxyTest
|
|||
throws ServletException, IOException
|
||||
{
|
||||
if (serverAddress.contains("close"))
|
||||
{
|
||||
HttpConnection.getCurrentConnection().getEndPoint().close();
|
||||
}
|
||||
else if (serverAddress.contains("error500"))
|
||||
{
|
||||
response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500);
|
||||
|
@ -93,39 +123,4 @@ public class HttpsViaBrokenHttpProxyTest
|
|||
baseRequest.setHandled(true);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void httpsViaProxyThatClosesConnectionOnConnectRequestTest()
|
||||
{
|
||||
sendRequestThroughProxy("close",9);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void httpsViaProxyThatReturns500ErrorTest() throws Exception
|
||||
{
|
||||
sendRequestThroughProxy("error500",9);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void httpsViaProxyThatReturns504ErrorTest() throws Exception
|
||||
{
|
||||
sendRequestThroughProxy("error504",8);
|
||||
}
|
||||
|
||||
private void sendRequestThroughProxy(String desiredBehaviour, int exptectedStatus)
|
||||
{
|
||||
String url = "https://" + desiredBehaviour + ".com/";
|
||||
try
|
||||
{
|
||||
ContentExchange exchange = new ContentExchange();
|
||||
exchange.setURL(url);
|
||||
exchange.addRequestHeader("behaviour",desiredBehaviour);
|
||||
_client.send(exchange);
|
||||
assertEquals(HttpExchange.toState(exptectedStatus) + " status awaited",exptectedStatus,exchange.waitForDone());
|
||||
}
|
||||
catch (Exception e)
|
||||
{
|
||||
System.out.println(e.getMessage());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue