HTTPCLIENT-757: Improved request wrapping in the DefaultClientRequestDirector. This also fixed the problem with the default proxy set at the client level having no effect.

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@645843 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Oleg Kalnichevski 2008-04-08 10:46:04 +00:00
parent fa366c559c
commit f2ef6c15a6
6 changed files with 108 additions and 44 deletions

View File

@ -1,6 +1,11 @@
Changes since 4.0 Alpha 3
-------------------
* [HTTPCLIENT-757] Improved request wrapping in the DefaultClientRequestDirector.
This also fixed the problem with the default proxy set at the client level
having no effect.
Contributed by Oleg Kalnichevski <olegk at apache.org>
* [HTTPCLIENT-734] Request abort will unblock the thread waiting for a connection
Contributed by Sam Berlin <sberlin at gmail.com>

View File

@ -216,17 +216,12 @@ public class DefaultClientRequestDirector
private RequestWrapper wrapRequest(
final HttpRequest request) throws ProtocolException {
try {
if (request instanceof HttpEntityEnclosingRequest) {
return new EntityEnclosingRequestWrapper(
(HttpEntityEnclosingRequest) request);
} else {
return new RequestWrapper(
request);
}
} catch (URISyntaxException ex) {
throw new ProtocolException("Invalid URI: " +
request.getRequestLine().getUri(), ex);
if (request instanceof HttpEntityEnclosingRequest) {
return new EntityEnclosingRequestWrapper(
(HttpEntityEnclosingRequest) request);
} else {
return new RequestWrapper(
request);
}
}
@ -264,9 +259,12 @@ public class DefaultClientRequestDirector
HttpContext context)
throws HttpException, IOException {
RoutedRequest roureq = determineRoute(target, request, context);
HttpRequest orig = request;
RequestWrapper origWrapper = wrapRequest(orig);
origWrapper.setParams(params);
HttpRoute origRoute = determineRoute(target, origWrapper, context);
final HttpRequest orig = request;
RoutedRequest roureq = new RoutedRequest.Impl(origWrapper, origRoute);
long timeout = HttpClientParams.getConnectionManagerTimeout(params);
@ -281,6 +279,7 @@ public class DefaultClientRequestDirector
// in the method arguments will be replaced. The original
// request is still available in 'orig'.
RequestWrapper wrapper = roureq.getRequest();
HttpRoute route = roureq.getRoute();
// Allocate connection if needed
@ -328,15 +327,11 @@ public class DefaultClientRequestDirector
}
}
// Wrap the request to be sent, original or followup.
RequestWrapper reqwrap = wrapRequest(roureq.getRequest());
reqwrap.setParams(this.params);
// Re-write request URI if needed
rewriteRequestURI(reqwrap, route);
rewriteRequestURI(wrapper, route);
// Use virtual host if set
target = (HttpHost) reqwrap.getParams().getParameter(
target = (HttpHost) wrapper.getParams().getParameter(
ClientPNames.VIRTUAL_HOST);
if (target == null) {
@ -356,17 +351,17 @@ public class DefaultClientRequestDirector
targetAuthState);
context.setAttribute(ClientContext.PROXY_AUTH_STATE,
proxyAuthState);
requestExec.preProcess(reqwrap, httpProcessor, context);
requestExec.preProcess(wrapper, httpProcessor, context);
context.setAttribute(ExecutionContext.HTTP_REQUEST,
reqwrap);
wrapper);
execCount++;
try {
if (LOG.isDebugEnabled()) {
LOG.debug("Attempt " + execCount + " to execute request");
}
response = requestExec.execute(reqwrap, managedConn, context);
response = requestExec.execute(wrapper, managedConn, context);
} catch (IOException ex) {
LOG.debug("Closing the connection.");
@ -386,11 +381,11 @@ public class DefaultClientRequestDirector
throw ex;
}
response.setParams(this.params);
response.setParams(params);
requestExec.postProcess(response, httpProcessor, context);
RoutedRequest followup =
handleResponse(roureq, reqwrap, response, context);
handleResponse(roureq, wrapper, response, context);
if (followup == null) {
done = true;
} else {
@ -466,11 +461,11 @@ public class DefaultClientRequestDirector
* @param context the context to use for the execution,
* never <code>null</code>
*
* @return the request along with the route it should take
* @return the route the request should take
*
* @throws HttpException in case of a problem
*/
protected RoutedRequest determineRoute(HttpHost target,
protected HttpRoute determineRoute(HttpHost target,
HttpRequest request,
HttpContext context)
throws HttpException {
@ -484,10 +479,7 @@ public class DefaultClientRequestDirector
("Target host must not be null, or set in parameters.");
}
final HttpRoute route =
this.routePlanner.determineRoute(target, request, context);
return new RoutedRequest.Impl(request, route);
return this.routePlanner.determineRoute(target, request, context);
}
@ -826,11 +818,14 @@ public class DefaultClientRequestDirector
uri.getScheme());
HttpGet redirect = new HttpGet(uri);
redirect.setParams(params);
RoutedRequest newRequest = determineRoute(newTarget, redirect, context);
RequestWrapper wrapper = new RequestWrapper(redirect);
wrapper.setParams(params);
HttpRoute newRoute = determineRoute(newTarget, wrapper, context);
RoutedRequest newRequest = new RoutedRequest.Impl(wrapper, newRoute);
if (LOG.isDebugEnabled()) {
LOG.debug("Redirecting to '" + uri + "' via " + newRequest.getRoute());
LOG.debug("Redirecting to '" + uri + "' via " + newRoute);
}
return newRequest;

View File

@ -31,11 +31,10 @@
package org.apache.http.impl.client;
import java.net.URISyntaxException;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpEntityEnclosingRequest;
import org.apache.http.ProtocolException;
import org.apache.http.protocol.HTTP;
/**
@ -58,7 +57,7 @@ class EntityEnclosingRequestWrapper extends RequestWrapper
private HttpEntity entity = null;
public EntityEnclosingRequestWrapper(final HttpEntityEnclosingRequest request)
throws URISyntaxException {
throws ProtocolException {
super(request);
this.entity = request.getEntity();
}

View File

@ -35,6 +35,7 @@ import java.net.URI;
import java.net.URISyntaxException;
import org.apache.http.HttpRequest;
import org.apache.http.ProtocolException;
import org.apache.http.ProtocolVersion;
import org.apache.http.RequestLine;
import org.apache.http.client.methods.HttpUriRequest;
@ -64,7 +65,7 @@ class RequestWrapper extends AbstractHttpMessage implements HttpUriRequest {
private String method;
private ProtocolVersion version;
public RequestWrapper(final HttpRequest request) throws URISyntaxException {
public RequestWrapper(final HttpRequest request) throws ProtocolException {
super();
if (request == null) {
throw new IllegalArgumentException("HTTP request may not be null");
@ -80,7 +81,12 @@ class RequestWrapper extends AbstractHttpMessage implements HttpUriRequest {
this.version = null;
} else {
RequestLine requestLine = request.getRequestLine();
this.uri = new URI(requestLine.getUri());
try {
this.uri = new URI(requestLine.getUri());
} catch (URISyntaxException ex) {
throw new ProtocolException("Invalid request URI: "
+ requestLine.getUri(), ex);
}
this.method = requestLine.getMethod();
this.version = request.getProtocolVersion();
}

View File

@ -31,7 +31,6 @@
package org.apache.http.impl.client;
import org.apache.http.HttpRequest;
import org.apache.http.conn.routing.HttpRoute;
@ -53,7 +52,7 @@ public interface RoutedRequest {
*
* @return the request
*/
HttpRequest getRequest()
RequestWrapper getRequest()
;
@ -71,7 +70,7 @@ public interface RoutedRequest {
*/
public static class Impl implements RoutedRequest {
protected final HttpRequest request;
protected final RequestWrapper request;
protected final HttpRoute route;
/**
@ -80,13 +79,13 @@ public interface RoutedRequest {
* @param req the request
* @param rou the route
*/
public Impl(HttpRequest req, HttpRoute rou) {
public Impl(RequestWrapper req, HttpRoute rou) {
this.request = req;
this.route = rou;
}
// non-javadoc, see interface
public final HttpRequest getRequest() {
public final RequestWrapper getRequest() {
return request;
}

View File

@ -38,6 +38,7 @@ import java.util.concurrent.atomic.AtomicReference;
import junit.framework.Test;
import junit.framework.TestSuite;
import org.apache.http.HttpEntity;
import org.apache.http.HttpException;
import org.apache.http.HttpHost;
import org.apache.http.HttpRequest;
@ -46,6 +47,7 @@ import org.apache.http.HttpStatus;
import org.apache.http.ProtocolVersion;
import org.apache.http.client.methods.AbortableHttpRequest;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.params.ClientPNames;
import org.apache.http.conn.ClientConnectionManager;
import org.apache.http.conn.ClientConnectionRequest;
import org.apache.http.conn.ConnectionPoolTimeoutException;
@ -555,5 +557,63 @@ public class TestDefaultClientRequestDirector extends ServerTestBase {
}
}
private class SimpleService implements HttpRequestHandler {
public SimpleService() {
super();
}
public void handle(
final HttpRequest request,
final HttpResponse response,
final HttpContext context) throws HttpException, IOException {
response.setStatusCode(HttpStatus.SC_OK);
StringEntity entity = new StringEntity("Whatever");
response.setEntity(entity);
}
}
public void testDefaultHostAtClientLevel() throws Exception {
int port = this.localServer.getServicePort();
this.localServer.register("*", new SimpleService());
HttpHost target = new HttpHost("localhost", port);
DefaultHttpClient client = new DefaultHttpClient();
client.getParams().setParameter(ClientPNames.DEFAULT_HOST, target);
String s = "/path";
HttpGet httpget = new HttpGet(s);
HttpResponse response = client.execute(httpget);
HttpEntity e = response.getEntity();
if (e != null) {
e.consumeContent();
}
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
}
public void testDefaultHostAtRequestLevel() throws Exception {
int port = this.localServer.getServicePort();
this.localServer.register("*", new SimpleService());
HttpHost target1 = new HttpHost("whatever", 80);
HttpHost target2 = new HttpHost("localhost", port);
DefaultHttpClient client = new DefaultHttpClient();
client.getParams().setParameter(ClientPNames.DEFAULT_HOST, target1);
String s = "/path";
HttpGet httpget = new HttpGet(s);
httpget.getParams().setParameter(ClientPNames.DEFAULT_HOST, target2);
HttpResponse response = client.execute(httpget);
HttpEntity e = response.getEntity();
if (e != null) {
e.consumeContent();
}
assertEquals(HttpStatus.SC_OK, response.getStatusLine().getStatusCode());
}
}