From 4d60b4a3ac9f5960b90fb6534f1c83635712c160 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 15 Sep 2016 10:54:47 +0200 Subject: [PATCH] SONAR-8073 no Throwable must be handled by Tomcat --- .../sonar/server/platform/web/RootFilter.java | 58 +++++++++++++------ .../server/platform/web/RootFilterTest.java | 31 ++++++++-- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/web/RootFilter.java b/server/sonar-server/src/main/java/org/sonar/server/platform/web/RootFilter.java index dec5a8d76c1..7d11e1782a0 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/web/RootFilter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/web/RootFilter.java @@ -31,11 +31,14 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; +import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import org.apache.commons.lang.StringUtils; import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Profiler; +import static java.lang.String.format; + /** *

Profile HTTP requests using platform profiling utility.

*

To avoid profiling of requests for static resources, the staticDirs @@ -72,23 +75,14 @@ public class RootFilter implements Filter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { if (request instanceof HttpServletRequest) { - HttpServletRequest httpRequest = new ServletRequestWrapper((HttpServletRequest) request); - String requestUri = httpRequest.getRequestURI(); - String rootDir = getRootDir(requestUri); - - if (staticResourceDirs.contains(rootDir)) { - // Static resource, not profiled - chain.doFilter(httpRequest, response); - } else { - Profiler profiler = Profiler.createIfDebug(Logger).start(); - try { - chain.doFilter(httpRequest, response); - } finally { - if (profiler.isDebugEnabled()) { - String queryString = httpRequest.getQueryString(); - String message = String.format(queryString == null ? MESSAGE_WITHOUT_QUERY : MESSAGE_WITH_QUERY, httpRequest.getMethod(), requestUri, queryString); - profiler.stopDebug(message); - } + HttpServletRequest httpRequest = (HttpServletRequest) request; + HttpServletResponse httpResponse = (HttpServletResponse) response; + try { + doFilter(new ServletRequestWrapper(httpRequest), httpResponse, chain); + } catch (Throwable e) { + Loggers.get(RootFilter.class).error(format("Processing of request %s failed", toUrl(httpRequest)), e); + if (!response.isCommitted()) { + httpResponse.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } } } else { @@ -97,6 +91,36 @@ public class RootFilter implements Filter { } } + private static String toUrl(HttpServletRequest request) { + String requestURI = request.getRequestURI(); + String queryString = request.getQueryString(); + if (queryString == null) { + return requestURI; + } + return requestURI + '?' + queryString; + } + + private void doFilter(HttpServletRequest httpRequest, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { + String requestUri = httpRequest.getRequestURI(); + String rootDir = getRootDir(requestUri); + + if (staticResourceDirs.contains(rootDir)) { + // Static resource, not profiled + chain.doFilter(httpRequest, response); + } else { + Profiler profiler = Profiler.createIfDebug(Logger).start(); + try { + chain.doFilter(httpRequest, response); + } finally { + if (profiler.isDebugEnabled()) { + String queryString = httpRequest.getQueryString(); + String message = format(queryString == null ? MESSAGE_WITHOUT_QUERY : MESSAGE_WITH_QUERY, httpRequest.getMethod(), requestUri, queryString); + profiler.stopDebug(message); + } + } + } + } + private String getRootDir(String requestUri) { String rootPath = ""; String localPath = StringUtils.substringAfter(requestUri, contextRoot); diff --git a/server/sonar-server/src/test/java/org/sonar/server/platform/web/RootFilterTest.java b/server/sonar-server/src/test/java/org/sonar/server/platform/web/RootFilterTest.java index aff2ff1b642..03878774212 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/platform/web/RootFilterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/platform/web/RootFilterTest.java @@ -22,20 +22,21 @@ package org.sonar.server.platform.web; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletContext; -import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.mockito.ArgumentCaptor; -import org.mockito.Mockito; import static org.assertj.core.api.Java6Assertions.assertThat; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -75,10 +76,22 @@ public class RootFilterTest { filter.doFilter(request("POST", "/context", null), null, chain); } - @Test(expected = ServletException.class) - public void should_profile_even_when_exception() throws Exception { - Mockito.doThrow(new ServletException()).when(chain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); - filter.doFilter(request("POST", "/context/service/call", "param=value"), null, chain); + @Test + public void throwable_in_dofilter_is_caught_and_500_error_returned_if_response_is_not_committed() throws Exception { + doThrow(new RuntimeException()).when(chain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); + HttpServletResponse response = mockHttpResponse(false); + filter.doFilter(request("POST", "/context/service/call", "param=value"), response, chain); + + verify(response).sendError(500); + } + + @Test + public void throwable_in_dofilter_is_caught_but_no_500_response_is_sent_if_response_already_committed() throws Exception { + doThrow(new RuntimeException()).when(chain).doFilter(any(ServletRequest.class), any(ServletResponse.class)); + HttpServletResponse response = mockHttpResponse(true); + filter.doFilter(request("POST", "/context/service/call", "param=value"), response, chain); + + verify(response, never()).sendError(500); } @Test @@ -149,4 +162,10 @@ public class RootFilterTest { when(request.getQueryString()).thenReturn(query); return request; } + + private HttpServletResponse mockHttpResponse(boolean commited) { + HttpServletResponse response = mock(HttpServletResponse.class); + when(response.isCommitted()).thenReturn(commited); + return response; + } } -- 2.39.5