fix: generate valid URL route paths for pages on Windows

Previously route paths were manipulated by file-system path utilities.
On Windows this resulted in URLs that had backslashes, which are invalid for such URLs.

Fixes cloudflare#51
Closes cloudflare#235
Closes cloudflare#330
Closes cloudflare#327

packages/wrangler/pages/functions/filepath-routing.test.ts

@@ -1,39 +1,47 @@
+import { toUrlPath } from "../../src/paths";
 import { compareRoutes } from "./filepath-routing";
 
 describe("compareRoutes()", () => {
+  const url = toUrlPath;
   test("routes / last", () => {
-    expect(compareRoutes("/", "/foo")).toBeGreaterThanOrEqual(1);
-    expect(compareRoutes("/", "/:foo")).toBeGreaterThanOrEqual(1);
-    expect(compareRoutes("/", "/:foo*")).toBeGreaterThanOrEqual(1);
+    expect(compareRoutes([url("/")], [url("/foo")])).toBeGreaterThanOrEqual(1);
+    expect(compareRoutes([url("/")], [url("/:foo")])).toBeGreaterThanOrEqual(1);
+    expect(compareRoutes([url("/")], [url("/:foo*")])).toBeGreaterThanOrEqual(
+      1
+    );
   });
 
   test("routes with fewer segments come after those with more segments", () => {
-    expect(compareRoutes("/foo", "/foo/bar")).toBeGreaterThanOrEqual(1);
-    expect(compareRoutes("/foo", "/foo/bar/cat")).toBeGreaterThanOrEqual(1);
+    expect(
+      compareRoutes([url("/foo")], [url("/foo/bar")])
+    ).toBeGreaterThanOrEqual(1);
+    expect(
+      compareRoutes([url("/foo")], [url("/foo/bar/cat")])
+    ).toBeGreaterThanOrEqual(1);
   });
 
   test("routes with wildcard segments come after those without", () => {
-    expect(compareRoutes("/:foo*", "/foo")).toBe(1);
-    expect(compareRoutes("/:foo*", "/:foo")).toBe(1);
+    expect(compareRoutes([url("/:foo*")], [url("/foo")])).toBe(1);
+    expect(compareRoutes([url("/:foo*")], [url("/:foo")])).toBe(1);
   });
 
   test("routes with dynamic segments come after those without", () => {
-    expect(compareRoutes("/:foo", "/foo")).toBe(1);
+    expect(compareRoutes([url("/:foo")], [url("/foo")])).toBe(1);
   });
 
-  test("routes with dynamic segments occuring earlier come after those with dynamic segments in later positions", () => {
-    expect(compareRoutes("/foo/:id/bar", "/foo/bar/:id")).toBe(1);
+  test("routes with dynamic segments occurring earlier come after those with dynamic segments in later positions", () => {
+    expect(compareRoutes([url("/foo/:id/bar")], [url("/foo/bar/:id")])).toBe(1);
   });
 
   test("routes with no HTTP method come after those specifying a method", () => {
-    expect(compareRoutes("/foo", "GET /foo")).toBe(1);
+    expect(compareRoutes([url("/foo")], [url("/foo"), "GET"])).toBe(1);
   });
 
   test("two equal routes are sorted according to their original position in the list", () => {
-    expect(compareRoutes("GET /foo", "GET /foo")).toBe(0);
+    expect(compareRoutes([url("/foo"), "GET"], [url("/foo"), "GET"])).toBe(0);
   });
 
   test("it returns -1 if the first argument should appear first in the list", () => {
-    expect(compareRoutes("GET /foo", "/foo")).toBe(-1);
+    expect(compareRoutes([url("/foo"), "GET"], [url("/foo")])).toBe(-1);
   });
 });

packages/wrangler/pages/functions/filepath-routing.ts

@@ -3,26 +3,31 @@ import path from "node:path";
 import * as acorn from "acorn";
 import * as acornWalk from "acorn-walk";
 import { transform } from "esbuild";
+import { toUrlPath } from "../../src/paths";
+import type { UrlPath } from "../../src/paths";
 import type { Config, RouteConfig } from "./routes";
 import type { ExportNamedDeclaration, Identifier } from "estree";
 
 type Arguments = {
   baseDir: string;
-  baseURL: string;
+  baseURL: UrlPath;
 };
 
+type Method = "GET" | "POST" | "PUT" | "PATCH" | "DELETE" | "OPTIONS" | "HEAD";
+type RouteKey = [UrlPath, Method] | [UrlPath, undefined] | [UrlPath];
+
 export async function generateConfigFromFileTree({
   baseDir,
   baseURL,
 }: Arguments) {
-  let routeEntries: [string, RouteConfig][] = [];
+  let routeEntries: [RouteKey, RouteConfig][] = [];
 
   if (!baseURL.startsWith("/")) {
-    baseURL = `/${baseURL}`;
+    baseURL = `/${baseURL}` as UrlPath;
   }
 
   if (baseURL.endsWith("/")) {
-    baseURL = baseURL.slice(0, -1);
+    baseURL = baseURL.slice(0, -1) as UrlPath;
   }
 
   await forEachFile(baseDir, async (filepath) => {
@@ -108,12 +113,14 @@ export async function generateConfigFromFileTree({
               routePath = routePath.replace(/\[\[(.+)]]/g, ":$1*"); // transform [[id]] => :id*
               routePath = routePath.replace(/\[(.+)]/g, ":$1"); // transform [id] => :id
 
-              if (method) {
-                routePath = `${method.toUpperCase()} ${routePath}`;
-              }
+              const routeUrlPath = toUrlPath(routePath);
+              const routeKey: RouteKey = [
+                routeUrlPath,
+                method ? (method.toUpperCase() as Method) : undefined,
+              ];
 
               routeEntries.push([
-                routePath,
+                routeKey,
                 {
                   [isMiddlewareFile ? "middleware" : "module"]: [
                     `${path.relative(baseDir, filepath)}:${exportName}`,
@@ -146,28 +153,24 @@ export async function generateConfigFromFileTree({
     []
   );
 
-  routeEntries.sort(([pathA], [pathB]) => compareRoutes(pathA, pathB));
+  routeEntries.sort(([a], [b]) => compareRoutes(a, b));
 
   return { routes: Object.fromEntries(routeEntries) } as Config;
 }
 
 // Ensure routes are produced in order of precedence so that
 // more specific routes aren't occluded from matching due to
 // less specific routes appearing first in the route list.
-export function compareRoutes(a: string, b: string) {
-  function parseRoutePath(routePath: string): [string | null, string[]] {
-    const parts = routePath.split(" ", 2);
-    // split() will guarantee at least one element.
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    const segmentedPath = parts.pop()!;
-    const method = parts.pop() ?? null;
-
-    const segments = segmentedPath.slice(1).split("/").filter(Boolean);
-    return [method, segments];
+export function compareRoutes(
+  [routePathA, methodA]: RouteKey,
+  [routePathB, methodB]: RouteKey
+) {
+  function parseRoutePath(routePath: UrlPath): string[] {
+    return routePath.slice(1).split("/").filter(Boolean);
   }
 
-  const [methodA, segmentsA] = parseRoutePath(a);
-  const [methodB, segmentsB] = parseRoutePath(b);
+  const segmentsA = parseRoutePath(routePathA);
+  const segmentsB = parseRoutePath(routePathB);
 
   // sort routes with fewer segments after those with more segments
   if (segmentsA.length !== segmentsB.length) {
@@ -193,8 +196,8 @@ export function compareRoutes(a: string, b: string) {
   if (methodA && !methodB) return -1;
   if (!methodA && methodB) return 1;
 
-  // all else equal, just sort them lexicographically
-  return a.localeCompare(b);
+  // all else equal, just sort the paths lexicographically
+  return routePathA.localeCompare(routePathB);
 }
 
 async function forEachFile<T>(

packages/wrangler/pages/functions/routes.ts

@@ -1,6 +1,8 @@
 import fs from "node:fs/promises";
 import path from "node:path";
+import { toUrlPath } from "../../src/paths";
 import { isValidIdentifier, normalizeIdentifier } from "./identifiers";
+import type { UrlPath } from "../../src/paths";
 
 export const HTTP_METHODS = [
   "HEAD",
@@ -19,7 +21,7 @@ export function isHTTPMethod(
 }
 
 export type RoutesCollection = Array<{
-  routePath: string;
+  routePath: UrlPath;
   methods: HTTPMethod[];
   modules: string[];
   middlewares: string[];
@@ -31,7 +33,7 @@ export type Config = {
 };
 
 export type RoutesConfig = {
-  [route: string]: RouteConfig;
+  [route: UrlPath]: RouteConfig;
 };
 
 export type RouteConfig = {
@@ -122,7 +124,7 @@ export function parseConfig(config: Config, baseDir: string) {
     }
 
     routes.push({
-      routePath,
+      routePath: toUrlPath(routePath),
       methods: _methods.split("|").filter(isHTTPMethod),
       middlewares: parseModuleIdentifiers(props.middleware),
       modules: parseModuleIdentifiers(props.module),

packages/wrangler/src/pages.tsx

@@ -11,6 +11,7 @@ import open from "open";
 import { buildWorker } from "../pages/functions/buildWorker";
 import { generateConfigFromFileTree } from "../pages/functions/filepath-routing";
 import { writeRoutesModule } from "../pages/functions/routes";
+import { toUrlPath } from "./paths";
 import type { Config } from "../pages/functions/routes";
 import type { Headers, Request, fetch } from "@miniflare/core";
 import type { BuildResult } from "esbuild";
@@ -665,16 +666,17 @@ async function buildFunctions({
   );
 
   const routesModule = join(tmpdir(), "./functionsRoutes.mjs");
+  const baseURL = toUrlPath("/");
 
   const config: Config = await generateConfigFromFileTree({
     baseDir: functionsDirectory,
-    baseURL: "/",
+    baseURL,
   });
 
   if (outputConfigPath) {
     writeFileSync(
       outputConfigPath,
-      JSON.stringify({ ...config, baseURL: "/" }, null, 2)
+      JSON.stringify({ ...config, baseURL }, null, 2)
     );
   }
 

packages/wrangler/src/paths.ts

@@ -0,0 +1,26 @@
+import { assert } from "console";
+
+type DiscriminatedPath<Discriminator extends string> = string & {
+  _discriminator: Discriminator;
+};
+
+/**
+ * A branded string that expects to be URL compatible.
+ *
+ * Require this type when you want callers to ensure that they have converted file-path strings into URL-safe paths.
+ */
+export type UrlPath = DiscriminatedPath<"UrlPath">;
+
+/**
+ * Convert a file-path string to a URL-path string.
+ *
+ * Use this helper to convert a `string` to a `UrlPath` when it is not clear whether the string needs normalizing.
+ * Replaces all back-slashes with forward-slashes, and throws an error if the path contains a drive letter (e.g. `C:`).
+ */
+export function toUrlPath(path: string): UrlPath {
+  assert(
+    !/^[a-z]:/i.test(path),
+    "Tried to convert a Windows file path with a drive to a URL path."
+  );
+  return path.replace(/\\/g, "/") as UrlPath;
+}