Enhance SPA routing logic

insights-webapp/src/main/java/org/frankframework/insights/InsightsWebappApplication.java

@@ -1,10 +1,10 @@
 package org.frankframework.insights;
 
 import static org.springframework.web.servlet.function.RequestPredicates.path;
-import static org.springframework.web.servlet.function.RequestPredicates.pathExtension;
 import static org.springframework.web.servlet.function.RouterFunctions.route;
 
 import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock;
+import org.jspecify.annotations.NullMarked;
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
 import org.springframework.boot.context.properties.ConfigurationPropertiesScan;
@@ -16,19 +16,15 @@
 import org.springframework.web.servlet.function.RouterFunction;
 import org.springframework.web.servlet.function.ServerResponse;
 
-/**
- * Entry point of the Insights web application.
- * <p>
- * This module only reads from the Insights database and serves it through the REST API and the
- * bundled Angular single page application. Filling the database is the responsibility of the
- * separate {@code insights-data-import} module.
- */
 @SpringBootApplication
 @EnableScheduling
 @EnableSchedulerLock(defaultLockAtMostFor = "PT2H", proxyTargetClass = true)
 @ConfigurationPropertiesScan
 @EnableWebSecurity
+@NullMarked
 public class InsightsWebappApplication {
+    private static final String FRONTEND_LOCATION = "frontend/";
+
     public static void main(String[] args) {
         SpringApplication app = configureApplication();
         app.run(args);
@@ -38,23 +34,12 @@ public static SpringApplication configureApplication() {
         return new SpringApplication(InsightsWebappApplication.class);
     }
 
-    /**
-     * This is a custom router function to accommodate to our single page application that we serve from this spring boot backend as well.
-     * This RouterFunction will make sure that we serve `frontend/index.html` whenever the path does not start with `/api/`, is not `/error` and does
-     * not have a non-numeric path extension (to exclude static resources like JS/CSS/images).
-     * Version strings like "v9.0.0" are intentionally not treated as file extensions since their suffix is numeric.
-     *
-     * @see <a href="https://github.com/spring-projects/spring-framework/issues/27257">Spring framework issue 27257</a> for more details.
-     */
     @Bean
     RouterFunction<ServerResponse> spaRouter() {
-        ClassPathResource index = new ClassPathResource("frontend/index.html");
-        RequestPredicate spaPredicate = path("/api/**")
-                .or(path("/error"))
-                .or(pathExtension(
-                        extension -> !extension.isBlank() && !extension.chars().allMatch(Character::isDigit)))
-                .negate();
+        RequestPredicate clientSideRoute = path("/api/**").or(path("/error")).negate();
 
-        return route().resource(spaPredicate, index).build();
+        return route().resources("/**", new ClassPathResource(FRONTEND_LOCATION))
+                .resource(clientSideRoute, new ClassPathResource(FRONTEND_LOCATION + "index.html"))
+                .build();
     }
 }

insights-webapp/src/test/java/org/frankframework/insights/InsightsApplicationSpaRouterTest.java

@@ -0,0 +1,311 @@
+package org.frankframework.insights;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.Resource;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.web.servlet.function.EntityResponse;
+import org.springframework.web.servlet.function.HandlerFunction;
+import org.springframework.web.servlet.function.RouterFunction;
+import org.springframework.web.servlet.function.ServerRequest;
+import org.springframework.web.servlet.function.ServerResponse;
+
+public class InsightsApplicationSpaRouterTest {
+
+    private static final String BUILD_OUTPUT = "frontend/";
+    private static final String INDEX = BUILD_OUTPUT + "index.html";
+
+    private final RouterFunction<ServerResponse> router = new InsightsWebappApplication().spaRouter();
+
+    @ParameterizedTest(name = "/graph/{0}")
+    @ValueSource(
+            strings = {
+                "10.2-nightly",
+                "10.1.1-nightly",
+                "9.2-nightly",
+                "master-nightly",
+                "nightly",
+                "v9.0.0",
+                "v7.7.1",
+                "v10.2.0",
+                "8.0",
+                "10.2",
+                "v10.2.0-RC1",
+                "v10.2.0-SNAPSHOT",
+                "v8.1.0-beta.2",
+                "release-10.2"
+            })
+    public void releaseTag_isServedTheIndex(String tagName) {
+        assertThat(servedResource("/graph/" + tagName)).contains(INDEX);
+    }
+
+    @Test
+    public void releaseBranchNightly_isServedTheIndex_regressionForIssue712() {
+        assertThat(servedResource("/graph/10.2-nightly")).contains(INDEX);
+    }
+
+    @Test
+    public void releaseBranchNightly_withNightlyQueryParameter_isServedTheIndex() {
+        assertThat(servedResource("/graph/10.2-nightly", "nightly=")).contains(INDEX);
+    }
+
+    @Test
+    public void releaseTag_withQueryParameters_isServedTheIndex() {
+        assertThat(servedResource("/graph/v9.0.0", "extended=2&range=%5B9.0%2C10.0%29&nightly="))
+                .contains(INDEX);
+    }
+
+    @Test
+    public void releaseTag_thatIsUrlEncoded_isServedTheIndex() {
+        assertThat(servedResource("/graph/release%2F10.2")).contains(INDEX);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(
+            strings = {
+                "/",
+                "/graph",
+                "/graph/",
+                "/roadmap",
+                "/cve-overview",
+                "/cve-overview/CVE-2024-12345",
+                "/vulnerabilities/manage",
+                "/vulnerabilities/manage/CVE-2024-12345",
+                "/release-manage/v9.0.0",
+                "/release-manage/10.2-nightly",
+                "/release-manage/10.2-nightly/business-values",
+                "/release-manage/10.2-nightly/business-values/42",
+                "/not-found",
+                "/some/unknown/deep/route"
+            })
+    public void clientSideRoute_isServedTheIndex(String path) {
+        assertThat(servedResource(path)).contains(INDEX);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(strings = {"/assets", "/assets/", "/assets/icons", "/media"})
+    public void directory_isServedTheIndex(String path) {
+        assertThat(servedResource(path)).contains(INDEX);
+    }
+
+    @ParameterizedTest(name = "/{0}")
+    @MethodSource("everyFileInTheAngularBuild")
+    public void existingFile_isServedAsItself(String file) {
+        assertThat(servedResource("/" + file)).contains(BUILD_OUTPUT + file);
+    }
+
+    private static List<String> everyFileInTheAngularBuild() throws IOException {
+        ClassPathResource buildOutput = new ClassPathResource(BUILD_OUTPUT);
+        assertThat(buildOutput.exists())
+                .as(
+                        "Angular build output is missing from the classpath at \"%s\"; run `mvn generate-resources` first",
+                        BUILD_OUTPUT)
+                .isTrue();
+
+        Path root = buildOutput.getFile().toPath();
+
+        try (Stream<Path> files = Files.walk(root)) {
+            return files.filter(Files::isRegularFile)
+                    .map(file -> root.relativize(file).toString().replace(File.separatorChar, '/'))
+                    .toList();
+        }
+    }
+
+    @Test
+    public void existingFile_withQueryParameters_isServedAsItself() throws IOException {
+        String bundle = anyJavaScriptBundle();
+
+        assertThat(servedResource("/" + bundle, "v=2")).contains(BUILD_OUTPUT + bundle);
+    }
+
+    @Test
+    public void existingFile_withTrailingSlash_isServedAsItself() throws IOException {
+        String bundle = anyJavaScriptBundle();
+
+        assertThat(servedResource("/" + bundle + "/")).contains(BUILD_OUTPUT + bundle);
+    }
+
+    private static String anyJavaScriptBundle() throws IOException {
+        return everyFileInTheAngularBuild().stream()
+                .filter(file -> file.endsWith(".js"))
+                .findFirst()
+                .orElseThrow();
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(strings = {"/does-not-exist.js", "/assets/missing.svg", "/main-STALEHASH.js"})
+    public void missingFile_fallsBackToTheIndex(String path) {
+        assertThat(servedResource(path)).contains(INDEX);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(
+            strings = {
+                "/../application.properties",
+                "/assets/../../application.properties",
+                "/%2e%2e%2fapplication.properties",
+                "/..%2fapplication.properties",
+                "/assets/%2e%2e/%2e%2e/application.properties",
+                "/assets/..%2f..%2fapplication.properties"
+            })
+    public void pathTraversal_fallsBackToTheIndex(String path) {
+        assertThat(servedResource(path)).contains(INDEX);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(
+            strings = {
+                "/application.properties",
+                "/application-prod.properties",
+                "/db/migration",
+                "/org/frankframework/insights/InsightsWebappApplication.class"
+            })
+    public void classpathResourceOutsideTheBuildOutput_fallsBackToTheIndex(String path) {
+        assertThat(servedResource(path)).contains(INDEX);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(
+            strings = {
+                "/api",
+                "/api/releases",
+                "/api/releases/10.2-nightly",
+                "/api/vulnerabilities/release/10.2-nightly",
+                "/api/auth/user",
+                "/api/unknown-endpoint",
+                "/error"
+            })
+    public void excludedPath_isNotRouted(String path) {
+        assertThat(route(path)).isEmpty();
+    }
+
+    @Test
+    public void routeStartingWithApiLetters_isServedTheIndex() {
+        assertThat(servedResource("/apidocs")).contains(INDEX);
+    }
+
+    @Test
+    public void routeNestedUnderError_isServedTheIndex() {
+        assertThat(servedResource("/error/details")).contains(INDEX);
+    }
+
+    @Test
+    public void errorPathWithTrailingSlash_isServedTheIndex() {
+        assertThat(servedResource("/error/")).contains(INDEX);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(strings = {"/api/", "/api/releases/"})
+    public void excludedApiPathWithTrailingSlash_isNotRouted(String path) {
+        assertThat(route(path)).isEmpty();
+    }
+
+    @Test
+    public void excludedPath_withQueryParameters_isNotRouted() {
+        assertThat(route("GET", "/api/releases", "page=2&size=20")).isEmpty();
+    }
+
+    @Test
+    public void headRequest_forClientSideRoute_isRouted() {
+        assertThat(route("HEAD", "/graph/10.2-nightly", null)).isPresent();
+    }
+
+    @Test
+    public void headRequest_forExistingFile_isRouted() throws IOException {
+        assertThat(route("HEAD", "/" + anyJavaScriptBundle(), null)).isPresent();
+    }
+
+    @Test
+    public void getRequest_forClientSideRoute_isServedWithStatusOk() {
+        assertThat(handle("GET", "/graph/10.2-nightly").statusCode()).isEqualTo(HttpStatus.OK);
+    }
+
+    @ParameterizedTest(name = "{0}")
+    @ValueSource(strings = {"POST", "PUT", "PATCH", "DELETE"})
+    public void writeRequest_forClientSideRoute_isRejectedAsMethodNotAllowed(String method) {
+        ServerResponse response = handle(method, "/graph/10.2-nightly");
+
+        assertThat(response.statusCode()).isEqualTo(HttpStatus.METHOD_NOT_ALLOWED);
+    }
+
+    @Test
+    public void optionsRequest_forClientSideRoute_advertisesTheSupportedMethods() {
+        ServerResponse response = handle("OPTIONS", "/graph/10.2-nightly");
+
+        assertThat(response.statusCode()).isEqualTo(HttpStatus.OK);
+        assertThat(response.headers().getAllow())
+                .containsExactlyInAnyOrder(HttpMethod.GET, HttpMethod.HEAD, HttpMethod.OPTIONS);
+    }
+
+    @ParameterizedTest(name = "/graph/{0} (extension-like suffix \"{1}\")")
+    @CsvSource({
+        "10.2-nightly, 2-nightly",
+        "10.1.1-nightly, 1-nightly",
+        "v9.0.0, 0",
+        "v10.2.0-RC1, 0-RC1",
+        "v10.2.0-SNAPSHOT, 0-SNAPSHOT"
+    })
+    public void tagWithExtensionLikeSuffix_isServedTheIndex(String tagName, String extensionLikeSuffix) {
+        assertThat(tagName).endsWith("." + extensionLikeSuffix);
+        assertThat(servedResource("/graph/" + tagName)).contains(INDEX);
+    }
+
+    private Optional<String> servedResource(String path) {
+        return servedResource(path, null);
+    }
+
+    private Optional<String> servedResource(String path, String queryString) {
+        ServerRequest request = request("GET", path, queryString);
+
+        return router.route(request)
+                .map(handler -> handle(handler, request))
+                .map(response -> (Resource) ((EntityResponse<?>) response).entity())
+                .map(resource -> ((ClassPathResource) resource).getPath());
+    }
+
+    private Optional<HandlerFunction<ServerResponse>> route(String path) {
+        return route("GET", path, null);
+    }
+
+    private Optional<HandlerFunction<ServerResponse>> route(String method, String path, String queryString) {
+        return router.route(request(method, path, queryString));
+    }
+
+    private ServerResponse handle(String method, String path) {
+        ServerRequest request = request(method, path, null);
+        HandlerFunction<ServerResponse> handler = router.route(request).orElseThrow();
+
+        return handle(handler, request);
+    }
+
+    private ServerResponse handle(HandlerFunction<ServerResponse> handler, ServerRequest request) {
+        try {
+            return handler.handle(request);
+        } catch (Exception e) {
+            throw new IllegalStateException("Could not handle " + request.path(), e);
+        }
+    }
+
+    private ServerRequest request(String method, String path, String queryString) {
+        MockHttpServletRequest servletRequest = new MockHttpServletRequest(method, path);
+        servletRequest.setQueryString(queryString);
+
+        return ServerRequest.create(servletRequest, Collections.emptyList());
+    }
+}

pom.xml

@@ -155,7 +155,11 @@
 			<groupId>org.slf4j</groupId>
 			<artifactId>slf4j-api</artifactId>
 		</dependency>
-
+		<dependency>
+			<groupId>org.jspecify</groupId>
+			<artifactId>jspecify</artifactId>
+			<scope>provided</scope>
+		</dependency>
 		<dependency>
 			<groupId>org.springframework.boot</groupId>
 			<artifactId>spring-boot-starter-test</artifactId>