402341 - Host with default port causes redirects loop.

Removed default port from the Host request header.
Although allowed by RFC 2616, seems that many server chokes it.
This commit is contained in:
Simone Bordet 2013-03-04 16:32:31 +01:00
parent 8d6a4c39df
commit 868458f980
4 changed files with 44 additions and 13 deletions

View File

@ -29,7 +29,6 @@ import java.net.URI;
import java.nio.channels.SelectionKey;
import java.nio.channels.SocketChannel;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
@ -55,6 +54,7 @@ import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.io.ByteBufferPool;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.io.MappedByteBufferPool;
@ -476,7 +476,7 @@ public class HttpClient extends ContainerLifeCycle
protected void send(final Request request, List<Response.ResponseListener> listeners)
{
String scheme = request.getScheme().toLowerCase(Locale.ENGLISH);
if (!Arrays.asList("http", "https").contains(scheme))
if (!HttpScheme.HTTP.is(scheme) && !HttpScheme.HTTPS.is(scheme))
throw new IllegalArgumentException("Invalid protocol " + scheme);
HttpDestination destination = destinationFor(scheme, request.getHost(), request.getPort());
@ -903,8 +903,12 @@ public class HttpClient extends ContainerLifeCycle
protected int normalizePort(String scheme, int port)
{
return port > 0 ? port :
"https".equalsIgnoreCase(scheme) ? 443 : 80;
return port > 0 ? port : HttpScheme.HTTPS.is(scheme) ? 443 : 80;
}
protected boolean isDefaultPort(String scheme, int port)
{
return HttpScheme.HTTPS.is(scheme) ? port == 443 : port == 80;
}
protected HttpConnection newHttpConnection(HttpClient httpClient, EndPoint endPoint, HttpDestination destination)
@ -949,7 +953,7 @@ public class HttpClient extends ContainerLifeCycle
HttpDestination destination = callback.destination;
SslContextFactory sslContextFactory = getSslContextFactory();
if ("https".equals(destination.getScheme()))
if (HttpScheme.HTTPS.is(destination.getScheme()))
{
if (sslContextFactory == null)
{

View File

@ -85,7 +85,9 @@ public class HttpDestination implements Destination, AutoCloseable, Dumpable
proxyAddress = proxyConfig != null && proxyConfig.matches(host, port) ?
new Address(proxyConfig.getHost(), proxyConfig.getPort()) : null;
hostField = new HttpField(HttpHeader.HOST, host + ":" + port);
if (!client.isDefaultPort(scheme, port))
host += ":" + port;
hostField = new HttpField(HttpHeader.HOST, host);
}
protected BlockingQueue<Connection> getIdleConnections()
@ -462,7 +464,7 @@ public class HttpDestination implements Destination, AutoCloseable, Dumpable
public void succeeded(Connection connection)
{
boolean tunnel = isProxied() &&
"https".equalsIgnoreCase(getScheme()) &&
HttpScheme.HTTPS.is(getScheme()) &&
client.getSslContextFactory() != null;
if (tunnel)
tunnel(connection);

View File

@ -24,8 +24,10 @@ import java.net.URI;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.toolchain.test.TestTracker;
import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.junit.After;
@ -45,7 +47,7 @@ public class ExternalSiteTest
@Before
public void prepare() throws Exception
{
client = new HttpClient();
client = new HttpClient(new SslContextFactory());
client.start();
}
@ -64,7 +66,7 @@ public class ExternalSiteTest
// Verify that we have connectivity
try
{
new Socket(host, port);
new Socket(host, port).close();
}
catch (IOException x)
{
@ -113,7 +115,7 @@ public class ExternalSiteTest
// Verify that we have connectivity
try
{
new Socket(host, port);
new Socket(host, port).close();
}
catch (IOException x)
{
@ -142,7 +144,7 @@ public class ExternalSiteTest
// Verify that we have connectivity
try
{
new Socket(host, port);
new Socket(host, port).close();
}
catch (IOException x)
{
@ -179,4 +181,27 @@ public class ExternalSiteTest
Assert.assertTrue(latch.await(10, TimeUnit.SECONDS));
}
}
@Test
public void testExternalSiteRedirect() throws Exception
{
String host = "twitter.com";
int port = 443;
// Verify that we have connectivity
try
{
new Socket(host, port).close();
}
catch (IOException x)
{
Assume.assumeNoException(x);
}
ContentResponse response = client.newRequest(host, port)
.scheme(HttpScheme.HTTPS.asString())
.path("/twitter")
.send();
Assert.assertEquals(200, response.getStatus());
}
}

View File

@ -29,7 +29,6 @@ import java.util.Random;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@ -40,6 +39,7 @@ import org.eclipse.jetty.client.api.Response;
import org.eclipse.jetty.client.api.Result;
import org.eclipse.jetty.client.util.BytesContentProvider;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpScheme;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.toolchain.test.annotation.Slow;
import org.eclipse.jetty.toolchain.test.annotation.Stress;
@ -128,7 +128,7 @@ public class HttpClientLoadTest extends AbstractHttpClientServerTest
HttpMethod method = random.nextBoolean() ? HttpMethod.GET : HttpMethod.POST;
request.method(method);
boolean ssl = "https".equalsIgnoreCase(scheme);
boolean ssl = HttpScheme.HTTPS.is(scheme);
// Choose randomly whether to close the connection on the client or on the server
if (!ssl && random.nextBoolean())