svn commit: r981260 - in /wicket/trunk: wicket-util/src/test/java/org/apache/wicket/util/file/ wicket/src/main/java/org/apache/wicket/protocol/http/ wicket/src/test/java/org/apache/wicket/protocol/http/

[jd...@apache.org]

Author: jdonnerstag
Date: Sun Aug  1 16:22:26 2010
New Revision: 981260

URL: http://svn.apache.org/viewvc?rev=981260&view=rev
Log:
fixed WicketFilter (the bug I introduced today), made it easier testable and added a test

Added:
    wicket/trunk/wicket-util/src/test/java/org/apache/wicket/util/file/
    wicket/trunk/wicket-util/src/test/java/org/apache/wicket/util/file/WebXmlFileTest.java
Modified:
    wicket/trunk/wicket/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java
    wicket/trunk/wicket/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java

Added: wicket/trunk/wicket-util/src/test/java/org/apache/wicket/util/file/WebXmlFileTest.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket-util/src/test/java/org/apache/wicket/util/file/WebXmlFileTest.java?rev=981260&view=auto
==============================================================================
--- wicket/trunk/wicket-util/src/test/java/org/apache/wicket/util/file/WebXmlFileTest.java (added)
+++ wicket/trunk/wicket-util/src/test/java/org/apache/wicket/util/file/WebXmlFileTest.java Sun Aug  1 16:22:26 2010
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+package org.apache.wicket.util.file;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+
+import javax.xml.parsers.ParserConfigurationException;
+
+import junit.framework.TestCase;
+
+import org.xml.sax.SAXException;
+
+
+/**
+ * 
+ */
+public class WebXmlFileTest extends TestCase
+{
+	public void test_1() throws ParserConfigurationException, SAXException, IOException
+	{
+		StringBuffer webxml = new StringBuffer();
+		webxml.append("<web-app>");
+		webxml.append("<filter>");
+		webxml.append(" <filter-name>HelloWorldApplication</filter-name>");
+		webxml.append(" <filter-class>org.apache.wicket.protocol.http.WicketFilter</filter-class>");
+		webxml.append(" <init-param>");
+		webxml.append("  <param-name>applicationClassName</param-name>");
+		webxml.append("  <param-value>org.apache.wicket.examples.helloworld.HelloWorldApplication</param-value>");
+		webxml.append(" </init-param>");
+		webxml.append("</filter>");
+		webxml.append("");
+		webxml.append("<filter-mapping>");
+		webxml.append(" <filter-name>HelloWorldApplication</filter-name>");
+		webxml.append(" <url-pattern>/*</url-pattern>");
+		webxml.append(" <dispatcher>REQUEST</dispatcher>");
+		webxml.append(" <dispatcher>INCLUDE</dispatcher>");
+		webxml.append("</filter-mapping>");
+		webxml.append("</web-app>");
+
+		String path = new WebXmlFile().getFilterPath("HelloWorldApplication",
+			new ByteArrayInputStream(webxml.toString().getBytes()));
+		assertEquals("", path);
+	}
+}

Modified: wicket/trunk/wicket/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java?rev=981260&r1=981259&r2=981260&view=diff
==============================================================================
--- wicket/trunk/wicket/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java (original)
+++ wicket/trunk/wicket/src/main/java/org/apache/wicket/protocol/http/WicketFilter.java Sun Aug  1 16:22:26 2010
@@ -146,6 +146,11 @@ public class WicketFilter implements Fil
 			}
 			else
 			{
+				if (Strings.isEmpty(httpServletRequest.getQueryString()) == false)
+				{
+					redirectURL += "?" + httpServletRequest.getQueryString();
+				}
+
 				try
 				{
 					// send redirect - this will discard POST parameters if the request is POST
@@ -350,9 +355,18 @@ public class WicketFilter implements Fil
 	 */
 	private String checkIfRedirectRequired(final HttpServletRequest request)
 	{
-		String requestURI = request.getRequestURI();
-		String contextPath = request.getContextPath();
+		return checkIfRedirectRequired(request.getRequestURI(), request.getContextPath());
+	}
 
+	/**
+	 * Try to determine as fast as possible if a redirect is necessary
+	 * 
+	 * @param requestURI
+	 * @param contextPath
+	 * @return null, if no redirect is necessary. Else the redirect URL
+	 */
+	protected final String checkIfRedirectRequired(final String requestURI, final String contextPath)
+	{
 		// length without jesessionid (http://.../abc;jsessionid=...?param)
 		int uriLength = requestURI.indexOf(';');
 		if (uriLength == -1)
@@ -360,6 +374,7 @@ public class WicketFilter implements Fil
 			uriLength = requestURI.length();
 		}
 
+		// We only need to determine it once. It'll not change.
 		if (filterPathLength == -1)
 		{
 			filterPathLength = filterPath.length();
@@ -369,29 +384,34 @@ public class WicketFilter implements Fil
 			}
 		}
 
-		// uri != request.getContextPath() + "/" + filterPath (without "/" at the end)
-		if (uriLength != (contextPath.length() + 1 + filterPathLength))
+		// request.getContextPath() + "/" + filterPath. But without any trailing "/".
+		int homePathLenth = contextPath.length() + (filterPathLength > 0 ? 1 : 0) +
+			filterPathLength;
+		if (uriLength != homePathLenth)
 		{
+			// requestURI and homePath are different (in length)
+			// => continue with standard request processing. No redirect.
 			return null;
 		}
 
-		// current URI without jsessionid
-		String uri = requestURI.substring(0, uriLength);
+		// Fail fast failed. Revert to "slow" but exact check
+		String uri = Strings.stripJSessionId(requestURI);
 
 		// home page without trailing slash URI
-		String homePageUri = contextPath + "/" + filterPath.substring(0, filterPathLength);
-		if (uri.equals(homePageUri) == false)
+		String homePageUri = contextPath + "/" + filterPath;
+		if (homePageUri.endsWith("/"))
 		{
-			return null;
+			homePageUri = homePageUri.substring(0, homePageUri.length() - 1);
 		}
 
-		// create the redirect URI
-		uri += "/";
-		if (!Strings.isEmpty(request.getQueryString()))
+		// If both are equal => redirect
+		if (uri.equals(homePageUri))
 		{
-			uri += "?" + request.getQueryString();
+			uri += "/";
+			return uri;
 		}
 
-		return uri;
+		// no match => standard request processing; no redirect
+		return null;
 	}
 }

Modified: wicket/trunk/wicket/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java
URL: http://svn.apache.org/viewvc/wicket/trunk/wicket/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java?rev=981260&r1=981259&r2=981260&view=diff
==============================================================================
--- wicket/trunk/wicket/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java (original)
+++ wicket/trunk/wicket/src/test/java/org/apache/wicket/protocol/http/WicketFilterTest.java Sun Aug  1 16:22:26 2010
@@ -204,4 +204,12 @@ public class WicketFilterTest extends Te
 		{
 		}
 	}
+
+	public void testCheckRedirect_1()
+	{
+		WicketFilter filter = new WicketFilter();
+
+		// Simulate url-pattern = "/*" and request = http://localhost:8080
+		assertEquals("", filter.checkIfRedirectRequired("/", ""));
+	}
 }