Created new Filter to intercept and normalizate URIs (#17809)

* Created new Filter to intercept and normalizate URIs

* Applied feedback #17796

dotCMS/src/main/java/com/dotcms/filters/NormalizationFilter.java

@@ -0,0 +1,51 @@
+package com.dotcms.filters;
+
+import java.io.IOException;
+import java.net.URI;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+
+/**
+ * Filter created to wrap all the incoming requests to override the {@link
+ * HttpServletRequest#getRequestURI()} method in order to normalize the requested URIs.
+ */
+public class NormalizationFilter implements Filter {
+
+    @Override
+    public void init(FilterConfig filterConfig) throws ServletException {
+    }
+
+    @Override
+    public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse,
+            FilterChain filterChain) throws IOException, ServletException {
+
+        HttpServletRequestWrapper requestWrapper = new HttpServletRequestWrapper(
+                (HttpServletRequest) servletRequest) {
+
+            @Override
+            public String getRequestURI() {
+
+                /* Normalization is the process of removing unnecessary "." and ".." segments from the path component of a hierarchical URI.
+                 1. Each "." segment is simply removed.
+                 2. A ".." segment is removed only if it is preceded by a non-".." segment.
+                 3. Normalization has no effect upon opaque URIs. (mailto:a@b.com)
+                 */
+                return URI.create(super.getRequestURI()).normalize().toString();
+            }
+
+        };
+
+        filterChain.doFilter(requestWrapper, servletResponse);
+    }
+
+    @Override
+    public void destroy() {
+    }
+
+}

dotCMS/src/main/webapp/WEB-INF/web.xml

@@ -18,6 +18,11 @@
 	<!-- BEGIN CONTEXT-PARAMS -->
     <!-- END CONTEXT-PARAMS -->
 	<!-- DOTMARKETING FILTERS -->
+	<filter>
+		<filter-name>NormalizationFilter</filter-name>
+		<filter-class>com.dotcms.filters.NormalizationFilter</filter-class>
+		<async-supported>true</async-supported>
+	</filter>
 	<filter>
 		<filter-name>InterceptorFilter</filter-name>
 		<filter-class>com.dotmarketing.filters.InterceptorFilter</filter-class>
@@ -98,6 +103,10 @@
  	<!-- END FILTER-MAPPINGS -->
 
 	<!--DOTMARKETING FILTER-MAPPINGS-->
+	<filter-mapping>
+		<filter-name>NormalizationFilter</filter-name>
+		<url-pattern>/*</url-pattern>
+	</filter-mapping>
 	<filter-mapping>
 		<filter-name>InterceptorFilter</filter-name>
 		<url-pattern>/*</url-pattern>

dotCMS/src/test/java/com/dotcms/filters/NormalizationFilterTest.java

@@ -0,0 +1,115 @@
+package com.dotcms.filters;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.mock;
+
+import com.dotcms.UnitTestBase;
+import java.io.IOException;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
+public class NormalizationFilterTest extends UnitTestBase {
+
+    private final static NormalizationFilter normalizationFilter = new NormalizationFilter();
+    private static HttpServletResponse response;
+    private static FilterChain chain;
+    private static ArgumentCaptor<HttpServletRequest> capturedRequest;
+
+    @BeforeClass
+    public static void prepare() throws IOException, ServletException {
+
+        //Response
+        response = mock(HttpServletResponse.class);
+
+        //Chain
+        chain = mock(FilterChain.class);
+        capturedRequest = ArgumentCaptor
+                .forClass(HttpServletRequest.class);
+        //Capturing the request when is passed down to the chain
+        doNothing().when(chain).doFilter(capturedRequest.capture(), any(HttpServletResponse.class));
+    }
+
+    /**
+     * Test to verify the {@link NormalizationFilter} is applying properly the normalization on
+     * invalids URIs
+     */
+    @Test
+    public void test_uri_normalization_invalid_URI() throws IOException, ServletException {
+
+        // A ".." segment is removed only if it is preceded by a non-".." segment
+        String originalURI = "/test/../folder/important/secret_file.dat";
+        String expectedNormalizedURI = "/folder/important/secret_file.dat";
+        compare(originalURI, expectedNormalizedURI);
+
+        // A ".." segment is removed only if it is preceded by a non-".." segment
+        originalURI = "/test/../folder/folder1/forward_jsp.jsp?FORWARD_URL=http://google.com";
+        expectedNormalizedURI = "/folder/folder1/forward_jsp.jsp?FORWARD_URL=http://google.com";
+        compare(originalURI, expectedNormalizedURI);
+
+        // A ".." segment is removed only if it is preceded by a non-".." segment
+        originalURI = "/test/../folder/important/../secret_file.dat";
+        expectedNormalizedURI = "/folder/secret_file.dat";
+        compare(originalURI, expectedNormalizedURI);
+
+        // Each "." segment is simply removed
+        originalURI = "./folder/folder1/file.dat";
+        expectedNormalizedURI = "folder/folder1/file.dat";
+        compare(originalURI, expectedNormalizedURI);
+
+        // Each "." segment is simply removed
+        originalURI = "./folder/./folder1/file.dat";
+        expectedNormalizedURI = "folder/folder1/file.dat";
+        compare(originalURI, expectedNormalizedURI);
+
+        // Each "." segment is simply removed
+        originalURI = "/folder/./folder1/file.dat";
+        expectedNormalizedURI = "/folder/folder1/file.dat";
+        compare(originalURI, expectedNormalizedURI);
+    }
+
+    /**
+     * Test to verify the {@link NormalizationFilter} is applying properly the normalization on
+     * valid URIs
+     */
+    @Test
+    public void test_uri_normalization_valid_URI() throws IOException, ServletException {
+
+        String originalURI = "/folder/important/secret_file.dat";
+        compare(originalURI, originalURI);
+
+        originalURI = "/folder/folder1/forward_jsp.jsp?FORWARD_URL=http://google.com";
+        compare(originalURI, originalURI);
+
+        originalURI = "folder/folder1/file.dat";
+        compare(originalURI, originalURI);
+
+        // A ".." segment is removed only if it is preceded by a non-".." segment
+        originalURI = "../folder/folder1/file.dat";
+        compare(originalURI, originalURI);
+    }
+
+    private void compare(final String originalURI, final String expectedNormalizedURI)
+            throws IOException, ServletException {
+
+        HttpServletRequest request = mock(HttpServletRequest.class);
+        Mockito.when(request.getRequestURI()).thenReturn(originalURI);
+
+        //Calling the normalization filter
+        normalizationFilter.doFilter(request, response, chain);
+        //Verify the getRequestURI is working after passing throw the filter
+        final String normalizedValueByFilter = capturedRequest.getValue().getRequestURI();
+
+        assertNotNull(normalizedValueByFilter);
+        assertEquals(expectedNormalizedURI, normalizedValueByFilter);
+    }
+
+}