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.19 diff -u -r1.19 HttpMethodDirector.java --- java/org/apache/commons/httpclient/HttpMethodDirector.java 25 Mar 2004 20:37:19 -0000 1.19 +++ java/org/apache/commons/httpclient/HttpMethodDirector.java 10 Apr 2004 15:31:45 -0000 @@ -32,7 +32,9 @@ package org.apache.commons.httpclient; import java.io.IOException; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import org.apache.commons.httpclient.auth.AuthChallengeException; import org.apache.commons.httpclient.auth.AuthChallengeParser; @@ -66,9 +68,6 @@ /** The proxy authenticate response header. */ public static final String PROXY_AUTH_RESP = "Proxy-Authorization"; - /** Maximum number of redirects and authentications that will be followed */ - private static final int MAX_FORWARDS = 100; - private static final Log LOG = LogFactory.getLog(HttpMethodDirector.class); private ConnectMethod connectMethod; @@ -98,6 +97,8 @@ /** Authentication processor */ private AuthChallengeProcessor authProcessor = null; + private Set redirectLocations = null; + public HttpMethodDirector( final HttpConnectionManager connectionManager, final HostConfiguration hostConfiguration, @@ -125,13 +126,9 @@ } method.getParams().setDefaults(this.params); try { - int forwardCount = 0; //protect from an infinite loop + int maxRedirects = this.params.getIntParameter(HttpClientParams.MAX_REDIRECTS, 100); - while (forwardCount++ < MAX_FORWARDS) { - // on every retry, reset this state information. - if (LOG.isDebugEnabled()) { - LOG.debug("Execute loop try " + forwardCount); - } + for (int redirectCount = 0;;) { // make sure the connection we have is appropriate if (this.conn != null && !hostConfiguration.hostEquals(this.conn)) { @@ -169,6 +166,15 @@ if (isRedirectNeeded(method)) { if (processRedirectResponse(method)) { retry = true; + ++redirectCount; + if (redirectCount >= maxRedirects) { + LOG.error("Narrowly avoided an infinite loop in execute"); + throw new RedirectException("Maximum redirects (" + + maxRedirects + ") exceeded"); + } + if (LOG.isDebugEnabled()) { + LOG.debug("Execute redirect " + redirectCount + " of " + maxRedirects); + } } } if (isAuthenticationNeeded(method)) { @@ -189,13 +195,6 @@ } } //end of retry loop - - if (forwardCount >= MAX_FORWARDS) { - LOG.error("Narrowly avoided an infinite loop in execute"); - throw new ProtocolException("Maximum redirects (" - + MAX_FORWARDS + ") exceeded"); - } - } finally { if (this.conn != null) { this.conn.setLocked(false); @@ -279,7 +278,6 @@ private void authenticateProxy(final HttpMethod method) throws AuthenticationException { // Clean up existing authentication headers - // Clean up existing authentication headers if (!cleanAuthHeaders(method, PROXY_AUTH_RESP)) { // User defined authentication header(s) present return; @@ -494,8 +492,9 @@ * * @return true if the redirect was successful */ - private boolean processRedirectResponse(final HttpMethod method) { - + private boolean processRedirectResponse(final HttpMethod method) + throws RedirectException + { //get the location header to find out where to redirect to Header locationHeader = method.getResponseHeader("location"); if (locationHeader == null) { @@ -506,10 +505,9 @@ } String location = locationHeader.getValue(); if (LOG.isDebugEnabled()) { - LOG.debug("Redirect requested to location '" + location - + "'"); + LOG.debug("Redirect requested to location '" + location + "'"); } - + //rfc2616 demands the location value be a complete URI //Location = "Location" ":" absoluteURI URI redirectUri = null; @@ -538,9 +536,17 @@ LOG.warn("Redirected location '" + location + "' is malformed"); return false; } - //update the current location with the redirect location. - //avoiding use of URL.getPath() and URL.getQuery() to keep - //jdk1.2 comliance. + + if (this.params.isParameterFalse(HttpClientParams.CIRCULAR_REDIRECTS)) { + if (this.redirectLocations == null) { + this.redirectLocations = new HashSet(); + } + this.redirectLocations.add(currentUri); + if (this.redirectLocations.contains(redirectUri)) { + throw new RedirectException("Circular redirect to '" + + redirectUri + "'"); + } + } method.setPath(redirectUri.getEscapedPath()); method.setQueryString(redirectUri.getEscapedQuery()); hostConfiguration.setHost(redirectUri); Index: java/org/apache/commons/httpclient/RedirectException.java =================================================================== RCS file: java/org/apache/commons/httpclient/RedirectException.java diff -N java/org/apache/commons/httpclient/RedirectException.java --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ java/org/apache/commons/httpclient/RedirectException.java 10 Apr 2004 15:31:46 -0000 @@ -0,0 +1,66 @@ +/* + * $Header$ + * $Revision$ + * $Date$ + * + * ==================================================================== + * + * Copyright 1999-2004 The Apache Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ +package org.apache.commons.httpclient; + +/** + * Signals violation of HTTP specification caused by an invalid redirect + * + * @author Oleg Kalnichevski + * + * @since 3.0 + */ +public class RedirectException extends ProtocolException { + + /** + * Creates a new RedirectException with a null detail message. + */ + public RedirectException() { + super(); + } + + /** + * Creates a new RedirectException with the specified detail message. + * + * @param message The exception detail message + */ + public RedirectException(String message) { + super(message); + } + + /** + * Creates a new RedirectException with the specified detail message and cause. + * + * @param message the exception detail message + * @param cause the Throwable that caused this exception, or null + * if the cause is unavailable, unknown, or not a Throwable + */ + public RedirectException(String message, Throwable cause) { + super(message, cause); + } +} Index: java/org/apache/commons/httpclient/params/HttpClientParams.java =================================================================== RCS file: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/params/HttpClientParams.java,v retrieving revision 1.4 diff -u -r1.4 HttpClientParams.java --- java/org/apache/commons/httpclient/params/HttpClientParams.java 22 Feb 2004 18:08:48 -0000 1.4 +++ java/org/apache/commons/httpclient/params/HttpClientParams.java 10 Apr 2004 15:31:48 -0000 @@ -1,5 +1,5 @@ /* - * $Header: /home/cvspublic/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/params/HttpClientParams.java,v 1.4 2004/02/22 18:08:48 olegk Exp $ + * $Header: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/params/HttpClientParams.java,v 1.4 2004/02/22 18:08:48 olegk Exp $ * $Revision: 1.4 $ * $Date: 2004/02/22 18:08:48 $ * @@ -54,7 +54,9 @@ * Sets the timeout in milliseconds used when retrieving an * {@link org.apache.commons.httpclient.HttpConnection HTTP connection} from the * {@link org.apache.commons.httpclient.HttpConnectionManager HTTP connection manager}. + *

* This parameter expects a value of type {@link Long}. + *

*/ public static final String CONNECTION_MANAGER_TIMEOUT = "http.connection-manager.timeout"; @@ -62,22 +64,47 @@ * Defines the default * {@link org.apache.commons.httpclient.HttpConnectionManager HTTP connection manager} * class. + *

* This parameter expects a value of type {@link Class}. + *

*/ public static final String CONNECTION_MANAGER_CLASS = "http.connection-manager.class"; /** * Defines whether authentication should be attempted preemptively. + *

* This parameter expects a value of type {@link Boolean}. + *

*/ public static final String PREEMPTIVE_AUTHENTICATION = "http.authentication.preemptive"; /** * Defines whether relative redirects should be rejected. + *

* This parameter expects a value of type {@link Boolean}. + *

*/ public static final String REJECT_RELATIVE_REDIRECT = "http.protocol.reject-relative-redirect"; + /** + * Defines the maximum number of redirects to be followed. + * The limit on number of redirects is intended to prevent infinite loops. + *

+ * This parameter expects a value of type {@link Integer}. + *

+ */ + public static final String MAX_REDIRECTS = "http.protocol.max-redirects"; + + /** + * Defines whether circular redirects (redirects to the same location) should be allowed. + * The HTTP spec is not sufficiently clear whether circular redirects are permitted, + * therefore optionally they can be enabled + *

+ * This parameter expects a value of type {@link Boolean}. + *

+ */ + public static final String CIRCULAR_REDIRECTS = "http.protocol.circular-redirects"; + /** * Creates a new collection of parameters with the collection returned * by {@link #getDefaultParams()} as a parent. The collection will defer @@ -171,7 +198,8 @@ } private static final String[] PROTOCOL_STRICTNESS_PARAMETERS = { - REJECT_RELATIVE_REDIRECT + REJECT_RELATIVE_REDIRECT, + CIRCULAR_REDIRECTS }; Index: test/org/apache/commons/httpclient/TestNoHost.java =================================================================== RCS file: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestNoHost.java,v retrieving revision 1.33 diff -u -r1.33 TestNoHost.java --- test/org/apache/commons/httpclient/TestNoHost.java 25 Mar 2004 20:37:20 -0000 1.33 +++ test/org/apache/commons/httpclient/TestNoHost.java 10 Apr 2004 15:31:49 -0000 @@ -1,5 +1,5 @@ /* - * $Header: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestNoHost.java,v 1.33 2004/03/25 20:37:20 olegk Exp $ + * $Header: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestNoHost.java,v 1.33 2004/03/25 20:37:20 olegk Exp $ * $Revision: 1.33 $ * $Date: 2004/03/25 20:37:20 $ * ==================================================================== @@ -66,6 +66,7 @@ suite.addTest(TestChallengeProcessor.suite()); suite.addTest(TestAuthenticator.suite()); suite.addTest(TestBasicAuth.suite()); + suite.addTest(TestRedirects.suite()); suite.addTest(TestHttpUrlMethod.suite()); suite.addTest(TestURI.suite()); suite.addTest(TestURIUtil.suite()); Index: test/org/apache/commons/httpclient/TestRedirects.java =================================================================== RCS file: test/org/apache/commons/httpclient/TestRedirects.java diff -N test/org/apache/commons/httpclient/TestRedirects.java --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ test/org/apache/commons/httpclient/TestRedirects.java 10 Apr 2004 15:31:49 -0000 @@ -0,0 +1,143 @@ +/* + * $Header$ + * $Revision$ + * $Date$ + * ==================================================================== + * + * Copyright 1999-2004 The Apache Software Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + * [Additional notices, if required by prior licensing conditions] + * + */ + +package org.apache.commons.httpclient; + +import java.io.IOException; + +import junit.framework.Test; +import junit.framework.TestSuite; + +import org.apache.commons.httpclient.methods.GetMethod; +import org.apache.commons.httpclient.params.HttpClientParams; +import org.apache.commons.httpclient.server.HttpService; +import org.apache.commons.httpclient.server.RequestLine; +import org.apache.commons.httpclient.server.SimpleRequest; +import org.apache.commons.httpclient.server.SimpleResponse; + +/** + * Basic authentication test cases. + * + * @author Oleg Kalnichevski + * + * @version $Id: TestBasicAuth.java,v 1.2 2004/03/25 20:37:20 olegk Exp $ + */ +public class TestRedirects extends HttpClientTestBase { + + // ------------------------------------------------------------ Constructor + public TestRedirects(String testName) { + super(testName); + } + + // ------------------------------------------------------------------- Main + public static void main(String args[]) { + String[] testCaseName = { TestRedirects.class.getName() }; + junit.textui.TestRunner.main(testCaseName); + } + + // ------------------------------------------------------- TestCase Methods + + public static Test suite() { + return new TestSuite(TestRedirects.class); + } + + private class RedirectService implements HttpService { + + public RedirectService() { + super(); + } + + public boolean process(final SimpleRequest request, final SimpleResponse response) + throws IOException + { + RequestLine reqline = request.getRequestLine(); + if (reqline.getUri().equals("/circular-location1/")) { + response.setStatusLine("HTTP/1.1 302 Object moved"); + response.addHeader(new Header("Location", "/circular-location2/")); + } else if (reqline.getUri().equals("/circular-location2/")) { + response.setStatusLine("HTTP/1.1 302 Object moved"); + response.addHeader(new Header("Location", "/circular-location1/")); + } else if (reqline.getUri().equals("/location1/")) { + response.setStatusLine("HTTP/1.1 302 Object moved"); + response.addHeader(new Header("Location", "/location2/")); + } else if (reqline.getUri().equals("/location2/")) { + response.setStatusLine("HTTP/1.1 200 OK"); + response.setBodyString("Successful redirect"); + } else { + response.setStatusLine("HTTP/1.1 404 Not Found"); + } + return true; + } + } + + public void testMaxRedirectCheck() throws IOException { + this.server.setHttpService(new RedirectService()); + GetMethod httpget = new GetMethod("/circular-location1/"); + try { + this.client.getParams().setBooleanParameter(HttpClientParams.CIRCULAR_REDIRECTS, true); + this.client.getParams().setIntParameter(HttpClientParams.MAX_REDIRECTS, 5); + this.client.executeMethod(httpget); + fail("RedirectException exception should have been thrown"); + } + catch (RedirectException e) { + // expected + } finally { + httpget.releaseConnection(); + } + } + + public void testCircularRedirect() throws IOException { + this.server.setHttpService(new RedirectService()); + GetMethod httpget = new GetMethod("/circular-location1/"); + try { + this.client.getParams().setBooleanParameter(HttpClientParams.CIRCULAR_REDIRECTS, false); + this.client.executeMethod(httpget); + fail("RedirectException exception should have been thrown"); + } + catch (RedirectException e) { + // expected + } finally { + httpget.releaseConnection(); + } + } + + public void testRedirectLocation() throws IOException { + this.server.setHttpService(new RedirectService()); + GetMethod httpget = new GetMethod("/location1/"); + try { + this.client.executeMethod(httpget); + assertEquals("/location2/", httpget.getPath()); + assertEquals(new URI("/location2/", false), httpget.getURI()); + System.out.println(); + } finally { + httpget.releaseConnection(); + } + } +} Index: test/org/apache/commons/httpclient/TestWebappRedirect.java =================================================================== RCS file: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestWebappRedirect.java,v retrieving revision 1.21 diff -u -r1.21 TestWebappRedirect.java --- test/org/apache/commons/httpclient/TestWebappRedirect.java 22 Feb 2004 18:08:50 -0000 1.21 +++ test/org/apache/commons/httpclient/TestWebappRedirect.java 10 Apr 2004 15:31:51 -0000 @@ -1,5 +1,5 @@ /* - * $Header: /home/cvspublic/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestWebappRedirect.java,v 1.21 2004/02/22 18:08:50 olegk Exp $ + * $Header: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestWebappRedirect.java,v 1.21 2004/02/22 18:08:50 olegk Exp $ * $Revision: 1.21 $ * $Date: 2004/02/22 18:08:50 $ * @@ -179,7 +179,7 @@ try { client.executeMethod(method); fail("Expected HTTPException"); - } catch (HttpException t) { + } catch (ProtocolException t) { // expected } assertEquals(302,method.getStatusCode());