Revise SPA route detection

marksvc · marksvc · commit 3f66c57830e9 · 2025-09-12T13:36:45.000-06:00
diff --git a/src/SIL.XForge.Scripture/Startup.cs b/src/SIL.XForge.Scripture/Startup.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.IO;
+using System.Linq;
 using Autofac;
 using Autofac.Extensions.DependencyInjection;
 using Hangfire;
@@ -34,47 +35,62 @@ public enum SpaDevServerStartup
 
 public class Startup
 {
+    /// <summary>
+    /// Routes that should be handled by SPA in development but not in production.
+    /// </summary>
     private static readonly HashSet<string> DevelopmentSpaGetRoutes =
     [
-        "runtime.js",
-        "runtime.js.map",
-        "polyfills.js",
-        "polyfills.js.map",
-        "styles.css",
-        "styles.css.map",
-        "styles.js",
-        "styles.js.map",
-        "vendor.js",
-        "vendor.js.map",
-        "main.js",
-        "main.js.map",
         "@vite",
         "@fs",
-        "sockjs-node",
         "3rdpartylicenses.txt",
+        // sockjs-node is related to communication during `ng serve`
+        "sockjs-node",
     ];
 
-    // examples of filenames are "main-es5.4e5295b95e4b6c37b696.js", "styles.a2f070be0b37085d72ba.css"
-    private static readonly HashSet<string> ProductionSpaGetRoutes = ["polyfills", "main", "chunk", "styles"];
+    /// <summary>
+    /// Routes that should be handled by SPA in production but not in development
+    /// </summary>
+    private static readonly HashSet<string> ProductionSpaGetRoutes = [];
+
+    /// <summary>
+    /// Routes that should be handled by SPA in both production and development.
+    /// </summary>
     private static readonly HashSet<string> SpaGetRoutes =
     [
+        "index.html",
+        "prerendered-routes.json",
+        "3rdpartylicenses",
+        // PWA files
+        "ngsw.json",
+        "ngsw-worker.js",
+        "offline.html",
+        "safety-worker.js",
+        "sf-service-worker.js",
         "manifest.json",
+        // Application routes
         "callback",
         "connect-project",
         "login",
         "projects",
         "join",
         "serval-administration",
         "system-administration",
-        "favicon.ico",
+        // Asset and build files
         "assets",
+        "polyfills",
+        "main",
+        "chunk",
+        "styles",
+        "en",
+        "quill",
+        // Lynx-related
+        "worker",
+        "node_modules_sillsdev_lynx",
     ];
 
     private static readonly HashSet<string> DevelopmentSpaPostRoutes = ["sockjs-node"];
     private static readonly HashSet<string> ProductionSpaPostRoutes = [];
     private static readonly HashSet<string> SpaPostRoutes = [];
-    private const string SpaGetRoutesLynxPrefix = "node_modules_sillsdev_lynx";
-    private const string SpaGetRoutesWorkerPrefix = "worker-";
 
     public Startup(IConfiguration configuration, IWebHostEnvironment env, ILoggerFactory loggerFactory)
     {
@@ -320,39 +336,42 @@ IExceptionHandler exceptionHandler
         appLifetime.ApplicationStopped.Register(() => ApplicationContainer.Dispose());
     }
 
+    /// <summary>Is the request something that should be handled by the Angular SPA, instead of by ASP.NET?</summary>
+    /// <remarks>
+    /// A production environment will serve ASP.NET files in wwwroot and SPA files in dist/browser. A development
+    /// environment will serve ASP.NET files in wwwroot, and potentially SPA files from dist or in-memory files from `ng
+    /// serve`. Note that `ng serve` will generate different looking files depending on whether caching+prebundling is
+    /// used or not. When testing this method, it is helpful to run curl against port 5000 vs 4200. This method is
+    /// written with the assumption that undefined behaviour for malformed routes is not a concern for security or
+    /// functionality.
+    /// </remarks>
     internal bool IsSpaRoute(HttpContext context)
     {
         string path = context.Request.Path.Value;
-        if (path.Length <= 1)
-            return false;
-        int index = path.IndexOf("/", 1);
-        if (index == -1)
-            index = path.Length;
-        string prefix = path[1..index];
-        if (
-            !IsDevelopmentEnvironment
-            && (
-                prefix.EndsWith(".js")
-                || prefix.EndsWith(".js.map")
-                || prefix.EndsWith(".css")
-                || prefix.EndsWith(".css.map")
-            )
-        )
-        {
-            int hashDelimiterIndex = path.IndexOf("-");
-            prefix = prefix[..(hashDelimiterIndex - 1)];
-        }
-
-        bool isLazyChunkRoute =
-            context.Request.Method == HttpMethods.Get
-            && (prefix.StartsWith(SpaGetRoutesLynxPrefix) || prefix.StartsWith(SpaGetRoutesWorkerPrefix));
+        HashSet<string> spaRoutes =
+            context.Request.Method == HttpMethods.Get ? SpaGetRoutes
+            : context.Request.Method == HttpMethods.Post ? SpaPostRoutes
+            : [];
+
+        // SPA-handled paths will have forms like
+        //   /some-route/my-page?a=b
+        //   /polyfills-C3D4E5F6.js.map
+        //   /safety-worker.js
+        //   /@vite/client
+        // Anything could conceivably contain a '?'.
+
+        // Look at what is after the first slash, and before the next slash or '?'. Match paths like /projects/123456789
+        // as "projects", /login?a=b as "login", and /safety-worker.js as "safety-worker.js".
+        string exact = path.Split('/', '?').ElementAtOrDefault(1) ?? "";
+        if (spaRoutes.Contains(exact))
+            return true;
 
-        if (isLazyChunkRoute)
-        {
+        // Then look at what is before the first dash or dot. Match paths like /polyfills-C3D4E5F6.js.map and
+        // /polyfills.js as "polyfills".
+        string beginning = exact.Split(['-', '.']).FirstOrDefault();
+        if (spaRoutes.Contains(beginning))
             return true;
-        }
 
-        return (context.Request.Method == HttpMethods.Get && SpaGetRoutes.Contains(prefix))
-            || (context.Request.Method == HttpMethods.Post && SpaPostRoutes.Contains(prefix));
+        return false;
     }
 }
diff --git a/test/SIL.XForge.Scripture.Tests/StartupTests.cs b/test/SIL.XForge.Scripture.Tests/StartupTests.cs
@@ -11,6 +11,15 @@
 
 namespace SIL.XForge.Scripture;
 
+/// <summary>
+/// Represents what environment the application is running in.
+/// </summary>
+public enum RunMode
+{
+    Development,
+    Production,
+}
+
 [TestFixture]
 public class StartupTests
 {
@@ -115,6 +124,130 @@ public void IsSpaRoute_ProductionPath_IsRoute()
         Assert.IsTrue(actual);
     }
 
+    private static void IsSpaRoute_Helper(string path, RunMode[] runModes, bool expected)
+    {
+        foreach (RunMode runMode in runModes)
+        {
+            var env = new TestEnvironment(runMode.ToString());
+            env.Context.Request.Method = HttpMethods.Get;
+            env.Context.Request.Path = new PathString(path);
+            // SUT
+            bool actual = env.Startup.IsSpaRoute(env.Context);
+            Assert.AreEqual(expected, actual, $"Failed for path {path}, runMode {runMode}.");
+        }
+    }
+
+    // SPA route in production and development
+    [TestCase("/index.html")]
+    [TestCase("/chunk-A1B2C3D4.js")]
+    [TestCase("/chunk-A1B2C3D4.js.map")]
+    [TestCase("/chunk-E5F6G7H8.js")]
+    [TestCase("/chunk-E5F6G7H8.js.map")]
+    [TestCase("/main-Y5Z6A1B2.js")]
+    [TestCase("/main-Y5Z6A1B2.js.map")]
+    [TestCase("/polyfills-C3D4E5F6.js")]
+    [TestCase("/polyfills-C3D4E5F6.js.map")]
+    [TestCase("/styles-G7H8I9J0.css")]
+    [TestCase("/en-M3N4O5P6.js")]
+    [TestCase("/en-M3N4O5P6.js.map")]
+    [TestCase("/en-Q7R8S9T0.js")]
+    [TestCase("/en-Q7R8S9T0.js.map")]
+    [TestCase("/quill-U1V2W3X4.js")]
+    [TestCase("/quill-U1V2W3X4.js.map")]
+    [TestCase("/3rdpartylicenses.txt")]
+    [TestCase("/safety-worker.js")]
+    [TestCase("/sf-service-worker.js")]
+    [TestCase("/ngsw.json")]
+    [TestCase("/ngsw-worker.js")]
+    [TestCase("/offline.html")]
+    [TestCase("/prerendered-routes.json")]
+    [TestCase("/manifest.json")]
+    [TestCase("/assets/icons/sf-192x192.png")]
+    [TestCase("/assets/images/sf_logo_with_name_black.svg")]
+    [TestCase("/worker-I9J0K1L2.js")]
+    [TestCase("/worker-I9J0K1L2.js.map")]
+    [TestCase("/worker-basic.min.js")]
+    [TestCase("/node_modules_sillsdev_lynx")]
+    [TestCase("/projects")]
+    [TestCase("/projects/abc123/translate/GEN/1")]
+    [TestCase("/login")]
+    [TestCase("/login?sign-up=true")]
+    [TestCase("/join/AbCd/en-GB")]
+    [TestCase("/callback/auth0?code=1234&state=abcd")]
+    [TestCase("/connect-project")]
+    [TestCase("/serval-administration")]
+    [TestCase("/system-administration")]
+    public void IsSpaRoute_ProductionAndDevelopment_True(string path)
+    {
+        RunMode[] runModes = [RunMode.Production, RunMode.Development];
+        bool expected = true;
+        IsSpaRoute_Helper(path, runModes, expected);
+    }
+
+    // SPA route in development, but not expected in production
+    [TestCase("/@vite/client")]
+    [TestCase("/@fs/home/user/web-xforge/src/SIL.XForge.Scripture/ClientApp/node_modules/vite/dist/client/env.mjs")]
+    // `ng serve` with caching+prebundling can result in files like `main.js`, `polyfill.js`, and `styles.css` with no
+    // hashes in the filename.
+    [TestCase("/main.js")]
+    [TestCase("/polyfills.js")]
+    [TestCase("/styles.css")]
+    public void IsSpaRoute_Development_True(string path)
+    {
+        RunMode[] runModes = [RunMode.Development];
+        bool expected = true;
+        IsSpaRoute_Helper(path, runModes, expected);
+    }
+
+    // ASP.NET-handled path in production and development
+    [TestCase("/favicon.ico")]
+    [TestCase("/css/sf.min.css")]
+    [TestCase("/images/EarthLightsSmall.jpg")]
+    [TestCase("/scss/mixins/_breakpoints.scss")]
+    [TestCase("/scss/sf.scss")]
+    [TestCase("/images/multi-devices.svg")]
+    [TestCase("/images/community-checking.svg")]
+    [TestCase("/images/quoter.jpg")]
+    [TestCase("/terms")]
+    [TestCase("/privacy")]
+    [TestCase("/lib/material-design-lite/js/material.min.js")]
+    [TestCase("/lib/material-design-lite/css/material.sf_grey-pt_green.min.css")]
+    [TestCase("/non-existent-page")]
+    [TestCase("/?login")]
+    [TestCase("/?/login")]
+    [TestCase("//?login")]
+    [TestCase("//?/login")]
+    [TestCase("/??login")]
+    [TestCase("/??/login")]
+    [TestCase("/#login")]
+    [TestCase("/#/login")]
+    // It may or may not be possible for the path to be the empty string. If it is, let's have ASP.NET handle it.
+    [TestCase("")]
+    // The paths "/", "/Index", and "/Status/Error" are handled by ASP.NET and don't even get to IsSpaRoute. If they did
+    // for some reason, we'll have IsSpaRoute return false.
+    [TestCase("/")]
+    [TestCase("/Index")]
+    [TestCase("/Status/Error")]
+    // Paths with a beginning that start to match an SPA path, but ultimately don't, can be handled by ASP.NET.
+    [TestCase("/mainly")]
+    [TestCase("/enquiry")]
+    [TestCase("/workerbee")]
+    public void IsSpaRoute_ProductionAndDevelopment_False(string path)
+    {
+        RunMode[] runModes = [RunMode.Production, RunMode.Development];
+        bool expected = false;
+        IsSpaRoute_Helper(path, runModes, expected);
+    }
+
+    // ASP.NET-handled path in development, but not expected in production
+    [TestCase("/_framework/aspnetcore-browser-refresh.js")]
+    public void IsSpaRoute_Development_False(string path)
+    {
+        RunMode[] runModes = [RunMode.Development];
+        bool expected = false;
+        IsSpaRoute_Helper(path, runModes, expected);
+    }
+
     private class TestEnvironment
     {
         public TestEnvironment(string? environmentName = null)
