You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ol...@apache.org on 2003/11/11 00:19:50 UTC
cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient ConnectMethod.java HttpClient.java HttpMethod.java HttpMethodDirector.java
olegk 2003/11/10 15:19:50
Modified: httpclient/src/java/org/apache/commons/httpclient
ConnectMethod.java HttpClient.java HttpMethod.java
HttpMethodDirector.java
Log:
PR#16729 (Allow redirects between hosts and ports)
Follow-up patch
HttpMethodDirector code cleanup
Contributed by Oleg Kalnichevski
Reviewed by Michael Becke
Revision Changes Path
1.22 +4 -8 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/ConnectMethod.java
Index: ConnectMethod.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/ConnectMethod.java,v
retrieving revision 1.21
retrieving revision 1.22
diff -u -r1.21 -r1.22
--- ConnectMethod.java 12 Aug 2003 02:35:17 -0000 1.21
+++ ConnectMethod.java 10 Nov 2003 23:19:49 -0000 1.22
@@ -196,10 +196,6 @@
if (LOG.isDebugEnabled()) {
LOG.debug("CONNECT status code " + code);
}
- if ((code >= 200) && (code < 300)) {
- conn.tunnelCreated();
- }
-
return code;
}
1.89 +19 -21 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpClient.java
Index: HttpClient.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpClient.java,v
retrieving revision 1.88
retrieving revision 1.89
diff -u -r1.88 -r1.89
--- HttpClient.java 26 Oct 2003 09:49:16 -0000 1.88
+++ HttpClient.java 10 Nov 2003 23:19:49 -0000 1.89
@@ -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();
}
/**
1.31 +18 -4 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethod.java
Index: HttpMethod.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethod.java,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -r1.30 -r1.31
--- HttpMethod.java 3 Oct 2003 20:57:35 -0000 1.30
+++ HttpMethod.java 10 Nov 2003 23:19:49 -0000 1.31
@@ -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);
+
}
1.8 +127 -206 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodDirector.java
Index: HttpMethodDirector.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodDirector.java,v
retrieving revision 1.7
retrieving revision 1.8
diff -u -r1.7 -r1.8
--- HttpMethodDirector.java 22 Oct 2003 19:31:00 -0000 1.7
+++ HttpMethodDirector.java 10 Nov 2003 23:19:49 -0000 1.8
@@ -86,24 +86,21 @@
private static final Log LOG = LogFactory.getLog(HttpMethodDirector.class);
- private HttpMethod method;
-
- private HttpState state;
+ private HttpMethod method;
+
+ private HttpState state;
- private HostConfiguration hostConfiguration;
+ private HostConfiguration hostConfiguration;
private HttpConnectionManager connectionManager;
- private HttpConnection connection;
-
private HttpClientParams params;
+ private HttpConnection conn;
+
/** A flag to indicate if the connection should be released after the 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,6 +112,20 @@
/** 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.
@@ -122,10 +133,14 @@
* @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 connection we have is appropriate
+ if (this.conn != null && !hostConfiguration.hostEquals(this.conn)) {
+ this.conn.setLocked(false);
+ this.conn.releaseConnection();
+ this.conn = null;
+ }
+
+ // get a connection, 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;
@@ -145,8 +179,8 @@
// retry - close previous stream. Caution - this causes
// responseBodyConsumed to be called, which may also close the
// connection.
- if (method.getResponseBodyAsStream() != null) {
- method.getResponseBodyAsStream().close();
+ if (this.method.getResponseBodyAsStream() != null) {
+ this.method.getResponseBodyAsStream().close();
}
} //end of retry loop
@@ -158,8 +192,8 @@
}
} 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
// to the pool. Use this flag, rather than other tests (like
@@ -167,10 +201,10 @@
// 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,79 +239,6 @@
}
/**
- * Makes sure there is a connection allocated and that it is valid and open.
- *
- * @return <code>true</code> if a valid connection was established,
- * <code>false</code> 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.
*
* @throws IOException if an I/O (transport) error occurs. Some transport exceptions
@@ -285,8 +246,11 @@
* @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;
@@ -297,29 +261,50 @@
execCount++;
requestSent = false;
- if (!establishValidOpenConnection()) {
- return;
- }
-
if (LOG.isTraceEnabled()) {
LOG.trace("Attempt number " + execCount + " to process request");
}
+ if (!this.conn.isOpen()) {
+ // this connection 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 method
+ if (!executeConnect()) {
+ // abort, the connect 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.");
}
- 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,
+ MethodRetryHandler handler = curmethod.getMethodRetryHandler();
+ if (handler == null) {
+ handler = new DefaultMethodRetryHandler();
+ }
+ if (!handler.retryMethod(
+ curmethod,
+ this.conn,
httpre,
execCount,
requestSent)
@@ -337,15 +322,6 @@
}
- private MethodRetryHandler getMethodRetryHandler() {
-
- if (method instanceof HttpMethodBase) {
- return ((HttpMethodBase) method).getMethodRetryHandler();
- } else {
- return new DefaultMethodRetryHandler();
- }
- }
-
/**
* Executes a ConnectMethod to establish a tunneled connection.
*
@@ -354,27 +330,15 @@
* @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.
@@ -397,8 +361,8 @@
// This will happen when the response body is consumed, or when
// the wrapped method closes the response connection in
// releaseConnection().
- if (tempMethod instanceof HttpMethodBase) {
- ((HttpMethodBase) tempMethod).fakeResponse(
+ if (this.method instanceof HttpMethodBase) {
+ ((HttpMethodBase) this.method).fakeResponse(
connectMethod.getStatusLine(),
connectMethod.getResponseHeaderGroup(),
connectMethod.getResponseBodyAsStream()
@@ -408,7 +372,6 @@
LOG.warn(
"Unable to fake response on 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()) {
@@ -498,28 +461,28 @@
* @return <code>true</code> if the request has completed processing, <code>false</code>
* 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;
@@ -566,15 +529,15 @@
//remove preemptive header and reauthenticate
switch (statusCode) {
case HttpStatus.SC_UNAUTHORIZED:
- method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
+ this.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);
+ this.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 <code>true</code> 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;
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org