Bug 536708 - Multipart request responded with 302 under JettyY20180905-2200

Rework fix to use split with a max to handle trailing SLASH

Change-Id: Ia303f97db59bbd1de6a98bcbe7b1385788dd8c14
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>

6 files changed, 117 insertions, 12 deletions

diff --git a/bundles/org.eclipse.equinox.http.servlet.tests/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.http.servlet.tests/META-INF/MANIFEST.MF
index 7aa1f42a0..34a0ad3c9 100644
--- a/bundles/org.eclipse.equinox.http.servlet.tests/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.equinox.http.servlet.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: org.eclipse.equinox.http.servlet.tests
 Bundle-SymbolicName: org.eclipse.equinox.http.servlet.tests
-Bundle-Version: 1.5.100.qualifier
+Bundle-Version: 1.5.200.qualifier
 Bundle-RequiredExecutionEnvironment: JavaSE-1.7
 Eclipse-BundleShape: dir
 Bundle-Activator: org.eclipse.equinox.http.servlet.tests.bundle.Activator
diff --git a/bundles/org.eclipse.equinox.http.servlet.tests/pom.xml b/bundles/org.eclipse.equinox.http.servlet.tests/pom.xml
index 00fe5c416..1ed46a426 100644
--- a/bundles/org.eclipse.equinox.http.servlet.tests/pom.xml
+++ b/bundles/org.eclipse.equinox.http.servlet.tests/pom.xml
@@ -19,7 +19,7 @@
   </parent>
   <groupId>org.eclipse.equinox</groupId>
   <artifactId>org.eclipse.equinox.http.servlet.tests</artifactId>
-  <version>1.5.100-SNAPSHOT</version>
+  <version>1.5.200-SNAPSHOT</version>
   <packaging>eclipse-test-plugin</packaging>
 
   <build>
diff --git a/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/ServletTest.java b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/ServletTest.java
index fe0f8cc8e..9c9f6659b 100644
--- a/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/ServletTest.java
+++ b/bundles/org.eclipse.equinox.http.servlet.tests/src/org/eclipse/equinox/http/servlet/tests/ServletTest.java
@@ -3338,12 +3338,15 @@ public class ServletTest extends BaseTest {
 			props = new Hashtable<String, Object>();
 
 			requestAdvisor.request("a/b/c/");
+
 			Assert.assertEquals("/foo/a/b/c/", getRequestURI.get());
 			requestAdvisor.request("a/b/");
 			Assert.assertEquals("/foo/a/b/", getRequestURI.get());
 			requestAdvisor.request("a/");
 			Assert.assertEquals("/foo/a/", getRequestURI.get());
-			requestAdvisor.request("/");
+			// Note using empty string here because of the way requestAdvisor works
+			// by appending a slash first.
+			requestAdvisor.request("");
 			Assert.assertEquals("/foo/", getRequestURI.get());
 		}
 		finally {
@@ -3362,6 +3365,67 @@ public class ServletTest extends BaseTest {
 
 	@Test
 	public void test_getRequestURI_trailingSlash2() throws Exception {
+		try {
+			stopJetty();
+			System.setProperty("org.eclipse.equinox.http.jetty.context.path", "/foo");
+		}
+		finally {
+			startJetty();
+		}
+
+		Collection<ServiceRegistration<?>> registrations = new ArrayList<ServiceRegistration<?>>();
+		try {
+			final AtomicReference<String> getRequestURI = new AtomicReference<String>();
+
+			Servlet servletA = new HttpServlet() {
+				private static final long serialVersionUID = 1L;
+				@Override
+				protected void service(HttpServletRequest req, HttpServletResponse resp)
+					throws IOException, ServletException {
+					HttpSession session = req.getSession();
+					session.setAttribute("test", req.getParameter("p1"));
+					getRequestURI.set(resp.encodeURL(req.getRequestURI()));
+				}
+			};
+
+			Dictionary<String, Object> props = new Hashtable<String, Object>();
+			props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_NAME, "SA");
+			props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, "/*");
+			registrations.add(getBundleContext().registerService(Servlet.class, servletA, props));
+			props = new Hashtable<String, Object>();
+
+			requestAdvisor.request("a/b/c/?p1=v1");
+			// get the session
+			String initialURI = getRequestURI.get();
+			int sessionIdx = initialURI.indexOf(";jsessionid=");
+			Assert.assertTrue("No session: " + initialURI, sessionIdx > -1);
+			String sessionPostfix = initialURI.substring(sessionIdx);
+			Assert.assertEquals("/foo/a/b/c/" + sessionPostfix, getRequestURI.get());
+			requestAdvisor.request("a/b/" + sessionPostfix);
+			Assert.assertEquals("/foo/a/b/" + sessionPostfix, getRequestURI.get());
+			requestAdvisor.request("a/" + sessionPostfix);
+			Assert.assertEquals("/foo/a/" + sessionPostfix, getRequestURI.get());
+			// Note using empty string here because of the way requestAdvisor works
+			// by appending a slash first.
+			requestAdvisor.request("" + sessionPostfix);
+			Assert.assertEquals("/foo/" + sessionPostfix, getRequestURI.get());
+		}
+		finally {
+			for (ServiceRegistration<?> registration : registrations) {
+				registration.unregister();
+			}
+			try {
+				stopJetty();
+				System.setProperty("org.eclipse.equinox.http.jetty.context.path", "");
+			}
+			finally {
+				startJetty();
+			}
+		}
+	}
+
+	@Test
+	public void test_getRequestURI_trailingSlash3() throws Exception {
 		Collection<ServiceRegistration<?>> registrations = new ArrayList<ServiceRegistration<?>>();
 		try {
 			final AtomicReference<String> getRequestURI = new AtomicReference<String>();
@@ -3399,6 +3463,51 @@ public class ServletTest extends BaseTest {
 		}
 	}
 
+	@Test
+	public void test_getRequestURI_trailingSlash4() throws Exception {
+		Collection<ServiceRegistration<?>> registrations = new ArrayList<ServiceRegistration<?>>();
+		try {
+			final AtomicReference<String> getRequestURI = new AtomicReference<String>();
+
+			Servlet servletA = new HttpServlet() {
+				private static final long serialVersionUID = 1L;
+				@Override
+				protected void service(HttpServletRequest req, HttpServletResponse resp)
+					throws IOException, ServletException {
+					HttpSession session = req.getSession();
+					session.setAttribute("test", req.getParameter("p1"));
+					getRequestURI.set(resp.encodeURL(req.getRequestURI()));
+				}
+			};
+
+			Dictionary<String, Object> props = new Hashtable<String, Object>();
+			props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_NAME, "SA");
+			props.put(HttpWhiteboardConstants.HTTP_WHITEBOARD_SERVLET_PATTERN, "/*");
+			registrations.add(getBundleContext().registerService(Servlet.class, servletA, props));
+			props = new Hashtable<String, Object>();
+
+			requestAdvisor.request("a/b/c/?p1=v1");
+			// get the session
+			String initialURI = getRequestURI.get();
+			int sessionIdx = initialURI.indexOf(";jsessionid=");
+			Assert.assertTrue("No session: " + initialURI, sessionIdx > -1);
+			String sessionPostfix = initialURI.substring(sessionIdx);
+			Assert.assertEquals("/a/b/c/" + sessionPostfix, getRequestURI.get());
+			requestAdvisor.request("a/b/" + sessionPostfix);
+			Assert.assertEquals("/a/b/" + sessionPostfix, getRequestURI.get());
+			requestAdvisor.request("a/" + sessionPostfix);
+			Assert.assertEquals("/a/" + sessionPostfix, getRequestURI.get());
+			// Note using empty string here because of the way requestAdvisor works
+			// by appending a slash first.
+			requestAdvisor.request("" + sessionPostfix);
+			Assert.assertEquals("/" + sessionPostfix, getRequestURI.get());
+		}
+		finally {
+			for (ServiceRegistration<?> registration : registrations) {
+				registration.unregister();
+			}
+		}
+	}
 
 	@Test
 	public void test_Listener1() throws Exception {
diff --git a/bundles/org.eclipse.equinox.http.servlet/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.http.servlet/META-INF/MANIFEST.MF
index 8e5f5b1b3..92348f3cc 100644
--- a/bundles/org.eclipse.equinox.http.servlet/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.equinox.http.servlet/META-INF/MANIFEST.MF
@@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2
 Bundle-Name: %bundleName
 Bundle-Vendor: %providerName
 Bundle-SymbolicName: org.eclipse.equinox.http.servlet
-Bundle-Version: 1.5.100.qualifier
+Bundle-Version: 1.5.200.qualifier
 Bundle-Activator: org.eclipse.equinox.http.servlet.internal.Activator
 Bundle-Localization: plugin
 Bundle-RequiredExecutionEnvironment: JavaSE-1.6
diff --git a/bundles/org.eclipse.equinox.http.servlet/pom.xml b/bundles/org.eclipse.equinox.http.servlet/pom.xml
index 5a45564fa..303618123 100644
--- a/bundles/org.eclipse.equinox.http.servlet/pom.xml
+++ b/bundles/org.eclipse.equinox.http.servlet/pom.xml
@@ -20,6 +20,6 @@
   </parent>
   <groupId>org.eclipse.equinox</groupId>
   <artifactId>org.eclipse.equinox.http.servlet</artifactId>
-  <version>1.5.100-SNAPSHOT</version>
+  <version>1.5.200-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
 </project>
diff --git a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ProxyServlet.java b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ProxyServlet.java
index db26e9dcb..a4d64bb2b 100644
--- a/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ProxyServlet.java
+++ b/bundles/org.eclipse.equinox.http.servlet/src/org/eclipse/equinox/http/servlet/internal/servlet/ProxyServlet.java
@@ -67,9 +67,9 @@ public class ProxyServlet extends HttpServlet {
 		}
 		String requestUri = HttpServletRequestWrapperImpl.getDispatchRequestURI(request);
 		
-		// NOTE split does not include trailing empty strings for paths that end in SLASH
-		String[] pathInfoSegments = pathInfo.split(Const.SLASH);
-		String[] requestUriSegments = requestUri.split(Const.SLASH);
+		// NOTE use split that takes a max to preserve ending SLASH
+		String[] pathInfoSegments = pathInfo.split(Const.SLASH, Integer.MAX_VALUE - 1);
+		String[] requestUriSegments = requestUri.split(Const.SLASH, Integer.MAX_VALUE - 1);
 		
 		if(pathInfoSegments.length == requestUriSegments.length) {
 			return requestUri;
@@ -79,10 +79,6 @@ public class ProxyServlet extends HttpServlet {
 		for(int i=(requestUriSegments.length - pathInfoSegments.length + 1);i<requestUriSegments.length;i++) {
 			aliasBuilder.append(Const.SLASH).append(requestUriSegments[i]);
 		}
-		// if the original request ends in '/' then be sure to append a '/'
-		if (requestUri.endsWith(Const.SLASH)) {
-			aliasBuilder.append(Const.SLASH);
-		}
 		return aliasBuilder.toString();
 	}