From 58a51147742df9eb82cbf7a4bdc01d2cd08e31a0 Mon Sep 17 00:00:00 2001 From: Marcio Lima Date: Wed, 19 Nov 2014 14:59:02 -0200 Subject: [PATCH 1/5] Bypass VRaptor filter in WebSocket requests. --- .../src/main/java/br/com/caelum/vraptor/VRaptor.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java index 5447b5acc..a80c40781 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java @@ -104,7 +104,13 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) final HttpServletRequest baseRequest = (HttpServletRequest) req; final HttpServletResponse baseResponse = (HttpServletResponse) res; - + + // Do not filter websocket requests -- this allows the use of websockets in VRaptor apps. + if ("websocket".equals(baseRequest.getHeader("upgrade"))) { + chain.doFilter(req, res); + return; + } + if (staticHandler.requestingStaticFile(baseRequest)) { staticHandler.deferProcessingToContainer(chain, baseRequest, baseResponse); } else { From aff770235ae34a21f496dc953d4bb467a3f4e43f Mon Sep 17 00:00:00 2001 From: Marcio Lima Date: Wed, 19 Nov 2014 23:58:42 -0200 Subject: [PATCH 2/5] 1. Moving the Websocket request identification to an specific method. 2. Creation of a unit test for the Websocket request bypass mechanism. --- .../main/java/br/com/caelum/vraptor/VRaptor.java | 9 ++++++++- .../java/br/com/caelum/vraptor/VRaptorTest.java | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java index a80c40781..83bb911bd 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java @@ -106,7 +106,7 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) final HttpServletResponse baseResponse = (HttpServletResponse) res; // Do not filter websocket requests -- this allows the use of websockets in VRaptor apps. - if ("websocket".equals(baseRequest.getHeader("upgrade"))) { + if (isWebsocketRequest(baseRequest)) { chain.doFilter(req, res); return; } @@ -178,4 +178,11 @@ private void validateIfCdiIsFound() throws ServletException { throw new ServletException("Dependencies were not set. Do you have a Weld/CDI listener setup in your web.xml?"); } } + + private boolean isWebsocketRequest(HttpServletRequest request) { + // according to the Websocket spec (https://tools.ietf.org/html/rfc6455): The WebSocket Protocol + // 5. The request MUST contain an |Upgrade| header field whose value MUST include the "websocket" keyword. + return request.getHeader("Upgrade") != null && request.getHeader("Upgrade").toLowerCase().contains("websocket"); + } + } diff --git a/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java b/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java index cf994c886..9d08a77b5 100644 --- a/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java +++ b/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java @@ -19,6 +19,7 @@ import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import javax.inject.Inject; import javax.servlet.FilterChain; @@ -33,6 +34,7 @@ import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; + @RunWith(WeldJunitRunner.class) public class VRaptorTest { @@ -60,4 +62,18 @@ public void shouldDeferToContainerIfStaticFile() throws Exception{ vRaptor.doFilter(request, response, chain); assertThat(handler.isDeferProcessingToContainerCalled(), is(true)); } + + @Test + public void shouldBypassWebsocketRequests() throws Exception{ + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + FilterChain chain = mock(FilterChain.class); + + when(request.getHeader("Upgrade")).thenReturn("Websocket"); + + handler.setRequestingStaticFile(false); + vRaptor.doFilter(request, response, chain); + assertThat(handler.isDeferProcessingToContainerCalled(), is(false)); + } + } From d63f531e2548c47d274077d9938be60c100d3caa Mon Sep 17 00:00:00 2001 From: Marcio Lima Date: Thu, 20 Nov 2014 16:13:26 -0200 Subject: [PATCH 3/5] Moving comments within the method to the method's javadoc. --- .../src/main/java/br/com/caelum/vraptor/VRaptor.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java index 83bb911bd..bb3374b72 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java @@ -105,7 +105,6 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) final HttpServletRequest baseRequest = (HttpServletRequest) req; final HttpServletResponse baseResponse = (HttpServletResponse) res; - // Do not filter websocket requests -- this allows the use of websockets in VRaptor apps. if (isWebsocketRequest(baseRequest)) { chain.doFilter(req, res); return; @@ -179,10 +178,12 @@ private void validateIfCdiIsFound() throws ServletException { } } + /** + * According to the Websocket spec (https://tools.ietf.org/html/rfc6455): The WebSocket Protocol + * 5. The request MUST contain an |Upgrade| header field whose value MUST include the "websocket" keyword. + */ private boolean isWebsocketRequest(HttpServletRequest request) { - // according to the Websocket spec (https://tools.ietf.org/html/rfc6455): The WebSocket Protocol - // 5. The request MUST contain an |Upgrade| header field whose value MUST include the "websocket" keyword. - return request.getHeader("Upgrade") != null && request.getHeader("Upgrade").toLowerCase().contains("websocket"); + return request.getHeader("Upgrade") != null && request.getHeader("Upgrade").toLowerCase().contains("websocket"); } } From 58eac50f59ef086d7f2e3a4b70559ba5a744cea7 Mon Sep 17 00:00:00 2001 From: Marcio Lima Date: Thu, 20 Nov 2014 23:54:28 -0200 Subject: [PATCH 4/5] Avoiding a second call to request.getHeader() --- vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java index bb3374b72..dd0376da2 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/VRaptor.java @@ -183,7 +183,8 @@ private void validateIfCdiIsFound() throws ServletException { * 5. The request MUST contain an |Upgrade| header field whose value MUST include the "websocket" keyword. */ private boolean isWebsocketRequest(HttpServletRequest request) { - return request.getHeader("Upgrade") != null && request.getHeader("Upgrade").toLowerCase().contains("websocket"); + String upgradeHeader = request.getHeader("Upgrade"); + return upgradeHeader != null && upgradeHeader.toLowerCase().contains("websocket"); } } From b36da4f571938ac2ed3638beed7d8732534c4959 Mon Sep 17 00:00:00 2001 From: Marcio Lima Date: Thu, 20 Nov 2014 23:54:52 -0200 Subject: [PATCH 5/5] Refactoring the test to reflect the expected behaviour --- .../br/com/caelum/vraptor/VRaptorTest.java | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java b/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java index 9d08a77b5..726aacc9d 100644 --- a/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java +++ b/vraptor-core/src/test/java/br/com/caelum/vraptor/VRaptorTest.java @@ -16,9 +16,11 @@ */ package br.com.caelum.vraptor; - import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import javax.inject.Inject; @@ -34,15 +36,16 @@ import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; - @RunWith(WeldJunitRunner.class) public class VRaptorTest { @Rule public ExpectedException exception = ExpectedException.none(); - @Inject private VRaptor vRaptor; - @Inject private MockStaticContentHandler handler; + @Inject + private VRaptor vRaptor; + @Inject + private MockStaticContentHandler handler; @Test public void shoudlComplainIfNotInAServletEnviroment() throws Exception { @@ -54,7 +57,7 @@ public void shoudlComplainIfNotInAServletEnviroment() throws Exception { } @Test - public void shouldDeferToContainerIfStaticFile() throws Exception{ + public void shouldDeferToContainerIfStaticFile() throws Exception { HttpServletRequest request = mock(HttpServletRequest.class); HttpServletResponse response = mock(HttpServletResponse.class); FilterChain chain = mock(FilterChain.class); @@ -62,18 +65,20 @@ public void shouldDeferToContainerIfStaticFile() throws Exception{ vRaptor.doFilter(request, response, chain); assertThat(handler.isDeferProcessingToContainerCalled(), is(true)); } - + @Test - public void shouldBypassWebsocketRequests() throws Exception{ + public void shouldBypassWebsocketRequests() throws Exception { HttpServletRequest request = mock(HttpServletRequest.class); HttpServletResponse response = mock(HttpServletResponse.class); FilterChain chain = mock(FilterChain.class); - + when(request.getHeader("Upgrade")).thenReturn("Websocket"); - handler.setRequestingStaticFile(false); vRaptor.doFilter(request, response, chain); - assertThat(handler.isDeferProcessingToContainerCalled(), is(false)); + verify(request).getHeader("Upgrade"); + verify(chain).doFilter(request, response); + + verifyNoMoreInteractions(request, response); } - + }