FELIX-3988 - better handling of HttpContext#handleSecurity:
- in case the HttpContext#handleSecurity either committed its own
response, or did set a non-default status code, do not overwrite
the status code with SC_FORBIDDEN;
- modified logic for both the FilterHandler & ServletHandler to make
them symmetrical, added additional test cases to verify the
behaviour.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1567863 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/FilterHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/FilterHandler.java
index 52770fe..aa8464d 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/FilterHandler.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/FilterHandler.java
@@ -16,6 +16,7 @@
*/
package org.apache.felix.http.base.internal.handler;
+import static javax.servlet.http.HttpServletResponse.*;
import java.io.IOException;
import java.util.regex.Pattern;
@@ -102,16 +103,21 @@
final void doHandle(HttpServletRequest req, HttpServletResponse res, FilterChain chain) throws ServletException, IOException
{
- if (!getContext().handleSecurity(req, res))
- {
- res.sendError(HttpServletResponse.SC_FORBIDDEN);
- }
- else
+ if (getContext().handleSecurity(req, res))
{
this.filter.doFilter(req, res, chain);
}
+ else
+ {
+ // FELIX-3988: If the response is not yet committed and still has the default
+ // status, we're going to override this and send an error instead.
+ if (!res.isCommitted() && res.getStatus() == SC_OK)
+ {
+ res.sendError(SC_FORBIDDEN);
+ }
+ }
}
-
+
@Override
protected Object getSubject()
{
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
index 3a8c0a1..a31e5aa 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/handler/ServletHandler.java
@@ -26,6 +26,8 @@
import static javax.servlet.RequestDispatcher.INCLUDE_QUERY_STRING;
import static javax.servlet.RequestDispatcher.INCLUDE_REQUEST_URI;
import static javax.servlet.RequestDispatcher.INCLUDE_SERVLET_PATH;
+import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
import static org.apache.felix.http.base.internal.util.UriUtils.concat;
import java.io.IOException;
@@ -332,16 +334,19 @@
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);
if (getContext().handleSecurity(req, res))
{
- // reset status to OK for further processing
- res.setStatus(HttpServletResponse.SC_OK);
-
this.servlet.service(req, res);
}
+ else
+ {
+ // FELIX-3988: If the response is not yet committed and still has the default
+ // status, we're going to override this and send an error instead.
+ if (!res.isCommitted() && res.getStatus() == SC_OK)
+ {
+ res.sendError(SC_FORBIDDEN);
+ }
+ }
}
@Override
diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/FilterHandlerTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/FilterHandlerTest.java
index 53da35d..8aaebac 100644
--- a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/FilterHandlerTest.java
+++ b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/FilterHandlerTest.java
@@ -16,16 +16,27 @@
*/
package org.apache.felix.http.base.internal.handler;
-import org.junit.Test;
-import org.junit.Before;
-import org.junit.Assert;
-import org.mockito.Mockito;
+import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static javax.servlet.http.HttpServletResponse.SC_PAYMENT_REQUIRED;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
import javax.servlet.Filter;
-import javax.servlet.FilterConfig;
import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.junit.Before;
+import org.junit.Test;
+
public class FilterHandlerTest extends AbstractHandlerTest
{
private Filter filter;
@@ -34,17 +45,7 @@
public void setUp()
{
super.setUp();
- this.filter = Mockito.mock(Filter.class);
- }
-
- protected AbstractHandler createHandler()
- {
- return createHandler("dummy", 0);
- }
-
- private FilterHandler createHandler(String pattern, int ranking)
- {
- return new FilterHandler(this.context, this.filter, pattern, ranking, null /* name */);
+ this.filter = mock(Filter.class);
}
@Test
@@ -53,8 +54,163 @@
FilterHandler h1 = createHandler("a", 0);
FilterHandler h2 = createHandler("b", 10);
- Assert.assertEquals(1, h1.compareTo(h2));
- Assert.assertEquals(-1, h2.compareTo(h1));
+ assertEquals(1, h1.compareTo(h2));
+ assertEquals(-1, h2.compareTo(h1));
+ }
+
+ @Test
+ public void testDestroy()
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ h1.destroy();
+ verify(this.filter).destroy();
+ }
+
+ @Test
+ public void testHandleFound() throws Exception
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+ when(this.context.handleSecurity(req, res)).thenReturn(true);
+
+ when(req.getPathInfo()).thenReturn("/a");
+ h1.handle(req, res, chain);
+
+ verify(this.filter).doFilter(req, res, chain);
+ verify(chain, never()).doFilter(req, res);
+ }
+
+ @Test
+ public void testHandleFoundContextRoot() throws Exception
+ {
+ FilterHandler h1 = createHandler("/", 0);
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+ when(this.context.handleSecurity(req, res)).thenReturn(true);
+
+ when(req.getPathInfo()).thenReturn(null);
+ h1.handle(req, res, chain);
+
+ verify(this.filter).doFilter(req, res, chain);
+ verify(chain, never()).doFilter(req, res);
+ }
+
+ /**
+ * FELIX-3988: only send an error for uncomitted responses with default status codes.
+ */
+ @Test
+ public void testHandleFoundForbidden() throws Exception
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+
+ when(req.getPathInfo()).thenReturn("/a");
+ // Default behaviour: uncomitted response and default status code...
+ when(res.isCommitted()).thenReturn(false);
+ when(res.getStatus()).thenReturn(SC_OK);
+
+ when(this.context.handleSecurity(req, res)).thenReturn(false);
+
+ h1.handle(req, res, chain);
+
+ verify(this.filter, never()).doFilter(req, res, chain);
+ verify(chain, never()).doFilter(req, res);
+ verify(res).sendError(SC_FORBIDDEN);
+ }
+
+ /**
+ * FELIX-3988: do not try to write to an already committed response.
+ */
+ @Test
+ public void testHandleFoundForbiddenCommittedOwnResponse() throws Exception
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+
+ when(req.getPathInfo()).thenReturn("/a");
+ // Simulate an already committed response...
+ when(res.isCommitted()).thenReturn(true);
+ when(res.getStatus()).thenReturn(SC_OK);
+
+ when(this.context.handleSecurity(req, res)).thenReturn(false);
+
+ h1.handle(req, res, chain);
+
+ verify(this.filter, never()).doFilter(req, res, chain);
+ verify(chain, never()).doFilter(req, res);
+ // Should not be called from our handler...
+ verify(res, never()).sendError(SC_FORBIDDEN);
+ }
+
+ /**
+ * FELIX-3988: do not overwrite custom set status code.
+ */
+ @Test
+ public void testHandleFoundForbiddenCustomStatusCode() throws Exception
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+
+ when(req.getPathInfo()).thenReturn("/a");
+ // Simulate an uncommitted response with a non-default status code...
+ when(res.isCommitted()).thenReturn(false);
+ when(res.getStatus()).thenReturn(SC_PAYMENT_REQUIRED);
+
+ when(this.context.handleSecurity(req, res)).thenReturn(false);
+
+ h1.handle(req, res, chain);
+
+ verify(this.filter, never()).doFilter(req, res, chain);
+ verify(chain, never()).doFilter(req, res);
+ // Should not be called from our handler...
+ verify(res, never()).sendError(SC_FORBIDDEN);
+ }
+
+ @Test
+ public void testHandleNotFound() throws Exception
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+
+ when(req.getPathInfo()).thenReturn("/");
+ h1.handle(req, res, chain);
+
+ verify(this.filter, never()).doFilter(req, res, chain);
+ verify(chain).doFilter(req, res);
+ }
+
+ @Test
+ public void testHandleNotFoundContextRoot() throws Exception
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ FilterChain chain = mock(FilterChain.class);
+
+ when(req.getPathInfo()).thenReturn(null);
+ h1.handle(req, res, chain);
+
+ verify(this.filter, never()).doFilter(req, res, chain);
+ verify(chain).doFilter(req, res);
+ }
+
+ @Test
+ public void testInit() throws Exception
+ {
+ FilterHandler h1 = createHandler("/a", 0);
+ h1.init();
+ verify(this.filter).init(any(FilterConfig.class));
}
@Test
@@ -65,114 +221,29 @@
FilterHandler h3 = createHandler("/", 0);
FilterHandler h4 = createHandler("/.*", 0);
- Assert.assertFalse(h1.matches(null));
- Assert.assertFalse(h1.matches("/a"));
- Assert.assertTrue(h1.matches("/a/b"));
- Assert.assertFalse(h1.matches("/a/b/c"));
- Assert.assertFalse(h2.matches(null));
- Assert.assertFalse(h1.matches("/a"));
- Assert.assertTrue(h2.matches("/a/b/c"));
- Assert.assertFalse(h2.matches("/a/b/"));
- Assert.assertTrue(h3.matches(null));
- Assert.assertTrue(h3.matches("/"));
- Assert.assertFalse(h3.matches("/a/b/"));
- Assert.assertTrue(h4.matches(null));
- Assert.assertTrue(h4.matches("/"));
- Assert.assertTrue(h4.matches("/a/b/"));
+ assertFalse(h1.matches(null));
+ assertFalse(h1.matches("/a"));
+ assertTrue(h1.matches("/a/b"));
+ assertFalse(h1.matches("/a/b/c"));
+ assertFalse(h2.matches(null));
+ assertFalse(h1.matches("/a"));
+ assertTrue(h2.matches("/a/b/c"));
+ assertFalse(h2.matches("/a/b/"));
+ assertTrue(h3.matches(null));
+ assertTrue(h3.matches("/"));
+ assertFalse(h3.matches("/a/b/"));
+ assertTrue(h4.matches(null));
+ assertTrue(h4.matches("/"));
+ assertTrue(h4.matches("/a/b/"));
}
- @Test
- public void testInit() throws Exception
+ protected AbstractHandler createHandler()
{
- FilterHandler h1 = createHandler("/a", 0);
- h1.init();
- Mockito.verify(this.filter).init(Mockito.any(FilterConfig.class));
+ return createHandler("dummy", 0);
}
- @Test
- public void testDestroy()
+ private FilterHandler createHandler(String pattern, int ranking)
{
- FilterHandler h1 = createHandler("/a", 0);
- h1.destroy();
- Mockito.verify(this.filter).destroy();
- }
-
- @Test
- public void testHandleNotFound() throws Exception
- {
- FilterHandler h1 = createHandler("/a", 0);
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- FilterChain chain = Mockito.mock(FilterChain.class);
-
- Mockito.when(req.getPathInfo()).thenReturn("/");
- h1.handle(req, res, chain);
-
- Mockito.verify(this.filter, Mockito.never()).doFilter(req, res, chain);
- Mockito.verify(chain).doFilter(req, res);
- }
-
- @Test
- public void testHandleFound() throws Exception
- {
- FilterHandler h1 = createHandler("/a", 0);
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- FilterChain chain = Mockito.mock(FilterChain.class);
- Mockito.when(this.context.handleSecurity(req, res)).thenReturn(true);
-
- Mockito.when(req.getPathInfo()).thenReturn("/a");
- h1.handle(req, res, chain);
-
- Mockito.verify(this.filter).doFilter(req, res, chain);
- Mockito.verify(chain, Mockito.never()).doFilter(req, res);
- }
-
- @Test
- public void testHandleFoundForbidden() throws Exception
- {
- FilterHandler h1 = createHandler("/a", 0);
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- FilterChain chain = Mockito.mock(FilterChain.class);
- Mockito.when(this.context.handleSecurity(req, res)).thenReturn(false);
-
- Mockito.when(req.getPathInfo()).thenReturn("/a");
- h1.handle(req, res, chain);
-
- Mockito.verify(this.filter, Mockito.never()).doFilter(req, res, chain);
- Mockito.verify(chain, Mockito.never()).doFilter(req, res);
- Mockito.verify(res).sendError(HttpServletResponse.SC_FORBIDDEN);
- }
-
- @Test
- public void testHandleNotFoundContextRoot() throws Exception
- {
- FilterHandler h1 = createHandler("/a", 0);
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- FilterChain chain = Mockito.mock(FilterChain.class);
-
- Mockito.when(req.getPathInfo()).thenReturn(null);
- h1.handle(req, res, chain);
-
- Mockito.verify(this.filter, Mockito.never()).doFilter(req, res, chain);
- Mockito.verify(chain).doFilter(req, res);
- }
-
- @Test
- public void testHandleFoundContextRoot() throws Exception
- {
- FilterHandler h1 = createHandler("/", 0);
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- FilterChain chain = Mockito.mock(FilterChain.class);
- Mockito.when(this.context.handleSecurity(req, res)).thenReturn(true);
-
- Mockito.when(req.getPathInfo()).thenReturn(null);
- h1.handle(req, res, chain);
-
- Mockito.verify(this.filter).doFilter(req, res, chain);
- Mockito.verify(chain, Mockito.never()).doFilter(req, res);
+ return new FilterHandler(this.context, this.filter, pattern, ranking, null /* name */);
}
}
diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java
index f1f9626..a8c14d2 100644
--- a/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java
+++ b/http/base/src/test/java/org/apache/felix/http/base/internal/handler/ServletHandlerTest.java
@@ -16,15 +16,25 @@
*/
package org.apache.felix.http.base.internal.handler;
+import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static javax.servlet.http.HttpServletResponse.SC_PAYMENT_REQUIRED;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
import javax.servlet.Servlet;
import javax.servlet.ServletConfig;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
-import org.mockito.Mockito;
public class ServletHandlerTest extends AbstractHandlerTest
{
@@ -34,7 +44,180 @@
public void setUp()
{
super.setUp();
- this.servlet = Mockito.mock(Servlet.class);
+ this.servlet = mock(Servlet.class);
+ }
+
+ @Test
+ public void testCompare()
+ {
+ ServletHandler h1 = createHandler("/a");
+ ServletHandler h2 = createHandler("/a/b");
+
+ assertEquals(2, h1.compareTo(h2));
+ assertEquals(-2, h2.compareTo(h1));
+ }
+
+ @Test
+ public void testDestroy()
+ {
+ ServletHandler h1 = createHandler("/a");
+ h1.destroy();
+ verify(this.servlet).destroy();
+ }
+
+ @Test
+ public void testHandleFound() throws Exception
+ {
+ ServletHandler h1 = createHandler("/a");
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ when(this.context.handleSecurity(req, res)).thenReturn(true);
+
+ when(req.getPathInfo()).thenReturn("/a/b");
+ boolean result = h1.handle(req, res);
+
+ assertTrue(result);
+ verify(this.servlet).service(any(HttpServletRequest.class), any(HttpServletResponse.class));
+ }
+
+ @Test
+ public void testHandleFoundContextRoot() throws Exception
+ {
+ ServletHandler h1 = createHandler("/");
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ when(this.context.handleSecurity(req, res)).thenReturn(true);
+
+ when(req.getPathInfo()).thenReturn(null);
+ boolean result = h1.handle(req, res);
+
+ assertTrue(result);
+ verify(this.servlet).service(any(HttpServletRequest.class), any(HttpServletResponse.class));
+ }
+
+ /**
+ * FELIX-3988: only send an error for uncomitted responses with default status codes.
+ */
+ @Test
+ public void testHandleFoundForbidden() throws Exception
+ {
+ ServletHandler h1 = createHandler("/a");
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+
+ when(req.getPathInfo()).thenReturn("/a");
+ // Default behaviour: uncomitted response and default status code...
+ when(res.isCommitted()).thenReturn(false);
+ when(res.getStatus()).thenReturn(SC_OK);
+
+ when(this.context.handleSecurity(req, res)).thenReturn(false);
+
+ when(req.getPathInfo()).thenReturn("/a/b");
+ boolean result = h1.handle(req, res);
+
+ assertTrue(result);
+ verify(this.servlet, never()).service(req, res);
+ verify(res).sendError(SC_FORBIDDEN);
+ }
+
+ /**
+ * FELIX-3988: do not try to write to an already committed response.
+ */
+ @Test
+ public void testHandleFoundForbiddenCommittedOwnResponse() throws Exception
+ {
+ ServletHandler h1 = createHandler("/a");
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+
+ when(req.getPathInfo()).thenReturn("/a");
+ // Comitted response with default status code...
+ when(res.isCommitted()).thenReturn(true);
+ when(res.getStatus()).thenReturn(SC_OK);
+
+ when(this.context.handleSecurity(req, res)).thenReturn(false);
+
+ when(req.getPathInfo()).thenReturn("/a/b");
+ boolean result = h1.handle(req, res);
+
+ assertTrue(result);
+ verify(this.servlet, never()).service(req, res);
+ verify(res, never()).sendError(SC_FORBIDDEN);
+ }
+
+ /**
+ * FELIX-3988: do not overwrite custom set status code.
+ */
+ @Test
+ public void testHandleFoundForbiddenCustomStatusCode() throws Exception
+ {
+ ServletHandler h1 = createHandler("/a");
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+
+ when(req.getPathInfo()).thenReturn("/a");
+ // Unomitted response with default status code...
+ when(res.isCommitted()).thenReturn(false);
+ when(res.getStatus()).thenReturn(SC_PAYMENT_REQUIRED);
+
+ when(this.context.handleSecurity(req, res)).thenReturn(false);
+
+ when(req.getPathInfo()).thenReturn("/a/b");
+ boolean result = h1.handle(req, res);
+
+ assertTrue(result);
+ verify(this.servlet, never()).service(req, res);
+ verify(res, never()).sendError(SC_FORBIDDEN);
+ }
+
+ @Test
+ public void testHandleNotFound() throws Exception
+ {
+ ServletHandler h1 = createHandler("/a");
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+
+ when(req.getPathInfo()).thenReturn("/");
+ boolean result = h1.handle(req, res);
+
+ assertFalse(result);
+ verify(this.servlet, never()).service(req, res);
+ }
+
+ @Test
+ public void testHandleNotFoundContextRoot() throws Exception
+ {
+ ServletHandler h1 = createHandler("/a");
+ HttpServletRequest req = mock(HttpServletRequest.class);
+ HttpServletResponse res = mock(HttpServletResponse.class);
+ when(this.context.handleSecurity(req, res)).thenReturn(true);
+
+ when(req.getPathInfo()).thenReturn(null);
+ boolean result = h1.handle(req, res);
+
+ assertFalse(result);
+ verify(this.servlet, never()).service(req, res);
+ }
+
+ @Test
+ public void testInit() throws Exception
+ {
+ ServletHandler h1 = createHandler("/a");
+ h1.init();
+ verify(this.servlet).init(any(ServletConfig.class));
+ }
+
+ @Test
+ public void testMatches()
+ {
+ ServletHandler h1 = createHandler("/a/b");
+
+ assertFalse(h1.matches(null));
+ assertFalse(h1.matches("/"));
+ assertFalse(h1.matches("/a/"));
+ assertTrue(h1.matches("/a/b"));
+ assertTrue(h1.matches("/a/b/"));
+ assertTrue(h1.matches("/a/b/c"));
}
protected AbstractHandler createHandler()
@@ -46,118 +229,4 @@
{
return new ServletHandler(this.context, this.servlet, alias, null /* name */);
}
-
- @Test
- public void testCompare()
- {
- ServletHandler h1 = createHandler("/a");
- ServletHandler h2 = createHandler("/a/b");
-
- Assert.assertEquals(2, h1.compareTo(h2));
- Assert.assertEquals(-2, h2.compareTo(h1));
- }
-
- @Test
- public void testMatches()
- {
- ServletHandler h1 = createHandler("/a/b");
-
- Assert.assertFalse(h1.matches(null));
- Assert.assertFalse(h1.matches("/"));
- Assert.assertFalse(h1.matches("/a/"));
- Assert.assertTrue(h1.matches("/a/b"));
- Assert.assertTrue(h1.matches("/a/b/"));
- Assert.assertTrue(h1.matches("/a/b/c"));
- }
-
- @Test
- public void testInit() throws Exception
- {
- ServletHandler h1 = createHandler("/a");
- h1.init();
- Mockito.verify(this.servlet).init(Mockito.any(ServletConfig.class));
- }
-
- @Test
- public void testDestroy()
- {
- ServletHandler h1 = createHandler("/a");
- h1.destroy();
- Mockito.verify(this.servlet).destroy();
- }
-
- @Test
- public void testHandleNotFound() throws Exception
- {
- ServletHandler h1 = createHandler("/a");
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
-
- Mockito.when(req.getPathInfo()).thenReturn("/");
- boolean result = h1.handle(req, res);
-
- Assert.assertFalse(result);
- Mockito.verify(this.servlet, Mockito.never()).service(req, res);
- }
-
- @Test
- public void testHandleFound() throws Exception
- {
- ServletHandler h1 = createHandler("/a");
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- Mockito.when(this.context.handleSecurity(req, res)).thenReturn(true);
-
- Mockito.when(req.getPathInfo()).thenReturn("/a/b");
- boolean result = h1.handle(req, res);
-
- Assert.assertTrue(result);
- Mockito.verify(this.servlet).service(Mockito.any(HttpServletRequest.class), Mockito.any(HttpServletResponse.class));
- }
-
- @Test
- public void testHandleFoundForbidden() throws Exception
- {
- ServletHandler h1 = createHandler("/a");
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- Mockito.when(this.context.handleSecurity(req, res)).thenReturn(false);
-
- Mockito.when(req.getPathInfo()).thenReturn("/a/b");
- boolean result = h1.handle(req, res);
-
- Assert.assertTrue(result);
- Mockito.verify(this.servlet, Mockito.never()).service(req, res);
- Mockito.verify(res).setStatus(HttpServletResponse.SC_FORBIDDEN);
- }
-
- @Test
- public void testHandleNotFoundContextRoot() throws Exception
- {
- ServletHandler h1 = createHandler("/a");
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- Mockito.when(this.context.handleSecurity(req, res)).thenReturn(true);
-
- Mockito.when(req.getPathInfo()).thenReturn(null);
- boolean result = h1.handle(req, res);
-
- Assert.assertFalse(result);
- Mockito.verify(this.servlet, Mockito.never()).service(req, res);
- }
-
- @Test
- public void testHandleFoundContextRoot() throws Exception
- {
- ServletHandler h1 = createHandler("/");
- HttpServletRequest req = Mockito.mock(HttpServletRequest.class);
- HttpServletResponse res = Mockito.mock(HttpServletResponse.class);
- Mockito.when(this.context.handleSecurity(req, res)).thenReturn(true);
-
- Mockito.when(req.getPathInfo()).thenReturn(null);
- boolean result = h1.handle(req, res);
-
- Assert.assertTrue(result);
- Mockito.verify(this.servlet).service(Mockito.any(HttpServletRequest.class), Mockito.any(HttpServletResponse.class));
- }
}
diff --git a/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java b/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java
index caa9809..ba69bf3 100644
--- a/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java
+++ b/http/itest/src/test/java/org/apache/felix/http/itest/HttpJettyTest.java
@@ -21,6 +21,7 @@
import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static javax.servlet.http.HttpServletResponse.SC_PAYMENT_REQUIRED;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
@@ -35,6 +36,7 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
+import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.ServletConfig;
import javax.servlet.ServletContext;
@@ -248,6 +250,108 @@
}
/**
+ * Test case for FELIX-3988, handling security constraints in {@link Filter}s.
+ */
+ @Test
+ public void testHandleSecurityInFilterOk() 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)
+ {
+ return null;
+ }
+
+ public boolean handleSecurity(HttpServletRequest request, HttpServletResponse response) throws IOException
+ {
+ if (request.getParameter("setStatus") != null)
+ {
+ response.setStatus(SC_PAYMENT_REQUIRED);
+ }
+ else if (request.getParameter("sendError") != null)
+ {
+ response.sendError(SC_PAYMENT_REQUIRED);
+ }
+ else if (request.getParameter("commit") != null)
+ {
+ response.getWriter().append("Not allowed!");
+ response.flushBuffer();
+ }
+ return false;
+ }
+ };
+
+ TestFilter filter = new TestFilter(initLatch, destroyLatch);
+
+ register("/.*", filter, context);
+
+ URL url1 = createURL("/foo");
+ URL url2 = createURL("/foo?sendError=true");
+ URL url3 = createURL("/foo?setStatus=true");
+ URL url4 = createURL("/foo?commit=true");
+
+ assertTrue(initLatch.await(5, TimeUnit.SECONDS));
+
+ assertResponseCode(SC_FORBIDDEN, url1);
+ assertResponseCode(SC_PAYMENT_REQUIRED, url2);
+ assertResponseCode(SC_PAYMENT_REQUIRED, url3);
+ assertContent(SC_OK, "Not allowed!", url4);
+
+ unregister(filter);
+
+ assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
+
+ assertResponseCode(SC_NOT_FOUND, url1);
+ }
+
+ /**
+ * Tests the handling of URI path parameters.
+ */
+ @Test
+ public void testHandleUriPathParametersOk() throws Exception
+ {
+ CountDownLatch initLatch = new CountDownLatch(1);
+ CountDownLatch destroyLatch = new CountDownLatch(1);
+
+ TestServlet servlet = new TestServlet(initLatch, destroyLatch)
+ {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
+ {
+ assertEquals("", request.getContextPath());
+ assertEquals("/foo", request.getServletPath());
+ assertEquals("/a", request.getPathInfo()); // /a,b/c;d/e.f;g/h
+ assertEquals("/foo/a;b/c;d/e;f;g/h", request.getRequestURI());
+ assertEquals("i=j+k&l=m", request.getQueryString());
+ }
+ };
+
+ register("/foo", servlet);
+
+ URL testURL = createURL("/foo/a;b/c;d/e;f;g/h?i=j+k&l=m");
+
+ assertTrue(initLatch.await(5, TimeUnit.SECONDS));
+
+ assertResponseCode(SC_OK, testURL);
+
+ unregister(servlet);
+
+ assertTrue(destroyLatch.await(5, TimeUnit.SECONDS));
+
+ assertResponseCode(SC_NOT_FOUND, testURL);
+ }
+
+ /**
* Tests that we can register a filter with Jetty and that its lifecycle is correctly controlled.
*/
@Test