Mailing-List: contact commits-help@felix.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@felix.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1567569 - in /felix/trunk/http:
 base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
 itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java
Date: Wed, 12 Feb 2014 08:54:19 -0000
To: commits@felix.apache.org
From: jawi@apache.org
Message-Id: <20140212085419.7D208238899C@eris.apache.org>

Author: jawi
Date: Wed Feb 12 08:54:19 2014
New Revision: 1567569

URL: http://svn.apache.org/r1567569
Log:
FELIX-3054: Incorrect PathInfo & ServletPath:

- the ServletRequest given to the HttpContext was not wrapped, causing it
  to provide incorrect PathInfo & ServletPath information. A small addition
  to the solution for FELIX-2774 provides a proper fix. Added itest to 
  verify this behaviour.


Modified:
    felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
    felix/trunk/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java

Modified: felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
URL: http://svn.apache.org/viewvc/felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java?rev=1567569&r1=1567568&r2=1567569&view=diff
==============================================================================
--- felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java (original)
+++ felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java Wed Feb 12 08:54:19 2014
@@ -325,6 +325,13 @@ public final class ServletHandler extend
 
     final void doHandle(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException
     {
+        // Only wrap the original ServletRequest in case we're handling plain requests, 
+        // not inclusions or forwards from servlets. Should solve FELIX-2774 and FELIX-3054... 
+        if (DispatcherType.REQUEST == req.getDispatcherType())
+        {
+            req = new ServletHandlerRequest(req, getContext(), this.alias);
+        }
+
         // set a sensible status code in case handleSecurity returns false
         // but fails to send a response
         res.setStatus(HttpServletResponse.SC_FORBIDDEN);
@@ -333,16 +340,7 @@ public final class ServletHandler extend
             // reset status to OK for further processing
             res.setStatus(HttpServletResponse.SC_OK);
 
-            // Only wrap the original ServletRequest in case we're handling plain requests, 
-            // not inclusions or forwards from servlets. Should solve FELIX-2774 (partly)... 
-            if (DispatcherType.REQUEST == req.getDispatcherType())
-            {
-                this.servlet.service(new ServletHandlerRequest(req, getContext(), this.alias), res);
-            }
-            else
-            {
-                this.servlet.service(req, res);
-            }
+            this.servlet.service(req, res);
         }
     }
 

Modified: felix/trunk/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java?rev=1567569&r1=1567568&r2=1567569&view=diff
==============================================================================
--- felix/trunk/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java (original)
+++ felix/trunk/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java Wed Feb 12 08:54:19 2014
@@ -108,108 +108,48 @@ public class HttpJettyTest extends BaseI
         assertResponseCode(SC_NOT_FOUND, createURL("/test"));
     }
 
-    /**
-     * Tests that we can register a filter with Jetty and that its lifecycle is correctly controlled.
-     */
     @Test
-    public void testRegisterFilterLifecycleOk() throws Exception
-    {
-        CountDownLatch initLatch = new CountDownLatch(1);
-        CountDownLatch destroyLatch = new CountDownLatch(1);
-
-        TestFilter filter = new TestFilter(initLatch, destroyLatch);
-
-        register("/test", filter);
-
-        assertTrue(initLatch.await(5, TimeUnit.SECONDS));
-
-        unregister(filter);
-
-        assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
-    }
-
-    /**
-     * Tests that we can register a servlet with Jetty and that its lifecycle is correctly controlled.
-     */
-    @Test
-    public void testRegisterServletLifecycleOk() throws Exception
-    {
-        CountDownLatch initLatch = new CountDownLatch(1);
-        CountDownLatch destroyLatch = new CountDownLatch(1);
-
-        TestServlet servlet = new TestServlet(initLatch, destroyLatch);
-
-        register("/test", servlet);
-
-        assertTrue(initLatch.await(5, TimeUnit.SECONDS));
-
-        unregister(servlet);
-
-        assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
-    }
-
-    @Test
-    public void testUseServletContextOk() throws Exception
+    public void testCorrectPathInfoInHttpContextOk() throws Exception
     {
         CountDownLatch initLatch = new CountDownLatch(1);
         CountDownLatch destroyLatch = new CountDownLatch(1);
 
         HttpContext context = new HttpContext()
         {
-            public boolean handleSecurity(HttpServletRequest request, HttpServletResponse response) throws IOException
-            {
-                return true;
-            }
-
-            public URL getResource(String name)
+            public String getMimeType(String name)
             {
-                try
-                {
-                    File f = new File("src/test/resources/resource/" + name);
-                    if (f.exists())
-                    {
-                        return f.toURI().toURL();
-                    }
-                }
-                catch (MalformedURLException e)
-                {
-                    fail();
-                }
                 return null;
             }
 
-            public String getMimeType(String name)
+            public URL getResource(String name)
             {
                 return null;
             }
-        };
-
-        TestServlet servlet = new TestServlet(initLatch, destroyLatch)
-        {
-            private static final long serialVersionUID = 1L;
 
-            @Override
-            public void init(ServletConfig config) throws ServletException
+            public boolean handleSecurity(HttpServletRequest request, HttpServletResponse response) throws IOException
             {
-                ServletContext context = config.getServletContext();
                 try
                 {
-                    assertEquals("", context.getContextPath());
-                    assertNotNull(context.getResource("test.html"));
-                    assertNotNull(context.getRealPath("test.html"));
+                    assertEquals("", request.getContextPath());
+                    assertEquals("/foo", request.getServletPath());
+                    assertEquals("/bar", request.getPathInfo());
+                    assertEquals("/foo/bar", request.getRequestURI());
+                    assertEquals("qux=quu", request.getQueryString());
+                    return true;
                 }
-                catch (MalformedURLException e)
+                catch (Exception e)
                 {
-                    fail();
+                    e.printStackTrace();
                 }
-
-                super.init(config);
+                return false;
             }
         };
 
+        TestServlet servlet = new TestServlet(initLatch, destroyLatch);
+
         register("/foo", servlet, context);
 
-        URL testURL = createURL("/foo");
+        URL testURL = createURL("/foo/bar?qux=quu");
 
         assertTrue(initLatch.await(5, TimeUnit.SECONDS));
 
@@ -306,4 +246,118 @@ public class HttpJettyTest extends BaseI
 
         assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
     }
+
+    /**
+     * Tests that we can register a filter with Jetty and that its lifecycle is correctly controlled.
+     */
+    @Test
+    public void testRegisterFilterLifecycleOk() throws Exception
+    {
+        CountDownLatch initLatch = new CountDownLatch(1);
+        CountDownLatch destroyLatch = new CountDownLatch(1);
+
+        TestFilter filter = new TestFilter(initLatch, destroyLatch);
+
+        register("/test", filter);
+
+        assertTrue(initLatch.await(5, TimeUnit.SECONDS));
+
+        unregister(filter);
+
+        assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
+    }
+
+    /**
+     * Tests that we can register a servlet with Jetty and that its lifecycle is correctly controlled.
+     */
+    @Test
+    public void testRegisterServletLifecycleOk() throws Exception
+    {
+        CountDownLatch initLatch = new CountDownLatch(1);
+        CountDownLatch destroyLatch = new CountDownLatch(1);
+
+        TestServlet servlet = new TestServlet(initLatch, destroyLatch);
+
+        register("/test", servlet);
+
+        assertTrue(initLatch.await(5, TimeUnit.SECONDS));
+
+        unregister(servlet);
+
+        assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
+    }
+
+    @Test
+    public void testUseServletContextOk() throws Exception
+    {
+        CountDownLatch initLatch = new CountDownLatch(1);
+        CountDownLatch destroyLatch = new CountDownLatch(1);
+
+        HttpContext context = new HttpContext()
+        {
+            public String getMimeType(String name)
+            {
+                return null;
+            }
+
+            public URL getResource(String name)
+            {
+                try
+                {
+                    File f = new File("src/test/resources/resource/" + name);
+                    if (f.exists())
+                    {
+                        return f.toURI().toURL();
+                    }
+                }
+                catch (MalformedURLException e)
+                {
+                    fail();
+                }
+                return null;
+            }
+
+            public boolean handleSecurity(HttpServletRequest request, HttpServletResponse response) throws IOException
+            {
+                return true;
+            }
+        };
+
+        TestServlet servlet = new TestServlet(initLatch, destroyLatch)
+        {
+            private static final long serialVersionUID = 1L;
+
+            @Override
+            public void init(ServletConfig config) throws ServletException
+            {
+                ServletContext context = config.getServletContext();
+                try
+                {
+                    assertEquals("", context.getContextPath());
+                    assertNotNull(context.getResource("test.html"));
+                    assertNotNull(context.getRealPath("test.html"));
+                }
+                catch (MalformedURLException e)
+                {
+                    fail();
+                }
+
+                super.init(config);
+            }
+        };
+
+        register("/foo", servlet, context);
+
+        URL testURL = createURL("/foo");
+
+        assertTrue(initLatch.await(5, TimeUnit.SECONDS));
+
+        assertResponseCode(SC_OK, testURL);
+
+        unregister(servlet);
+
+        assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
+
+        assertResponseCode(SC_NOT_FOUND, testURL);
+    }
 }