Index: java/org/apache/commons/httpclient/ConnectMethod.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/ConnectMethod.java,v
retrieving revision 1.21
diff -u -r1.21 ConnectMethod.java
--- java/org/apache/commons/httpclient/ConnectMethod.java 12 Aug 2003 02:35:17 -0000 1.21
+++ java/org/apache/commons/httpclient/ConnectMethod.java 8 Nov 2003 21:31:26 -0000
@@ -196,10 +196,6 @@
if (LOG.isDebugEnabled()) {
LOG.debug("CONNECT status code " + code);
}
- if ((code >= 200) && (code < 300)) {
- conn.tunnelCreated();
- }
-
return code;
}
Index: java/org/apache/commons/httpclient/HttpClient.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpClient.java,v
retrieving revision 1.88
diff -u -r1.88 HttpClient.java
--- java/org/apache/commons/httpclient/HttpClient.java 26 Oct 2003 09:49:16 -0000 1.88
+++ java/org/apache/commons/httpclient/HttpClient.java 8 Nov 2003 21:31:29 -0000
@@ -423,21 +423,10 @@
}
HostConfiguration defaultHostConfiguration = null;
- HttpMethodDirector methodDirector = new HttpMethodDirector();
-
- /* access all synchronized data in a single block, this will keeps us
- * from accessing data asynchronously as well having to regain the lock
- * for each item.
- */
synchronized (this) {
- methodDirector.setParams(this.params);
- methodDirector.setState(state == null ? getState() : state);
- methodDirector.setConnectionManager(this.httpConnectionManager);
defaultHostConfiguration = getHostConfiguration();
}
-
HostConfiguration methodConfiguration = new HostConfiguration(hostConfiguration);
-
if (hostConfiguration != defaultHostConfiguration) {
// we may need to apply some defaults
if (!methodConfiguration.isHostSet()) {
@@ -463,12 +452,21 @@
}
}
- methodDirector.setHostConfiguration(methodConfiguration);
- methodDirector.setMethod(method);
-
- methodDirector.executeMethod();
-
- return methodDirector.getMethod().getStatusCode();
+ /* access all synchronized data in a single block, this will keeps us
+ * from accessing data asynchronously as well having to regain the lock
+ * for each item.
+ */
+ HttpMethodDirector methodDirector = null;
+ synchronized (this) {
+ methodDirector = new HttpMethodDirector(
+ this.httpConnectionManager,
+ methodConfiguration,
+ this.params,
+ (state == null ? getState() : state));
+ defaultHostConfiguration = getHostConfiguration();
+ }
+ methodDirector.executeMethod(method);
+ return method.getStatusCode();
}
/**
Index: java/org/apache/commons/httpclient/HttpMethod.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethod.java,v
retrieving revision 1.30
diff -u -r1.30 HttpMethod.java
--- java/org/apache/commons/httpclient/HttpMethod.java 3 Oct 2003 20:57:35 -0000 1.30
+++ java/org/apache/commons/httpclient/HttpMethod.java 8 Nov 2003 21:31:32 -0000
@@ -540,4 +540,18 @@
*/
public void setParams(final HttpMethodParams params);
+ /**
+ * Returns the {@link MethodRetryHandler retry handler} for this HTTP method
+ *
+ * @return the methodRetryHandler
+ */
+ public MethodRetryHandler getMethodRetryHandler();
+
+ /**
+ * Sets the {@link MethodRetryHandler retry handler} for this HTTP method
+ *
+ * @param handler the methodRetryHandler to use when this method executed
+ */
+ public void setMethodRetryHandler(MethodRetryHandler handler);
+
}
Index: java/org/apache/commons/httpclient/HttpMethodDirector.java
===================================================================
RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodDirector.java,v
retrieving revision 1.7
diff -u -r1.7 HttpMethodDirector.java
--- java/org/apache/commons/httpclient/HttpMethodDirector.java 22 Oct 2003 19:31:00 -0000 1.7
+++ java/org/apache/commons/httpclient/HttpMethodDirector.java 8 Nov 2003 21:31:36 -0000
@@ -77,7 +77,7 @@
import org.apache.commons.logging.LogFactory;
/**
- * Handles the process of executing a method including authentication, redirection and retries.
+ * Handles the process of executing a this.method including authentication, redirection and retries.
*/
class HttpMethodDirector {
@@ -86,24 +86,21 @@
private static final Log LOG = LogFactory.getLog(HttpMethodDirector.class);
- private HttpMethod method;
-
+ private HttpMethod method;
+
private HttpState state;
private HostConfiguration hostConfiguration;
private HttpConnectionManager connectionManager;
- private HttpConnection connection;
-
private HttpClientParams params;
- /** A flag to indicate if the connection should be released after the method is executed. */
+ private HttpConnection conn;
+
+ /** A flag to indicate if the this.conn should be released after the this.method is executed. */
private boolean releaseConnection = false;
- /** How many times did this transparently handle a recoverable exception? */
- private int recoverableExceptionCount = 0;
-
/** Realms that we tried to authenticate to */
private Set realms = null;
@@ -115,17 +112,35 @@
/** Actual proxy authentication realm */
private String proxyRealm = null;
+
+ public HttpMethodDirector(
+ final HttpConnectionManager connectionManager,
+ final HostConfiguration hostConfiguration,
+ final HttpClientParams params,
+ final HttpState state
+ ) {
+ super();
+ this.connectionManager = connectionManager;
+ this.hostConfiguration = hostConfiguration;
+ this.params = params;
+ this.state = state;
+ }
+
/**
- * Executes the method associated with this method director.
+ * Executes the this.method associated with this this.method director.
*
* @throws IOException
* @throws HttpException
*/
- public void executeMethod() throws IOException, HttpException {
-
- method.getParams().setDefaults(this.params);
+ public void executeMethod(final HttpMethod method) throws IOException, HttpException {
+ if (method == null)
+ {
+ throw new IllegalArgumentException("Method may not be null");
+ }
+ this.method = method;
+ this.method.getParams().setDefaults(this.params);
try {
int forwardCount = 0; //protect from an infinite loop
@@ -135,8 +150,27 @@
LOG.debug("Execute loop try " + forwardCount);
}
- executeMethodForHost();
+ // make sure the this.conn we have is appropriate
+ if (this.conn != null && !hostConfiguration.hostEquals(this.conn)) {
+ this.conn.setLocked(false);
+ this.conn.releaseConnection();
+ this.conn = null;
+ }
+
+ // get a this.conn, if we need one
+ if (this.conn == null) {
+ this.conn = connectionManager.getConnectionWithTimeout(
+ hostConfiguration,
+ this.params.getConnectionManagerTimeout()
+ );
+ this.conn.setLocked(true);
+ realms = new HashSet();
+ proxyRealms = new HashSet();
+
+ addPreemtiveAuthenticationHeaders();
+ }
+ executeWithRetry(this.method);
if (!isRetryNeeded()) {
// nope, no retry needed, exit loop.
break;
@@ -144,9 +178,9 @@
// retry - close previous stream. Caution - this causes
// responseBodyConsumed to be called, which may also close the
- // connection.
- if (method.getResponseBodyAsStream() != null) {
- method.getResponseBodyAsStream().close();
+ // this.conn.
+ if (this.method.getResponseBodyAsStream() != null) {
+ this.method.getResponseBodyAsStream().close();
}
} //end of retry loop
@@ -158,19 +192,19 @@
}
} finally {
- if (connection != null) {
- connection.setLocked(false);
+ if (this.conn != null) {
+ this.conn.setLocked(false);
}
- // If the response has been fully processed, return the connection
+ // If the response has been fully processed, return the this.conn
// to the pool. Use this flag, rather than other tests (like
// responseStream == null), as subclasses, might reset the stream,
// for example, reading the entire response into a file and then
// setting the file as the stream.
if (
- (releaseConnection || method.getResponseBodyAsStream() == null)
- && connection != null
+ (releaseConnection || this.method.getResponseBodyAsStream() == null)
+ && this.conn != null
) {
- connection.releaseConnection();
+ this.conn.releaseConnection();
}
}
@@ -189,11 +223,11 @@
LOG.debug("Preemptively sending default basic credentials");
try {
- if (HttpAuthenticator.authenticateDefault(method, connection, state)) {
+ if (HttpAuthenticator.authenticateDefault(this.method, this.conn, state)) {
LOG.debug("Default basic credentials applied");
}
- if (connection.isProxied()) {
- if (HttpAuthenticator.authenticateProxyDefault(method, connection, state)) {
+ if (this.conn.isProxied()) {
+ if (HttpAuthenticator.authenticateProxyDefault(this.method, this.conn, state)) {
LOG.debug("Default basic proxy credentials applied");
}
}
@@ -205,121 +239,72 @@
}
/**
- * Makes sure there is a connection allocated and that it is valid and open.
- *
- * @return true if a valid connection was established,
- * false otherwise
- *
- * @throws IOException
- * @throws HttpException
- */
- private boolean establishValidOpenConnection() throws IOException, HttpException {
-
- // make sure the connection we have is appropriate
- if (connection != null && !hostConfiguration.hostEquals(connection)) {
- connection.setLocked(false);
- connection.releaseConnection();
- connection = null;
- }
-
- // get a connection, if we need one
- if (connection == null) {
- connection = connectionManager.getConnectionWithTimeout(
- hostConfiguration,
- this.params.getConnectionManagerTimeout()
- );
- connection.setLocked(true);
-
- realms = new HashSet();
- proxyRealms = new HashSet();
-
- addPreemtiveAuthenticationHeaders();
- }
-
- try {
- // Catch all possible exceptions to make sure to release the
- // connection, as although the user may call
- // Method->releaseConnection(), the method doesn't know about the
- // connection until HttpMethod.execute() is called.
-
- if (!connection.isOpen()) {
- // this connection must be opened before it can be used
- connection.open();
- if (connection.isProxied() && connection.isSecure()) {
- // we need to create a secure tunnel before we can execute the real method
- if (!executeConnect()) {
- // abort, the connect method failed
- return false;
- }
- }
- } else if (
- !(method instanceof ConnectMethod)
- && connection.isProxied()
- && connection.isSecure()
- && !connection.isTransparent()
- ) {
- // this connection is open but the secure tunnel has not be created yet,
- // execute the connect again
- if (!executeConnect()) {
- // abort, the connect method failed
- return false;
- }
- }
-
- } catch (IOException e) {
- releaseConnection = true;
- throw e;
- } catch (RuntimeException e) {
- releaseConnection = true;
- throw e;
- }
-
- return true;
- }
-
- /**
- * Executes a method with the current hostConfiguration.
+ * Executes a this.method with the current hostConfiguration.
*
* @throws IOException if an I/O (transport) error occurs. Some transport exceptions
* can be recovered from.
* @throws HttpException if a protocol exception occurs. Usually protocol exceptions
* cannot be recovered from.
*/
- private void executeMethodForHost() throws IOException, HttpException {
+ private void executeWithRetry(final HttpMethod curmethod)
+ throws IOException, HttpException {
+ /** How many times did this transparently handle a recoverable exception? */
+ int recoverableExceptionCount = 0;
int execCount = 0;
// TODO: how do we get requestSent?
boolean requestSent = false;
- // loop until the method is successfully processed, the retryHandler
+ // loop until the this.method is successfully processed, the retryHandler
// returns false or a non-recoverable exception is thrown
while (true) {
execCount++;
requestSent = false;
- if (!establishValidOpenConnection()) {
- return;
- }
-
if (LOG.isTraceEnabled()) {
LOG.trace("Attempt number " + execCount + " to process request");
}
+ if (!this.conn.isOpen()) {
+ // this this.conn must be opened before it can be used
+ // This has nothing to do with opening a secure tunnel
+ try {
+ this.conn.open();
+ if (this.conn.isProxied() && this.conn.isSecure()
+ && !(curmethod instanceof ConnectMethod)) {
+ // we need to create a secure tunnel before we can execute the real this.method
+ if (!executeConnect()) {
+ // abort, the connect this.method failed
+ break;
+ }
+ }
+ } catch (IOException e) {
+ releaseConnection = true;
+ throw e;
+ } catch (RuntimeException e) {
+ releaseConnection = true;
+ throw e;
+ }
+ }
try {
- method.execute(state, connection);
+ curmethod.execute(state, this.conn);
break;
} catch (HttpRecoverableException httpre) {
if (LOG.isDebugEnabled()) {
- LOG.debug("Closing the connection.");
+ LOG.debug("Closing the this.conn.");
}
- connection.close();
+ this.conn.close();
LOG.info("Recoverable exception caught when processing request");
// update the recoverable exception count.
recoverableExceptionCount++;
- // test if this method should be retried
- if (!getMethodRetryHandler().retryMethod(
- method,
- connection,
+ // test if this this.method should be retried
+ MethodRetryHandler handler = curmethod.getMethodRetryHandler();
+ if (handler == null) {
+ handler = new DefaultMethodRetryHandler();
+ }
+ if (!handler.retryMethod(
+ curmethod,
+ this.conn,
httpre,
execCount,
requestSent)
@@ -328,7 +313,7 @@
"Recoverable exception caught but MethodRetryHandler.retryMethod() "
+ "returned false, rethrowing exception"
);
- // this connection can no longer be used, it has been closed
+ // this this.conn can no longer be used, it has been closed
releaseConnection = true;
throw httpre;
}
@@ -337,68 +322,47 @@
}
- private MethodRetryHandler getMethodRetryHandler() {
-
- if (method instanceof HttpMethodBase) {
- return ((HttpMethodBase) method).getMethodRetryHandler();
- } else {
- return new DefaultMethodRetryHandler();
- }
- }
-
/**
- * Executes a ConnectMethod to establish a tunneled connection.
+ * Executes a ConnectMethod to establish a tunneled this.conn.
*
* @return true if the connect was successful
*
* @throws IOException
* @throws HttpException
*/
- private boolean executeConnect() throws IOException, HttpException {
+ private boolean executeConnect()
+ throws IOException, HttpException {
ConnectMethod connectMethod = new ConnectMethod();
-
- HttpMethod tempMethod = this.method;
- this.method = connectMethod;
-
- try {
- executeMethod();
- } catch (HttpException e) {
- this.method = tempMethod;
- throw e;
- } catch (IOException e) {
- this.method = tempMethod;
- throw e;
- }
-
- int code = method.getStatusCode();
+ executeWithRetry(connectMethod);
+ int code = connectMethod.getStatusCode();
if ((code >= 200) && (code < 300)) {
- this.method = tempMethod;
+ this.conn.tunnelCreated();
return true;
} else {
// What is to follow is an ugly hack.
// I REALLY hate having to resort to such
// an appalling trick
- // TODO: Connect method must be redesigned.
+ // TODO: Connect this.method must be redesigned.
// The only feasible solution is to split monolithic
// HttpMethod into HttpRequest/HttpResponse pair.
- // That would allow to execute CONNECT method
+ // That would allow to execute CONNECT this.method
// behind the scene and return CONNECT HttpResponse
// object in response to the original request that
// contains the correct status line, headers &
// response body.
- LOG.debug("CONNECT failed, fake the response for the original method");
+ LOG.debug("CONNECT failed, fake the response for the original this.method");
// Pass the status, headers and response stream to the wrapped
- // method.
- // To ensure that the connection is not released more than once
- // this method is still responsible for releasing the connection.
+ // this.method.
+ // To ensure that the this.conn is not released more than once
+ // this this.method is still responsible for releasing the this.conn.
// This will happen when the response body is consumed, or when
- // the wrapped method closes the response connection in
+ // the wrapped this.method closes the response this.conn in
// releaseConnection().
- if (tempMethod instanceof HttpMethodBase) {
- ((HttpMethodBase) tempMethod).fakeResponse(
+ if (this.method instanceof HttpMethodBase) {
+ ((HttpMethodBase) this.method).fakeResponse(
connectMethod.getStatusLine(),
connectMethod.getResponseHeaderGroup(),
connectMethod.getResponseBodyAsStream()
@@ -406,9 +370,8 @@
} else {
releaseConnection = true;
LOG.warn(
- "Unable to fake response on method as it is not derived from HttpMethodBase.");
+ "Unable to fake response on this.method as it is not derived from HttpMethodBase.");
}
- this.method = tempMethod;
return false;
}
}
@@ -420,17 +383,17 @@
*/
private boolean processRedirectResponse() {
- if (!method.getFollowRedirects()) {
+ if (!this.method.getFollowRedirects()) {
LOG.info("Redirect requested but followRedirects is "
+ "disabled");
return false;
}
//get the location header to find out where to redirect to
- Header locationHeader = method.getResponseHeader("location");
+ Header locationHeader = this.method.getResponseHeader("location");
if (locationHeader == null) {
// got a redirect response, but no location header
- LOG.error("Received redirect response " + method.getStatusCode()
+ LOG.error("Received redirect response " + this.method.getStatusCode()
+ " but no location header");
return false;
}
@@ -447,15 +410,15 @@
try {
currentUri = new URI(
- connection.getProtocol().getScheme(),
+ this.conn.getProtocol().getScheme(),
null,
- connection.getHost(),
- connection.getPort(),
- method.getPath()
+ this.conn.getHost(),
+ this.conn.getPort(),
+ this.method.getPath()
);
redirectUri = new URI(location, true);
if (redirectUri.isRelativeURI()) {
- if (method.isStrictMode()) {
+ if (this.method.isStrictMode()) {
LOG.warn("Redirected location '" + location
+ "' is not acceptable in strict mode");
return false;
@@ -473,12 +436,12 @@
//invalidate the list of authentication attempts
this.realms.clear();
//remove exisitng authentication headers
- method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
+ this.method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
//update the current location with the redirect location.
//avoiding use of URL.getPath() and URL.getQuery() to keep
//jdk1.2 comliance.
- method.setPath(redirectUri.getEscapedPath());
- method.setQueryString(redirectUri.getEscapedQuery());
+ this.method.setPath(redirectUri.getEscapedPath());
+ this.method.setQueryString(redirectUri.getEscapedQuery());
hostConfiguration.setHost(redirectUri);
if (LOG.isDebugEnabled()) {
@@ -493,33 +456,33 @@
* Processes a response that requires authentication
*
* @param state the current state
- * @param conn The connection
+ * @param conn The this.conn
*
* @return true if the request has completed processing, false
* if more attempts are needed
*/
- private boolean processAuthenticationResponse(HttpState state, HttpConnection conn) {
+ private boolean processAuthenticationResponse() {
LOG.trace("enter HttpMethodBase.processAuthenticationResponse("
+ "HttpState, HttpConnection)");
- int statusCode = method.getStatusCode();
+ int statusCode = this.method.getStatusCode();
// handle authentication required
Header[] challenges = null;
Set realmsUsed = null;
String host = null;
switch (statusCode) {
case HttpStatus.SC_UNAUTHORIZED:
- challenges = method.getResponseHeaders(HttpAuthenticator.WWW_AUTH);
+ challenges = this.method.getResponseHeaders(HttpAuthenticator.WWW_AUTH);
realmsUsed = realms;
- host = conn.getVirtualHost();
+ host = this.conn.getVirtualHost();
if (host == null) {
- host = conn.getHost();
+ host = this.conn.getHost();
}
break;
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
- challenges = method.getResponseHeaders(HttpAuthenticator.PROXY_AUTH);
+ challenges = this.method.getResponseHeaders(HttpAuthenticator.PROXY_AUTH);
realmsUsed = proxyRealms;
- host = conn.getProxyHost();
+ host = this.conn.getProxyHost();
break;
}
boolean authenticated = false;
@@ -554,7 +517,7 @@
buffer.append("' authentication realm at ");
buffer.append(host);
buffer.append(", but still receiving: ");
- buffer.append(method.getStatusLine().toString());
+ buffer.append(this.method.getStatusLine().toString());
LOG.info(buffer.toString());
}
return true;
@@ -562,19 +525,19 @@
realmsUsed.add(realm);
}
+ this.method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
+ this.method.removeRequestHeader(HttpAuthenticator.PROXY_AUTH_RESP);
try {
//remove preemptive header and reauthenticate
switch (statusCode) {
case HttpStatus.SC_UNAUTHORIZED:
- method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
authenticated = HttpAuthenticator.authenticate(
- authscheme, method, conn, state);
+ authscheme, this.method, this.conn, this.state);
this.realm = authscheme.getRealm();
break;
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
- method.removeRequestHeader(HttpAuthenticator.PROXY_AUTH_RESP);
authenticated = HttpAuthenticator.authenticateProxy(
- authscheme, method, conn, state);
+ authscheme, this.method, this.conn, this.state);
this.proxyRealm = authscheme.getRealm();
break;
}
@@ -611,14 +574,14 @@
* @return boolean true if a retry is needed.
*/
private boolean isRetryNeeded() {
- switch (method.getStatusCode()) {
+ switch (this.method.getStatusCode()) {
case HttpStatus.SC_UNAUTHORIZED:
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
LOG.debug("Authorization required");
- if (method.getDoAuthentication()) { //process authentication response
+ if (this.method.getDoAuthentication()) { //process authentication response
//if the authentication is successful, return the statusCode
//otherwise, drop through the switch and try again.
- if (processAuthenticationResponse(state, connection)) {
+ if (processAuthenticationResponse()) {
return false;
}
} else { //let the client handle the authenticaiton
@@ -653,27 +616,6 @@
}
/**
- * @param hostConfiguration
- */
- public void setHostConfiguration(HostConfiguration hostConfiguration) {
- this.hostConfiguration = hostConfiguration;
- }
-
- /**
- * @return
- */
- public HttpMethod getMethod() {
- return method;
- }
-
- /**
- * @param method
- */
- public void setMethod(HttpMethod method) {
- this.method = method;
- }
-
- /**
* @return
*/
public HttpState getState() {
@@ -681,13 +623,6 @@
}
/**
- * @param state
- */
- public void setState(HttpState state) {
- this.state = state;
- }
-
- /**
* @return
*/
public HttpConnectionManager getConnectionManager() {
@@ -695,24 +630,10 @@
}
/**
- * @param connectionManager
- */
- public void setConnectionManager(HttpConnectionManager connectionManager) {
- this.connectionManager = connectionManager;
- }
-
- /**
* @return
*/
public HttpParams getParams() {
return this.params;
- }
-
- /**
- * @param params
- */
- public void setParams(final HttpClientParams params) {
- this.params = params;
}
}