commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient HttpMethod.java HttpMethodDirector.java
Date Wed, 19 Nov 2003 21:11:16 GMT
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


Mime
View raw message