fix: address basePath review feedback

1. Make basePath a module-level variable (like BUILD_ID) instead of
   threading it through asset()/assetSrcSet() parameters. The public
   API no longer exposes basePath — it's set once via setBasePath()
   during app.handler() initialization.

2. Guard applyBasePath against double-prefixing — if the path already
   starts with basePath, return it unchanged.

3. Improve basePath validation — use URL round-trip check instead of
   an incomplete character regex, catching all invalid path characters.

4. Island CSS paths confirmed safe — they are root-relative from the
   build system and never include basePath, so applyBasePath (now via
   asset()) is correct.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: bartlomieju

packages/fresh/src/app.ts

@@ -2,6 +2,7 @@ import { trace } from "@opentelemetry/api";
 
 import { DENO_DEPLOYMENT_ID } from "@fresh/build-id";
 import * as colors from "@std/fmt/colors";
+import { setBasePath } from "./runtime/shared.ts";
 import {
   type MaybeLazyMiddleware,
   type Middleware,
@@ -218,10 +219,17 @@ export class App<State> {
       );
     }
 
-    const hasInvalidChars = /[@#?&=\s]/.test(basePath);
-    if (hasInvalidChars) {
+    // Validate by round-tripping through URL — catches all invalid path chars
+    try {
+      const url = new URL(basePath, "https://localhost");
+      if (url.pathname !== basePath) {
+        throw new Error(
+          `Invalid basePath: "${basePath}". Contains characters that require encoding`,
+        );
+      }
+    } catch {
       throw new Error(
-        `Invalid basePath: "${basePath}". Contains invalid characters`,
+        `Invalid basePath: "${basePath}". Must be a valid URL path segment`,
       );
     }
   }
@@ -422,6 +430,8 @@ export class App<State> {
       }
     }
 
+    setBasePath(this.config.basePath);
+
     const router = new UrlPatternRouter<MaybeLazyMiddleware<State>>();
 
     const { rootMiddlewares } = applyCommands(

packages/fresh/src/runtime/server/preact_hooks.ts

@@ -16,7 +16,6 @@ import { asset, Partial, type PartialProps } from "../shared.ts";
 import { stringify } from "../../jsonify/stringify.ts";
 import type { Island } from "../../context.ts";
 import {
-  applyBasePath,
   assetHashingHook,
   CLIENT_NAV_ATTR,
   DATA_FRESH_KEY,
@@ -116,8 +115,7 @@ options[OptionsType.VNODE] = (vnode) => {
       setActiveUrl(vnode, RENDER_STATE.ctx.url.pathname);
     }
   }
-  const basePath = RENDER_STATE?.ctx.config.basePath;
-  assetHashingHook(vnode, BUILD_ID, basePath);
+  assetHashingHook(vnode, BUILD_ID, RENDER_STATE?.ctx.config.basePath);
 
   if (typeof vnode.type === "function") {
     if (vnode.type === Partial) {
@@ -295,7 +293,6 @@ options[OptionsType.DIFF] = (vnode) => {
           RENDER_STATE!.renderedHtmlHead = true;
 
           const entryAssets = RENDER_STATE.buildCache.getEntryAssets();
-          const basePath = RENDER_STATE.ctx.config.basePath;
           // deno-lint-ignore no-explicit-any
           const items: VNode<any>[] = [];
           if (entryAssets.length > 0) {
@@ -307,7 +304,7 @@ options[OptionsType.DIFF] = (vnode) => {
                   h(
                     "link",
                     // deno-lint-ignore no-explicit-any
-                    { rel: "stylesheet", href: asset(id, basePath) } as any,
+                    { rel: "stylesheet", href: asset(id) } as any,
                   ),
                 );
               }
@@ -467,7 +464,6 @@ options[OptionsType.DIFFED] = (vnode) => {
 
 function RemainingHead() {
   if (RENDER_STATE !== null) {
-    const basePath = RENDER_STATE.ctx.config.basePath;
     // deno-lint-ignore no-explicit-any
     const items: VNode<any>[] = [];
     if (RENDER_STATE.headComponents.size > 0) {
@@ -477,16 +473,15 @@ function RemainingHead() {
     RENDER_STATE.islands.forEach((island) => {
       if (island.css.length > 0) {
         for (let i = 0; i < island.css.length; i++) {
-          const css = island.css[i];
-          const fullPath = applyBasePath(css, basePath);
-          items.push(h("link", { rel: "stylesheet", href: fullPath }));
+          items.push(
+            h("link", { rel: "stylesheet", href: asset(island.css[i]) }),
+          );
         }
       }
     });
 
     RENDER_STATE.islandAssets.forEach((css) => {
-      const fullPath = applyBasePath(css, basePath);
-      items.push(h("link", { rel: "stylesheet", href: fullPath }));
+      items.push(h("link", { rel: "stylesheet", href: asset(css) }));
     });
 
     if (items.length > 0) {

packages/fresh/src/runtime/shared.ts

@@ -2,6 +2,8 @@ import type { ComponentChildren, VNode } from "preact";
 import { BUILD_ID } from "@fresh/build-id";
 import { assetInternal, assetSrcSetInternal } from "./shared_internal.ts";
 
+let BASE_PATH = "";
+
 export { HttpError } from "../error.ts";
 
 /**
@@ -22,18 +24,23 @@ export { HttpError } from "../error.ts";
  */
 export const IS_BROWSER = typeof document !== "undefined";
 
+/** @internal Set the base path for asset URLs. Called once during app init. */
+export function setBasePath(basePath: string) {
+  BASE_PATH = basePath;
+}
+
 /**
  * Create a "locked" asset path. This differs from a plain path in that it is
  * specific to the current version of the application, and as such can be safely
  * served with a very long cache lifetime (1 year).
  */
-export function asset(path: string, basePath?: string): string {
-  return assetInternal(path, BUILD_ID, basePath);
+export function asset(path: string): string {
+  return assetInternal(path, BUILD_ID, BASE_PATH);
 }
 
 /** Apply the `asset` function to urls in a `srcset` attribute. */
-export function assetSrcSet(srcset: string, basePath?: string): string {
-  return assetSrcSetInternal(srcset, BUILD_ID, basePath);
+export function assetSrcSet(srcset: string): string {
+  return assetSrcSetInternal(srcset, BUILD_ID, BASE_PATH);
 }
 
 export interface PartialProps {

packages/fresh/src/runtime/shared_internal.ts

@@ -176,5 +176,10 @@ export function applyBasePath(path: string, basePath?: string): string {
     return path;
   }
 
+  // Avoid double-prefixing if the path already starts with basePath
+  if (path.startsWith(basePath + "/") || path === basePath) {
+    return path;
+  }
+
   return basePath + path;
 }