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/19 22:11:16 UTC
cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient HttpMethod.java HttpMethodDirector.java
olegk 2003/11/19 13:11:16
Modified: httpclient/src/examples TrivialApp.java
httpclient/src/java/org/apache/commons/httpclient
HttpMethod.java HttpMethodDirector.java
Log:
PR #16729 (Allow redirects between hosts and ports)
- Fixes breakage in the proxy authentication logic
- Further code cleanup
Contributed by Oleg Kalnichevski
Revision Changes Path
1.16 +3 -4 jakarta-commons/httpclient/src/examples/TrivialApp.java
Index: TrivialApp.java
===================================================================
RCS file: /home/cvs/jakarta-commons/httpclient/src/examples/TrivialApp.java,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -r1.15 -r1.16
--- TrivialApp.java 26 Oct 2003 09:49:16 -0000 1.15
+++ TrivialApp.java 19 Nov 2003 21:11:16 -0000 1.16
@@ -124,7 +124,6 @@
//create a method object
method = new GetMethod(url);
method.setFollowRedirects(true);
- method.setStrictMode(false);
//} catch (MalformedURLException murle) {
// System.out.println("<url> argument '" + url
// + "' is not a valid URL");
1.32 +10 -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.31
retrieving revision 1.32
diff -u -r1.31 -r1.32
--- HttpMethod.java 10 Nov 2003 23:19:49 -0000 1.31
+++ HttpMethod.java 19 Nov 2003 21:11:16 -0000 1.32
@@ -154,6 +154,9 @@
*
* @param strictMode <tt>true</tt> for strict mode, <tt>false</tt> otherwise
*
+ * @deprecated Use {@link HttpParams#setParameter(String, Object)} to exercise
+ * a more granular control over HTTP protocol strictness.
+ *
* @see #isStrictMode()
*/
void setStrictMode(boolean strictMode);
@@ -162,6 +165,9 @@
* Returns the value of the strict mode flag.
*
* @return <tt>true</tt> if strict mode is enabled, <tt>false</tt> otherwise
+ *
+ * @deprecated Use {@link HttpParams#setParameter(String, Object)} to exercise
+ * a more granular control over HTTP protocol strictness.
*
* @see #setStrictMode(boolean)
*/
1.10 +190 -130 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.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- HttpMethodDirector.java 13 Nov 2003 22:24:47 -0000 1.9
+++ HttpMethodDirector.java 19 Nov 2003 21:11:16 -0000 1.10
@@ -86,7 +86,7 @@
private static final Log LOG = LogFactory.getLog(HttpMethodDirector.class);
- private HttpMethod method;
+ private ConnectMethod connectMethod;
private HttpState state;
@@ -139,8 +139,7 @@
{
throw new IllegalArgumentException("Method may not be null");
}
- this.method = method;
- this.method.getParams().setDefaults(this.params);
+ method.getParams().setDefaults(this.params);
try {
int forwardCount = 0; //protect from an infinite loop
@@ -168,10 +167,26 @@
realms = new HashSet();
proxyRealms = new HashSet();
- addPreemtiveAuthenticationHeaders();
+ addPreemtiveAuthenticationHeaders(method);
}
- executeWithRetry(this.method);
- if (!isRetryNeeded()) {
+ executeWithRetry(method);
+ if (this.connectMethod != null) {
+ //CONNECT method failed. Bow out.
+ fakeResponse(method);
+ break;
+ } else if (isRedirectNeeded(method)) {
+ //if the redirect is unsuccessful, return the statusCode
+ //otherwise, drop through the switch and try again.
+ if (!processRedirectResponse(method)) {
+ break;
+ }
+ } else if (isAuthenticationNeeded(method)) {
+ //if the authentication is unsuccessful, return the statusCode
+ //otherwise, drop through the switch and try again.
+ if (!processAuthenticationResponse(method)) {
+ break;
+ }
+ } else {
// nope, no retry needed, exit loop.
break;
}
@@ -179,8 +194,8 @@
// retry - close previous stream. Caution - this causes
// responseBodyConsumed to be called, which may also close the
// connection.
- if (this.method.getResponseBodyAsStream() != null) {
- this.method.getResponseBodyAsStream().close();
+ if (method.getResponseBodyAsStream() != null) {
+ method.getResponseBodyAsStream().close();
}
} //end of retry loop
@@ -201,7 +216,7 @@
// for example, reading the entire response into a file and then
// setting the file as the stream.
if (
- (releaseConnection || this.method.getResponseBodyAsStream() == null)
+ (releaseConnection || method.getResponseBodyAsStream() == null)
&& this.conn != null
) {
this.conn.releaseConnection();
@@ -210,30 +225,42 @@
}
+ private void applyDefaultCredentials(final HttpMethod method) {
+ try {
+ if (HttpAuthenticator.authenticateDefault(method, this.conn, state)) {
+ LOG.debug("Default basic credentials applied");
+ }
+ } catch (AuthenticationException e) {
+ // Log error and move on
+ LOG.error(e.getMessage(), e);
+ }
+ }
+
+ private void applyDefaultProxyCredentials(final HttpMethod method) {
+ try {
+ if (HttpAuthenticator.authenticateProxyDefault(method, this.conn, state)) {
+ LOG.debug("Default basic proxy credentials applied");
+ }
+ } catch (AuthenticationException e) {
+ // Log error and move on
+ LOG.error(e.getMessage(), e);
+ }
+ }
+
/**
* Adds authentication headers if <code>authenticationPreemtive</code> has been set.
*
- * @see HttpState#isAuthenticationPreemptive()
+ * @see HttpClientParams#isAuthenticationPreemptive()
*/
- private void addPreemtiveAuthenticationHeaders() {
-
+ private void addPreemtiveAuthenticationHeaders(final HttpMethod method) {
+
//pre-emptively add the authorization header, if required.
if (this.params.isAuthenticationPreemptive()) {
LOG.debug("Preemptively sending default basic credentials");
-
- try {
- if (HttpAuthenticator.authenticateDefault(this.method, this.conn, state)) {
- LOG.debug("Default basic credentials applied");
- }
- if (this.conn.isProxied()) {
- if (HttpAuthenticator.authenticateProxyDefault(this.method, this.conn, state)) {
- LOG.debug("Default basic proxy credentials applied");
- }
- }
- } catch (AuthenticationException e) {
- // Log error and move on
- LOG.error(e.getMessage(), e);
+ applyDefaultCredentials(method);
+ if (this.conn.isProxied()) {
+ applyDefaultProxyCredentials(method);
}
}
}
@@ -246,7 +273,7 @@
* @throws HttpException if a protocol exception occurs. Usually protocol exceptions
* cannot be recovered from.
*/
- private void executeWithRetry(final HttpMethod curmethod)
+ private void executeWithRetry(final HttpMethod method)
throws IOException, HttpException {
/** How many times did this transparently handle a recoverable exception? */
@@ -270,11 +297,11 @@
try {
this.conn.open();
if (this.conn.isProxied() && this.conn.isSecure()
- && !(curmethod instanceof ConnectMethod)) {
+ && !(method instanceof ConnectMethod)) {
// we need to create a secure tunnel before we can execute the real method
if (!executeConnect()) {
// abort, the connect method failed
- break;
+ return;
}
}
} catch (IOException e) {
@@ -286,7 +313,7 @@
}
}
try {
- curmethod.execute(state, this.conn);
+ method.execute(state, this.conn);
break;
} catch (HttpRecoverableException httpre) {
if (LOG.isDebugEnabled()) {
@@ -298,12 +325,12 @@
recoverableExceptionCount++;
// test if this method should be retried
- MethodRetryHandler handler = curmethod.getMethodRetryHandler();
+ MethodRetryHandler handler = method.getMethodRetryHandler();
if (handler == null) {
handler = new DefaultMethodRetryHandler();
}
if (!handler.retryMethod(
- curmethod,
+ method,
this.conn,
httpre,
execCount,
@@ -333,66 +360,89 @@
private boolean executeConnect()
throws IOException, HttpException {
- ConnectMethod connectMethod = new ConnectMethod();
- executeWithRetry(connectMethod);
- int code = connectMethod.getStatusCode();
+ this.connectMethod = new ConnectMethod();
+ this.connectMethod.getParams().setDefaults(this.params);
+ int code;
+ if (this.params.isAuthenticationPreemptive()) {
+ applyDefaultProxyCredentials(this.connectMethod);
+ }
+ for (;;) {
+ executeWithRetry(this.connectMethod);
+ code = this.connectMethod.getStatusCode();
+ if (code == HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED) {
+ if (!processAuthenticationResponse(this.connectMethod)) {
+ break;
+ }
+ } else {
+ break;
+ }
+ if (this.connectMethod.getResponseBodyAsStream() != null) {
+ this.connectMethod.getResponseBodyAsStream().close();
+ }
+ }
if ((code >= 200) && (code < 300)) {
this.conn.tunnelCreated();
+ // Drop the connect method, as it is no longer needed
+ this.connectMethod = null;
return true;
} else {
- // What is to follow is an ugly hack.
- // I REALLY hate having to resort to such
- // an appalling trick
- // The only feasible solution is to split monolithic
- // HttpMethod into HttpRequest/HttpResponse pair.
- // That would allow to execute CONNECT 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");
- // 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 will happen when the response body is consumed, or when
- // the wrapped method closes the response connection in
- // releaseConnection().
- if (this.method instanceof HttpMethodBase) {
- ((HttpMethodBase) this.method).fakeResponse(
- connectMethod.getStatusLine(),
- connectMethod.getResponseHeaderGroup(),
- connectMethod.getResponseBodyAsStream()
- );
- } else {
- releaseConnection = true;
- LOG.warn(
- "Unable to fake response on method as it is not derived from HttpMethodBase.");
- }
return false;
}
}
+
+ /**
+ * Fake response
+ * @param method
+ * @return
+ */
+
+ private void fakeResponse(final HttpMethod method)
+ throws IOException, HttpException {
+ // What is to follow is an ugly hack.
+ // I REALLY hate having to resort to such
+ // an appalling trick
+ // The only feasible solution is to split monolithic
+ // HttpMethod into HttpRequest/HttpResponse pair.
+ // That would allow to execute CONNECT 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");
+ // 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 will happen when the response body is consumed, or when
+ // the wrapped method closes the response connection in
+ // releaseConnection().
+ if (method instanceof HttpMethodBase) {
+ ((HttpMethodBase) method).fakeResponse(
+ this.connectMethod.getStatusLine(),
+ this.connectMethod.getResponseHeaderGroup(),
+ this.connectMethod.getResponseBodyAsStream()
+ );
+ this.connectMethod = null;
+ } else {
+ releaseConnection = true;
+ LOG.warn(
+ "Unable to fake response on method as it is not derived from HttpMethodBase.");
+ }
+ }
/**
* Process the redirect response.
*
* @return <code>true</code> if the redirect was successful
*/
- private boolean processRedirectResponse() {
-
- if (!this.method.getFollowRedirects()) {
- LOG.info("Redirect requested but followRedirects is "
- + "disabled");
- return false;
- }
+ private boolean processRedirectResponse(final HttpMethod method) {
//get the location header to find out where to redirect to
- Header locationHeader = this.method.getResponseHeader("location");
+ Header locationHeader = method.getResponseHeader("location");
if (locationHeader == null) {
// got a redirect response, but no location header
- LOG.error("Received redirect response " + this.method.getStatusCode()
+ LOG.error("Received redirect response " + method.getStatusCode()
+ " but no location header");
return false;
}
@@ -413,13 +463,12 @@
null,
this.conn.getHost(),
this.conn.getPort(),
- this.method.getPath()
+ method.getPath()
);
redirectUri = new URI(location, true);
if (redirectUri.isRelativeURI()) {
- if (this.method.isStrictMode()) {
- LOG.warn("Redirected location '" + location
- + "' is not acceptable in strict mode");
+ if (this.params.isParameterTrue(HttpClientParams.REJECT_RELATIVE_REDIRECT)) {
+ LOG.warn("Relative redirect location '" + location + "' not allowed");
return false;
} else {
//location is incomplete, use current values for defaults
@@ -435,12 +484,12 @@
//invalidate the list of authentication attempts
this.realms.clear();
//remove exisitng authentication headers
- this.method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
+ 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.
- this.method.setPath(redirectUri.getEscapedPath());
- this.method.setQueryString(redirectUri.getEscapedQuery());
+ method.setPath(redirectUri.getEscapedPath());
+ method.setQueryString(redirectUri.getEscapedQuery());
hostConfiguration.setHost(redirectUri);
if (LOG.isDebugEnabled()) {
@@ -457,21 +506,23 @@
* @param state the current state
* @param conn The connection
*
- * @return <code>true</code> if the request has completed processing, <code>false</code>
- * if more attempts are needed
+ * @return <tt>true</tt> if the authentication challenge can be responsed to,
+ * (that is, at least one of the requested authentication scheme is supported,
+ * and matching credentials have been found), <tt>false</tt> otherwise.
*/
- private boolean processAuthenticationResponse() {
+ private boolean processAuthenticationResponse(final HttpMethod method) {
LOG.trace("enter HttpMethodBase.processAuthenticationResponse("
+ "HttpState, HttpConnection)");
- int statusCode = this.method.getStatusCode();
+ boolean authenticated = false;
+ int statusCode = method.getStatusCode();
// handle authentication required
Header[] challenges = null;
Set realmsUsed = null;
String host = null;
switch (statusCode) {
case HttpStatus.SC_UNAUTHORIZED:
- challenges = this.method.getResponseHeaders(HttpAuthenticator.WWW_AUTH);
+ challenges = method.getResponseHeaders(HttpAuthenticator.WWW_AUTH);
realmsUsed = realms;
host = this.conn.getVirtualHost();
if (host == null) {
@@ -479,12 +530,11 @@
}
break;
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
- challenges = this.method.getResponseHeaders(HttpAuthenticator.PROXY_AUTH);
+ challenges = method.getResponseHeaders(HttpAuthenticator.PROXY_AUTH);
realmsUsed = proxyRealms;
host = this.conn.getProxyHost();
break;
}
- boolean authenticated = false;
// if there was a header requesting authentication
if (challenges.length > 0) {
AuthScheme authscheme = null;
@@ -494,12 +544,12 @@
if (LOG.isErrorEnabled()) {
LOG.error(e.getMessage(), e);
}
- return true;
+ return false;
} catch (UnsupportedOperationException e) {
if (LOG.isErrorEnabled()) {
LOG.error(e.getMessage(), e);
}
- return true;
+ return false;
}
StringBuffer buffer = new StringBuffer();
@@ -516,27 +566,27 @@
buffer.append("' authentication realm at ");
buffer.append(host);
buffer.append(", but still receiving: ");
- buffer.append(this.method.getStatusLine().toString());
+ buffer.append(method.getStatusLine().toString());
LOG.info(buffer.toString());
}
- return true;
+ return false;
} else {
realmsUsed.add(realm);
}
- this.method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
- this.method.removeRequestHeader(HttpAuthenticator.PROXY_AUTH_RESP);
+ method.removeRequestHeader(HttpAuthenticator.WWW_AUTH_RESP);
+ method.removeRequestHeader(HttpAuthenticator.PROXY_AUTH_RESP);
try {
//remove preemptive header and reauthenticate
switch (statusCode) {
case HttpStatus.SC_UNAUTHORIZED:
authenticated = HttpAuthenticator.authenticate(
- authscheme, this.method, this.conn, this.state);
+ authscheme, method, this.conn, this.state);
this.realm = authscheme.getRealm();
break;
case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
authenticated = HttpAuthenticator.authenticateProxy(
- authscheme, this.method, this.conn, this.state);
+ authscheme, method, this.conn, this.state);
this.proxyRealm = authscheme.getRealm();
break;
}
@@ -544,12 +594,12 @@
if (LOG.isWarnEnabled()) {
LOG.warn(e.getMessage());
}
- return true; // finished request
+ return false; // finished request
} catch (AuthenticationException e) {
if (LOG.isErrorEnabled()) {
LOG.error(e.getMessage(), e);
}
- return true; // finished request
+ return false; // finished request
}
if (!authenticated) {
// won't be able to authenticate to this challenge
@@ -564,48 +614,58 @@
}
}
- return !authenticated; // finished processing if we aren't authenticated
+ return authenticated;
}
- /**
- * Returns true if a retry is needed.
+ /**
+ * Tests if the {@link HttpMethod method} requires a redirect to another location.
*
- * @return boolean <code>true</code> if a retry is needed.
- */
- private boolean isRetryNeeded() {
- switch (this.method.getStatusCode()) {
- case HttpStatus.SC_UNAUTHORIZED:
- case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
- LOG.debug("Authorization required");
- 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()) {
- return false;
- }
- } else { //let the client handle the authenticaiton
- return false;
- }
- break;
-
+ * @param method HTTP method
+ *
+ * @return boolean <tt>true</tt> if a retry is needed, <tt>false</tt> otherwise.
+ */
+ private boolean isRedirectNeeded(final HttpMethod method) {
+ switch (method.getStatusCode()) {
case HttpStatus.SC_MOVED_TEMPORARILY:
case HttpStatus.SC_MOVED_PERMANENTLY:
case HttpStatus.SC_SEE_OTHER:
case HttpStatus.SC_TEMPORARY_REDIRECT:
LOG.debug("Redirect required");
-
- if (!processRedirectResponse()) {
- return false;
- }
- break;
-
+ if (method.getFollowRedirects()) {
+ return true;
+ } else {
+ LOG.info("Redirect requested but followRedirects is "
+ + "disabled");
+ return false;
+ }
default:
- // neither an unauthorized nor a redirect response
return false;
} //end of switch
-
- return true;
}
+
+ /**
+ * Tests if the {@link HttpMethod method} requires authentication.
+ *
+ * @param method HTTP method
+ *
+ * @return boolean <tt>true</tt> if a retry is needed, <tt>false</tt> otherwise.
+ */
+ private boolean isAuthenticationNeeded(final HttpMethod method) {
+ switch (method.getStatusCode()) {
+ case HttpStatus.SC_UNAUTHORIZED:
+ case HttpStatus.SC_PROXY_AUTHENTICATION_REQUIRED:
+ LOG.debug("Authorization required");
+ if (method.getDoAuthentication()) { //process authentication response
+ return true;
+ } else { //let the client handle the authenticaiton
+ LOG.info("Redirect requested but doAuthentication is "
+ + "disabled");
+ return false;
+ }
+ default:
+ return false;
+ } //end of switch
+ }
/**
* @return
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org