Merge branch 'dev' into markdalgleish/lazy-route-properties

Author: markdalgleish

.changeset/gentle-gifts-kneel.md

@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+Fix types on `unstable_MiddlewareFunction` to avoid type errors when a middleware doesn't return a value

.changeset/honest-mangos-sneeze.md

@@ -0,0 +1,5 @@
+---
+"@react-router/express": patch
+---
+
+Better validation of `x-forwarded-host` header to preent potential security issues.

.changeset/six-mugs-scream.md

@@ -0,0 +1,5 @@
+---
+"@react-router/dev": patch
+---
+
+When `future.unstable_viteEnvironmentApi` is enabled, ensure critical CSS in development works when using a custom Vite `base` has been configured

integration/helpers/vite.ts

@@ -102,6 +102,7 @@ export const viteConfig = {
 
 export const EXPRESS_SERVER = (args: {
   port: number;
+  base?: string;
   loadContext?: Record<string, unknown>;
 }) =>
   String.raw`

integration/vite-css-test.ts

@@ -152,12 +152,19 @@ const files = {
   `,
 };
 
-const VITE_CONFIG = async (port: number) => dedent`
+const VITE_CONFIG = async ({
+  port,
+  base,
+}: {
+  port: number;
+  base?: string;
+}) => dedent`
   import { reactRouter } from "@react-router/dev/vite";
   import { vanillaExtractPlugin } from "@vanilla-extract/vite-plugin";
 
   export default {
     ${await viteConfig.server({ port })}
+    ${base ? `base: "${base}",` : ""}
     plugins: [
       reactRouter(),
       vanillaExtractPlugin({
@@ -182,7 +189,7 @@ test.describe("Vite CSS", () => {
               "react-router.config.ts": reactRouterConfig({
                 viteEnvironmentApi: templateName === "vite-6-template",
               }),
-              "vite.config.ts": await VITE_CONFIG(port),
+              "vite.config.ts": await VITE_CONFIG({ port }),
               ...files,
             },
             templateName
@@ -207,6 +214,45 @@ test.describe("Vite CSS", () => {
         });
       });
 
+      test.describe("vite dev with custom base", async () => {
+        let port: number;
+        let cwd: string;
+        let stop: () => void;
+        let base = "/custom/base/";
+
+        test.beforeAll(async () => {
+          port = await getPort();
+          cwd = await createProject(
+            {
+              "react-router.config.ts": reactRouterConfig({
+                viteEnvironmentApi: templateName === "vite-6-template",
+                basename: base,
+              }),
+              "vite.config.ts": await VITE_CONFIG({ port, base }),
+              ...files,
+            },
+            templateName
+          );
+          stop = await dev({ cwd, port });
+        });
+        test.afterAll(() => stop());
+
+        test.describe(() => {
+          test.use({ javaScriptEnabled: false });
+          test("without JS", async ({ page }) => {
+            await pageLoadWorkflow({ page, port, base });
+          });
+        });
+
+        test.describe(() => {
+          test.use({ javaScriptEnabled: true });
+          test("with JS", async ({ page }) => {
+            await pageLoadWorkflow({ page, port, base });
+            await hmrWorkflow({ page, port, cwd, base });
+          });
+        });
+      });
+
       test.describe("express", async () => {
         let port: number;
         let cwd: string;
@@ -215,7 +261,7 @@ test.describe("Vite CSS", () => {
         test.beforeAll(async () => {
           port = await getPort();
           cwd = await createProject({
-            "vite.config.ts": await VITE_CONFIG(port),
+            "vite.config.ts": await VITE_CONFIG({ port }),
             "server.mjs": EXPRESS_SERVER({ port }),
             ...files,
           });
@@ -247,7 +293,7 @@ test.describe("Vite CSS", () => {
         test.beforeAll(async () => {
           port = await getPort();
           cwd = await createProject({
-            "vite.config.ts": await VITE_CONFIG(port),
+            "vite.config.ts": await VITE_CONFIG({ port }),
             ...files,
           });
 
@@ -290,11 +336,19 @@ test.describe("Vite CSS", () => {
   });
 });
 
-async function pageLoadWorkflow({ page, port }: { page: Page; port: number }) {
+async function pageLoadWorkflow({
+  page,
+  port,
+  base,
+}: {
+  page: Page;
+  port: number;
+  base?: string;
+}) {
   let pageErrors: Error[] = [];
   page.on("pageerror", (error) => pageErrors.push(error));
 
-  await page.goto(`http://localhost:${port}/`, {
+  await page.goto(`http://localhost:${port}${base ?? "/"}`, {
     waitUntil: "networkidle",
   });
 
@@ -316,15 +370,17 @@ async function hmrWorkflow({
   page,
   cwd,
   port,
+  base,
 }: {
   page: Page;
   cwd: string;
   port: number;
+  base?: string;
 }) {
   let pageErrors: Error[] = [];
   page.on("pageerror", (error) => pageErrors.push(error));
 
-  await page.goto(`http://localhost:${port}/`, {
+  await page.goto(`http://localhost:${port}${base ?? "/"}`, {
     waitUntil: "networkidle",
   });
 

packages/react-router-dev/vite/plugin.ts

@@ -765,9 +765,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = () => {
               export const unstable_getCriticalCss = ({ pathname }) => {
                 return {
                   rel: "stylesheet",
-                  href: "${
-                    viteUserConfig.base ?? "/"
-                  }@react-router/critical.css?pathname=" + pathname,
+                  href: "${ctx.publicPath}@react-router/critical.css?pathname=" + pathname,
                 };
               }
             `
@@ -1566,7 +1564,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = () => {
         if (ctx.reactRouterConfig.future.unstable_viteEnvironmentApi) {
           viteDevServer.middlewares.use(async (req, res, next) => {
             let [reqPathname, reqSearch] = (req.url ?? "").split("?");
-            if (reqPathname === "/@react-router/critical.css") {
+            if (reqPathname === `${ctx.publicPath}@react-router/critical.css`) {
               let pathname = new URLSearchParams(reqSearch).get("pathname");
               if (!pathname) {
                 return next("No pathname provided");

packages/react-router-express/__tests__/server-test.ts

@@ -217,4 +217,28 @@ describe("express createRemixRequest", () => {
     );
     expect(remixRequest.headers.get("host")).toBe("localhost:3000");
   });
+
+  it("validates parsed port", async () => {
+    let expressRequest = createRequest({
+      url: "/foo/bar",
+      method: "GET",
+      protocol: "http",
+      hostname: "localhost",
+      headers: {
+        "Cache-Control": "max-age=300, s-maxage=3600",
+        Host: "localhost:3000",
+        "x-forwarded-host": ":/spoofed",
+      },
+    });
+    let expressResponse = createResponse();
+
+    let remixRequest = createRemixRequest(expressRequest, expressResponse);
+
+    expect(remixRequest.method).toBe("GET");
+    expect(remixRequest.headers.get("cache-control")).toBe(
+      "max-age=300, s-maxage=3600"
+    );
+    expect(remixRequest.headers.get("host")).toBe("localhost:3000");
+    expect(remixRequest.url).toBe("http://localhost:3000/foo/bar");
+  });
 });

packages/react-router-express/server.ts

@@ -98,9 +98,15 @@ export function createRemixRequest(
 ): Request {
   // req.hostname doesn't include port information so grab that from
   // `X-Forwarded-Host` or `Host`
-  let [, hostnamePort] = req.get("X-Forwarded-Host")?.split(":") ?? [];
-  let [, hostPort] = req.get("host")?.split(":") ?? [];
-  let port = hostnamePort || hostPort;
+  let [, hostnamePortStr] = req.get("X-Forwarded-Host")?.split(":") ?? [];
+  let [, hostPortStr] = req.get("host")?.split(":") ?? [];
+  let hostnamePort = Number.parseInt(hostnamePortStr, 10);
+  let hostPort = Number.parseInt(hostPortStr, 10);
+  let port = Number.isSafeInteger(hostnamePort)
+    ? hostnamePort
+    : Number.isSafeInteger(hostPort)
+    ? hostPort
+    : "";
   // Use req.hostname here as it respects the "trust proxy" setting
   let resolvedHost = `${req.hostname}${port ? `:${port}` : ""}`;
   // Use `req.originalUrl` so Remix is aware of the full path

packages/react-router/lib/router/utils.ts

@@ -220,7 +220,7 @@ export interface unstable_MiddlewareNextFunction<Result = unknown> {
 export type unstable_MiddlewareFunction<Result = unknown> = (
   args: DataFunctionArgs<unstable_RouterContextProvider>,
   next: unstable_MiddlewareNextFunction<Result>
-) => MaybePromise<Result | undefined>;
+) => MaybePromise<Result | void>;
 
 /**
  * Arguments passed to loader functions

packages/react-router/lib/types/route-module.ts

@@ -190,12 +190,12 @@ type ServerDataFunctionArgs<T extends RouteInfo> = {
 export type CreateServerMiddlewareFunction<T extends RouteInfo> = (
   args: ServerDataFunctionArgs<T>,
   next: unstable_MiddlewareNextFunction<Response>
-) => MaybePromise<Response | undefined>;
+) => MaybePromise<Response | void>;
 
 export type CreateClientMiddlewareFunction<T extends RouteInfo> = (
   args: ClientDataFunctionArgs<T>,
   next: unstable_MiddlewareNextFunction<undefined>
-) => MaybePromise<undefined>;
+) => MaybePromise<void>;
 
 export type CreateServerLoaderArgs<T extends RouteInfo> =
   ServerDataFunctionArgs<T>;