jsdever 2002/10/30 23:45:35 Modified: httpclient/src/java/org/apache/commons/httpclient HttpConnection.java HttpMethodBase.java httpclient/src/test/org/apache/commons/httpclient SimpleHttpConnection.java TestMethodsNoHost.java TestNoHost.java Added: httpclient/src/test/org/apache/commons/httpclient TestMethodsRedirectNoHost.java Log: Factored out the redirect logic from httpmethodbase into a processRedirectResponse method. Simplified the logic to make it more robust. Contributed by: Jeff Dever Revision Changes Path 1.23 +12 -5 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java Index: HttpConnection.java =================================================================== RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpConnection.java,v retrieving revision 1.22 retrieving revision 1.23 diff -u -r1.22 -r1.23 --- HttpConnection.java 25 Oct 2002 10:15:51 -0000 1.22 +++ HttpConnection.java 31 Oct 2002 07:45:34 -0000 1.23 @@ -453,14 +453,21 @@ * the returned stream will be configured * to read chunked bytes. * - * @param method the http method to get the input stream from + * @param method This argument is ignored. * @throws IllegalStateException if I am not connected * @throws IOException if an I/O problem occurs * @return a stream to read the response from + * @deprecated Use getResponseInputStream() instead. */ public InputStream getResponseInputStream(HttpMethod method) throws IOException, IllegalStateException { log.trace("enter HttpConnection.getResponseInputStream(HttpMethod)"); + return getResponseInputStream(); + } + + public InputStream getResponseInputStream() + throws IOException, IllegalStateException { + log.trace("enter HttpConnection.getResponseInputStream()"); assertOpen(); return _input; } 1.76 +95 -87 jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java Index: HttpMethodBase.java =================================================================== RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v retrieving revision 1.75 retrieving revision 1.76 diff -u -r1.75 -r1.76 --- HttpMethodBase.java 29 Oct 2002 07:51:40 -0000 1.75 +++ HttpMethodBase.java 31 Oct 2002 07:45:34 -0000 1.76 @@ -69,6 +69,7 @@ import java.io.InputStreamReader; import java.io.OutputStream; import java.net.URL; +import java.net.MalformedURLException; import java.util.Date; import java.util.Map; import java.util.HashMap; @@ -836,86 +837,17 @@ return statusCode; } break; + case HttpStatus.SC_MOVED_TEMPORARILY: case HttpStatus.SC_MOVED_PERMANENTLY: case HttpStatus.SC_TEMPORARY_REDIRECT: log.debug("Redirect required"); - //TODO: This block should be factored into a new - //method called processRedirectResponse - if (!getFollowRedirects()) { - log.info("Redirect requested but followRedirects is " - + "disabled"); - return statusCode; - } - //get the location header to find out where to redirect to - Header locationHeader = getResponseHeader("location"); - if (locationHeader == null) { - // got a redirect response, but no location header - log.error("Received redirect response " + statusCode - + " but no location header"); - return statusCode; - } - String location = locationHeader.getValue(); - if (log.isDebugEnabled()) { - log.debug("Redirect requested to location '" + location - + "'"); - } - - - //rfc2616 demands the location value be a complete URI - //Location = "Location" ":" absoluteURI - URL url = null; //the new url - try { - url = new URL(location); - } catch (Exception ex) { - if (isStrictMode()) { - log.error("Redirected location '" + location + - "' is not acceptable in strict mode"); - return statusCode; //should we throw an exception? - } - } - if (url == null) { - //try to construct the new url based on the current url - try { - URL currentUrl = new URL(conn.getProtocol(), - conn.getHost(), - conn.getPort(), getPath()); - url = new URL(currentUrl, location); - } catch (Exception ex) { - log.error("Redirected location '" - + locationHeader.getValue() - + "' is malformed"); - return statusCode; - } - } - - //check for redirect to a different protocol, host or port - try{ - checkValidRedirect(conn, url); - } catch (HttpException ex) { - //log the error and let the client handle the redirect - log.warn(ex.getMessage()); + if (! processRedirectResponse(state, conn)) { return statusCode; } - - // change the path and query string to the redirect - String absolutePath = URIUtil.getPath(url.toString()); - String query = URIUtil.getQuery(url.toString()); - setPath(URIUtil.decode(absolutePath)); - setQueryString(query == null ? null : query); - - if (log.isDebugEnabled()) { - log.debug("Changing path from \"" + getPath() - + "\" to \"" + absolutePath - + "\" in response to " + statusCode - + " response."); - log.debug("Changing query string from \"" - + getQueryString() + "\" to \"" + query - + "\" in response to " + statusCode - + " response."); - } break; + default: // neither an unauthorized nor a redirect response return statusCode; @@ -944,6 +876,75 @@ throw new HttpRecoverableException("Maximum redirects ("+ maxForwards +") exceeded"); } + private boolean processRedirectResponse(HttpState state, HttpConnection conn) { + + if (!getFollowRedirects()) { + log.info("Redirect requested but followRedirects is " + + "disabled"); + return false; + } + + //get the location header to find out where to redirect to + Header locationHeader = getResponseHeader("location"); + if (locationHeader == null) { + // got a redirect response, but no location header + log.error("Received redirect response " + getStatusCode() + + " but no location header"); + return false; + } + String location = locationHeader.getValue(); + if (log.isDebugEnabled()) { + log.debug("Redirect requested to location '" + location + + "'"); + } + + //rfc2616 demands the location value be a complete URI + //Location = "Location" ":" absoluteURI + URL redirectUrl = null; + URL currentUrl = null; + + try { + currentUrl = new URL(conn.getProtocol(), + conn.getHost(), conn.getPort(), ""); + redirectUrl = new URL(location); + } catch (MalformedURLException e) { + if (isStrictMode()) { + log.warn("Redirected location '" + location + + "' is not acceptable in strict mode"); + return false; + } else { //location is incomplete, use current values for defaults + try { + redirectUrl = new URL(currentUrl, location); + } catch (MalformedURLException ex) { + log.warn("Redirected location '" + location + + "' is malformed"); + return false; + } + } + } + + //check for redirect to a different protocol, host or port + try{ + checkValidRedirect(currentUrl, redirectUrl); + } catch (HttpException ex) { + //log the error and let the client handle the redirect + log.warn(ex.getMessage()); + return false; + } + + //update the current location with the redirect location + setPath(redirectUrl.getPath()); + setQueryString(redirectUrl.getQuery()); + + if (log.isDebugEnabled()) { + log.debug("Redirecting from '" + currentUrl.toExternalForm() + + "' to '" + redirectUrl.toExternalForm()); + } + + return true; + + } + /** * Check for a valid redirect given the current conn and new url. @@ -954,29 +955,29 @@ * @throws HttpException if the redirect is invalid * @since 2.0 */ - private static void checkValidRedirect(HttpConnection conn, URL url) + private static void checkValidRedirect(URL currentUrl, URL redirectUrl) throws HttpException { log.trace("enter HttpMethodBase.checkValidRedirect(HttpConnection, URL)"); - String oldProtocol = conn.getProtocol().toLowerCase(); - String newProtocol = url.getProtocol().toLowerCase(); + String oldProtocol = currentUrl.getProtocol().toLowerCase(); + String newProtocol = redirectUrl.getProtocol().toLowerCase(); if (! oldProtocol.equals(newProtocol)) { throw new HttpException("Redirect from protocol " + oldProtocol + " to " + newProtocol + " is not supported"); } - String oldHost = conn.getHost(); - String newHost = url.getHost(); + String oldHost = currentUrl.getHost(); + String newHost = redirectUrl.getHost(); if (! oldHost.equalsIgnoreCase(newHost)) { throw new HttpException("Redirect from host " + oldHost + " to " + newHost + " is not supported"); } - int oldPort = conn.getPort(); + int oldPort = currentUrl.getPort(); if (oldPort < 0) { oldPort = getDefaultPort(oldProtocol); } - int newPort = url.getPort(); + int newPort = redirectUrl.getPort(); if (newPort < 0) { newPort = getDefaultPort(newProtocol); } @@ -1881,11 +1882,18 @@ throws IOException, HttpException { log.trace( "enter HttpMethodBase.writeRequestLine(HttpState, HttpConnection)"); - String requestLine = - HttpMethodBase.generateRequestLine(conn, getName(), - getPath(), getQueryString(), - getHttpVersion()); + String requestLine = getRequestLine(conn); conn.print(requestLine); + } + + /** + * Gets the request line that was sent to the http server. + * Consider making this public. Consider creating a new class + * RequestLine for this purpose. + */ + private String getRequestLine(HttpConnection conn) { + return HttpMethodBase.generateRequestLine(conn, getName(), + getPath(), getQueryString(), getHttpVersion()); } /** 1.4 +29 -4 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/SimpleHttpConnection.java Index: SimpleHttpConnection.java =================================================================== RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/SimpleHttpConnection.java,v retrieving revision 1.3 retrieving revision 1.4 diff -u -r1.3 -r1.4 --- SimpleHttpConnection.java 11 Oct 2002 12:59:07 -0000 1.3 +++ SimpleHttpConnection.java 31 Oct 2002 07:45:35 -0000 1.4 @@ -68,8 +68,10 @@ import java.io.BufferedReader; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.io.StringReader; import java.util.Vector; @@ -89,6 +91,7 @@ Vector bodies = new Vector(); BufferedReader headerReader = null; ByteArrayInputStream bodyInputStream = null; + ByteArrayOutputStream bodyOutputStream = null; public void addResponse(String header) { addResponse(header, ""); @@ -113,6 +116,18 @@ super(host, port, isSecure); } + public void assertOpen() throws IllegalStateException { + if (bodyInputStream == null) { + throw new IllegalStateException(); + } + } + + public void assertNotOpen() throws IllegalStateException{ + if (bodyInputStream != null) { + throw new IllegalStateException(); + } + } + public void open() throws IOException { if (headerReader != null) return; @@ -122,6 +137,7 @@ new StringReader((String)headers.elementAt(hits))); bodyInputStream = new ByteArrayInputStream( ((String)bodies.elementAt(hits)).getBytes()); + bodyOutputStream = new ByteArrayOutputStream(); hits++; } catch (ArrayIndexOutOfBoundsException aiofbe) { throw new IOException("SimpleHttpConnection has been opened more times " + @@ -138,6 +154,10 @@ try { bodyInputStream.close(); } catch(IOException e) {} bodyInputStream = null; } + if (bodyOutputStream != null) { + try { bodyOutputStream.close(); } catch(IOException e) {} + bodyOutputStream = null; + } } public void write(byte[] data) @@ -155,8 +175,13 @@ return str; } - public InputStream getResponseInputStream(HttpMethod method) { + + public InputStream getResponseInputStream() { return bodyInputStream; + } + + public OutputStream getRequestOutputStream() { + return bodyOutputStream; } } 1.11 +26 -5 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestMethodsNoHost.java Index: TestMethodsNoHost.java =================================================================== RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestMethodsNoHost.java,v retrieving revision 1.10 retrieving revision 1.11 diff -u -r1.10 -r1.11 --- TestMethodsNoHost.java 21 Oct 2002 14:03:07 -0000 1.10 +++ TestMethodsNoHost.java 31 Oct 2002 07:45:35 -0000 1.11 @@ -303,6 +303,27 @@ assertTrue(!conn.isOpen()); } + public void testSetGetQueryString1() { + HttpMethod method = new GetMethod(); + String qs1 = "name1=value1&name2=value2"; + method.setQueryString(qs1); + assertEquals(qs1, method.getQueryString()); + } + public void testQueryURIEncoding() { + HttpMethod method = new GetMethod("http://server/servlet?foo=bar&baz=schmoo"); + assertEquals("foo=bar&baz=schmoo", method.getQueryString()); + } + + public void testSetGetQueryString2() { + HttpMethod method = new GetMethod(); + NameValuePair[] q1 = new NameValuePair[] { + new NameValuePair("name1", "value1"), + new NameValuePair("name2", "value2") + }; + method.setQueryString(q1); + String qs1 = "name1=value1&name2=value2"; + assertEquals(qs1, method.getQueryString()); + } } 1.15 +5 -4 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestNoHost.java Index: TestNoHost.java =================================================================== RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestNoHost.java,v retrieving revision 1.14 retrieving revision 1.15 diff -u -r1.14 -r1.15 --- TestNoHost.java 29 Oct 2002 06:40:15 -0000 1.14 +++ TestNoHost.java 31 Oct 2002 07:45:35 -0000 1.15 @@ -95,6 +95,7 @@ suite.addTest(TestURIUtil.suite()); suite.addTest(TestURIUtil2.suite()); suite.addTest(TestMethodsNoHost.suite()); + suite.addTest(TestMethodsRedirectNoHost.suite()); suite.addTest(TestHttpState.suite()); suite.addTest(TestResponseHeaders.suite()); suite.addTest(TestRequestHeaders.suite()); 1.1 jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestMethodsRedirectNoHost.java Index: TestMethodsRedirectNoHost.java =================================================================== /* * $Header: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestMethodsRedirectNoHost.java,v 1.1 2002/10/31 07:45:35 jsdever Exp $ * $Revision: 1.1 $ * $Date: 2002/10/31 07:45:35 $ * ==================================================================== * * The Apache Software License, Version 1.1 * * Copyright (c) 2002 The Apache Software Foundation. All rights * reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: * * 1. Redistributions of source code must retain the above copyright * notice, this list of conditions and the following disclaimer. * * 2. Redistributions in binary form must reproduce the above copyright * notice, this list of conditions and the following disclaimer in * the documentation and/or other materials provided with the * distribution. * * 3. The end-user documentation included with the redistribution, if * any, must include the following acknowlegement: * "This product includes software developed by the * Apache Software Foundation (http://www.apache.org/)." * Alternately, this acknowlegement may appear in the software itself, * if and wherever such third-party acknowlegements normally appear. * * 4. The names "The Jakarta Project", "Tomcat", and "Apache Software * Foundation" must not be used to endorse or promote products derived * from this software without prior written permission. For written * permission, please contact [EMAIL PROTECTED] * * 5. Products derived from this software may not be called "Apache" * nor may "Apache" appear in their names without prior written * permission of the Apache Group. * * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE * DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF * SUCH DAMAGE. * ==================================================================== * * 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 * <http://www.apache.org/>. * * [Additional notices, if required by prior licensing conditions] * */ package org.apache.commons.httpclient; import junit.framework.Test; import junit.framework.TestCase; import junit.framework.TestSuite; import org.apache.commons.httpclient.methods.*; /** * @author <a href="mailto:jsdever@;apache.org">Jeff Dever</a> * @version $Revision: 1.1 $ */ public class TestMethodsRedirectNoHost extends TestCase { SimpleHttpConnection conn; // ------------------------------------------------------------ Constructor public TestMethodsRedirectNoHost(String testName) { super(testName); } // ------------------------------------------------------- TestCase Methods public static Test suite() { return new TestSuite(TestMethodsRedirectNoHost.class); } public void setUp() throws Exception{ conn = new SimpleHttpConnection(); } private void addRedirectResponse(String location) { String headers = "HTTP/1.1 302 Redirect\r\n" +"Date: Wed, 28 Mar 2002 05:05:04 GMT\r\n" +"Location: " + location + "\r\n" +"Connection: close\r\n"; conn.addResponse(headers, ""); } private void addOkResponse() { String headers = "HTTP/1.1 200 OK\r\n" +"Date: Wed, 28 Mar 2001 05:05:04 GMT\r\n" +"Connection: close\r\n"; conn.addResponse(headers, ""); } // ----------------------------------------------------------------- Tests public void testRedirect() throws Exception { addRedirectResponse("http://localhost/newfile"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(true); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(200, method.getStatusCode()); assertEquals("/newfile", method.getPath()); } public void testPostRedirect() throws Exception { addRedirectResponse("http://localhost/newfile"); addOkResponse(); conn.open(); PostMethod method = new PostMethod("/oldfile"); method.setFollowRedirects(true); method.addParameter("name", "value"); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(200, method.getStatusCode()); assertEquals(1, method.getParameters().length); assertEquals("/newfile", method.getPath()); } public void testNoRedirect() throws Exception { addRedirectResponse("http://localhost/newfile"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(false); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(302, method.getStatusCode()); assertEquals("/oldfile", method.getPath()); } public void testRedirectBadLocation() throws Exception { addRedirectResponse("newfile"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(true); method.setStrictMode(false); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(200, method.getStatusCode()); assertEquals("/newfile", method.getPath()); } public void testRedirectBadLocationStrict() throws Exception { addRedirectResponse("newfile"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(true); method.setStrictMode(true); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(302, method.getStatusCode()); assertEquals("/oldfile", method.getPath()); } public void testRedirectBogusLocationStrict() throws Exception { addRedirectResponse("xxx://bogus"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(true); method.setStrictMode(true); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(302, method.getStatusCode()); assertEquals("/oldfile", method.getPath()); } public void testRedirectDifferentHost() throws Exception { conn = new SimpleHttpConnection("oldhost", 80, false); addRedirectResponse("http://newhost/newfile"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(true); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(302, method.getStatusCode()); assertEquals("/oldfile", method.getPath()); } public void testRedirectDifferentPort() throws Exception { conn = new SimpleHttpConnection("oldhost", 80, false); addRedirectResponse("http://oldhost:8080/newfile"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(true); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(302, method.getStatusCode()); assertEquals("/oldfile", method.getPath()); } public void testRedirectDifferentProtocol() throws Exception { conn = new SimpleHttpConnection("oldhost", 80, false); addRedirectResponse("https://oldhost:80/newfile"); addOkResponse(); conn.open(); HttpMethod method = new SimpleHttpMethod("/oldfile"); method.setFollowRedirects(true); method.execute(new HttpState(), conn); Header locationHeader = method.getResponseHeader("Location"); assertEquals(302, method.getStatusCode()); assertEquals("/oldfile", method.getPath()); } }
-- To unsubscribe, e-mail: <mailto:commons-dev-unsubscribe@;jakarta.apache.org> For additional commands, e-mail: <mailto:commons-dev-help@;jakarta.apache.org>