Skip to content

Commit

Permalink
fix(wrangler): Validate routes for Workers with assets
Browse files Browse the repository at this point in the history 
We want wrangler to error if users are trying to deploy
a Worker with assets, and routes with a path component.

All Workers with assets must have either:

- custom domain routes
- pattern routes which have no path component (except
for the wildcard splat) "some.domain.com/\*"
  • Loading branch information
@emily-shen @CarmenPopoviciu
emily-shen authored and CarmenPopoviciu committed Sep 11, 2024
1 parent 81c40f0 commit 8ee1fb1
Show file tree
Hide file tree
Showing 10 changed files with 300 additions and 39 deletions.
12 changes: 12 additions & 0 deletions .changeset/lazy-poems-mate.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"wrangler": patch
---

fix: Validate `routes` in `wrangler dev` and `wrangler deploy` for Workers with assets

We want wrangler to error if users are trying to deploy a Worker with assets, and routes with a path component.

All Workers with assets must have either:

- custom domain routes
- pattern routes which have no path component (except for the wildcard splat) "some.domain.com/\*"
15 changes: 15 additions & 0 deletions packages/workers-shared/asset-worker/wrangler.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,18 @@ account_id = "0f1b8aa119a907021f659042f95ea9ba"
workers_dev = false
main = "src/index.ts"
compatibility_date = "2024-07-31"
compatibility_flags = ["nodejs_compat"]

[[unsafe.bindings]]
name = "ASSETS_MANIFEST"
type = "param"
param = "assetManifest"
data_ref = true

[[unsafe.bindings]]
name = "ASSETS_KV_NAMESPACE"
type = "internal_assets"

[unsafe.metadata.build_options]
stable_id = "cloudflare/cf_asset_worker"
networks = ["cf","jdc"]
13 changes: 13 additions & 0 deletions packages/workers-shared/router-worker/wrangler.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,16 @@
name = "router-worker"
main = "src/index.ts"
compatibility_date = "2024-07-31"

[[unsafe.bindings]]
name = "ASSET_WORKER"
type = "internal_assets"
fetcherApi = "fetcher"

[[unsafe.bindings]]
name = "USER_WORKER"
type = "origin"

[unsafe.metadata.build_options]
stable_id = "cloudflare/cf_router_worker"
networks = ["cf","jdc"]
111 changes: 101 additions & 10 deletions packages/wrangler/src/__tests__/deploy.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1455,11 +1455,12 @@ Update them to point to this script instead?`,
routes: [{ pattern: "*.example.com", custom_domain: true }],
});
writeWorkerSource();
await expect(
runWrangler("deploy ./index")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Cannot use "*.example.com" as a Custom Domain; wildcard operators (*) are not allowed]`
);
await expect(runWrangler("deploy ./index")).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
*.example.com:
Wildcard operators (*) are not allowed in Custom Domains]
`);

writeWranglerToml({
routes: [
Expand All @@ -1469,11 +1470,12 @@ Update them to point to this script instead?`,
writeWorkerSource();
mockServiceScriptData({});

await expect(
runWrangler("deploy ./index")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Cannot use "api.example.com/at/a/path" as a Custom Domain; paths are not allowed]`
);
await expect(runWrangler("deploy ./index")).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
api.example.com/at/a/path:
Paths are not allowed in Custom Domains]
`);
});

it("should not continue with publishing an override if user does not confirm", async () => {
Expand Down Expand Up @@ -1516,6 +1518,95 @@ Update them to point to this script instead?`,
});
});

it("should error on routes with paths if experimental assets are present", async () => {
writeWranglerToml({
routes: [
"simple.co.uk/path",
"simple.co.uk/path/*",
"simple.co.uk/",
"simple.co.uk/*",
"simple.co.uk",
{ pattern: "route.co.uk/path", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/path/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/", zone_id: "asdfadsf" },
{ pattern: "route.co.uk", zone_id: "asdfadsf" },
{ pattern: "custom.co.uk/path", custom_domain: true },
{ pattern: "custom.co.uk/*", custom_domain: true },
{ pattern: "custom.co.uk", custom_domain: true },
],
});
writeWorkerSource();
writeAssets([{ filePath: "asset.txt", content: "Content of file-1" }]);

await expect(runWrangler(`deploy --experimental-assets="assets"`)).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
simple.co.uk/path:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path with /*
simple.co.uk/path/*:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path/* with /*
simple.co.uk/:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
simple.co.uk:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
route.co.uk/path:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path with /*
route.co.uk/path/*:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path/* with /*
route.co.uk/:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
route.co.uk:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
custom.co.uk/path:
Paths are not allowed in Custom Domains
custom.co.uk/*:
Wildcard operators (*) are not allowed in Custom Domains
Paths are not allowed in Custom Domains]
`);
});

it("shouldn't error on routes with paths if there are no experimental assets", async () => {
writeWranglerToml({
routes: [
"simple.co.uk/path",
"simple.co.uk/path/*",
"simple.co.uk/",
"simple.co.uk/*",
"simple.co.uk",
{ pattern: "route.co.uk/path", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/path/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/", zone_id: "asdfadsf" },
{ pattern: "route.co.uk", zone_id: "asdfadsf" },
{ pattern: "custom.co.uk/path", custom_domain: true },
{ pattern: "custom.co.uk/*", custom_domain: true },
{ pattern: "custom.co.uk", custom_domain: true },
],
});
writeWorkerSource();

await expect(runWrangler(`deploy ./index`)).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
custom.co.uk/path:
Paths are not allowed in Custom Domains
custom.co.uk/*:
Wildcard operators (*) are not allowed in Custom Domains
Paths are not allowed in Custom Domains]
`);
});

it.todo("should error if it's a workers.dev route");
});

Expand Down
84 changes: 84 additions & 0 deletions packages/wrangler/src/__tests__/dev.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,90 @@ describe("wrangler dev", () => {
})
);
});
it("should error if custom domains with paths are passed in but allow paths on normal routes", async () => {
fs.writeFileSync("index.js", `export default {};`);
writeWranglerToml({
main: "index.js",
routes: [
"simple.co.uk/path",
"simple.co.uk/*",
"simple.co.uk",
{ pattern: "route.co.uk/path", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk", zone_id: "asdfadsf" },
{ pattern: "custom.co.uk/path", custom_domain: true },
{ pattern: "custom.co.uk/*", custom_domain: true },
{ pattern: "custom.co.uk", custom_domain: true },
],
});
await expect(runWrangler(`dev`)).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
custom.co.uk/path:
Paths are not allowed in Custom Domains
custom.co.uk/*:
Wildcard operators (*) are not allowed in Custom Domains
Paths are not allowed in Custom Domains]
`);
});
it("should error on routes with paths if experimental assets are present", async () => {
writeWranglerToml({
routes: [
"simple.co.uk/path",
"simple.co.uk/path/*",
"simple.co.uk/",
"simple.co.uk/*",
"simple.co.uk",
{ pattern: "route.co.uk/path", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/path/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/", zone_id: "asdfadsf" },
{ pattern: "route.co.uk", zone_id: "asdfadsf" },
{ pattern: "custom.co.uk/path", custom_domain: true },
{ pattern: "custom.co.uk/*", custom_domain: true },
{ pattern: "custom.co.uk", custom_domain: true },
],
experimental_assets: {
directory: "assets",
},
});
fs.openSync("assets", "w");
await expect(runWrangler(`dev`)).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
simple.co.uk/path:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path with /*
simple.co.uk/path/*:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path/* with /*
simple.co.uk/:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
simple.co.uk:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
route.co.uk/path:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path with /*
route.co.uk/path/*:
Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /path/* with /*
route.co.uk/:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
route.co.uk:
Workers which have static assets must end with a wildcard path. Update the route to end with /*
custom.co.uk/path:
Paths are not allowed in Custom Domains
custom.co.uk/*:
Wildcard operators (*) are not allowed in Custom Domains
Paths are not allowed in Custom Domains]
`);
});
});

describe("host", () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/wrangler/src/api/startDevWorker/ConfigController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ async function resolveDevConfig(
routes: input.triggers?.filter(
(t): t is Extract<Trigger, { type: "route" }> => t.type === "route"
),
experimentalAssets: input.experimental?.assets?.directory,
},
config
);
Expand Down Expand Up @@ -163,6 +164,7 @@ async function resolveTriggers(
routes: input.triggers?.filter(
(t): t is Extract<Trigger, { type: "route" }> => t.type === "route"
),
experimentalAssets: input.experimental?.assets?.directory,
},
config
);
Expand Down
67 changes: 53 additions & 14 deletions packages/wrangler/src/deploy/deploy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,58 @@ function errIsStartupErr(err: unknown): err is ParseError & { code: 10021 } {
return false;
}

export const validateRoutes = (
routes: Route[],
hasExperimentalAssets: boolean
) => {
const invalidRoutes: Record<string, string[]> = {};
for (const route of routes) {
if (typeof route !== "string" && route.custom_domain) {
if (route.pattern.includes("*")) {
invalidRoutes[route.pattern] ??= [];
invalidRoutes[route.pattern].push(
`Wildcard operators (*) are not allowed in Custom Domains`
);
}
if (route.pattern.includes("/")) {
invalidRoutes[route.pattern] ??= [];
invalidRoutes[route.pattern].push(
`Paths are not allowed in Custom Domains`
);
}
} else if (hasExperimentalAssets) {
const pattern = typeof route === "string" ? route : route.pattern;
const components = pattern.split("/");

if (
// = ["route.com"] bare domains are invalid as it would only match exactly that
components.length === 1 ||
// = ["route.com",""] as above
(components.length === 2 && components[1] === "")
) {
invalidRoutes[pattern] ??= [];
invalidRoutes[pattern].push(
`Workers which have static assets must end with a wildcard path. Update the route to end with /*`
);
// ie it doesn't match exactly "route.com/*" = [route.com, *]
} else if (!(components.length === 2 && components[1] === "*")) {
invalidRoutes[pattern] ??= [];
invalidRoutes[pattern].push(
`Workers which have static assets cannot be routed on a URL which has a path component. Update the route to replace /${components.slice(1).join("/")} with /*`
);
}
}
}
if (Object.keys(invalidRoutes).length > 0) {
throw new UserError(
`Invalid Routes:\n` +
Object.entries(invalidRoutes)
.map(([route, errors]) => `${route}:\n` + errors.join("\n"))
.join(`\n\n`)
);
}
};

export function renderRoute(route: Route): string {
let result = "";
if (typeof route === "string") {
Expand Down Expand Up @@ -383,20 +435,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m

const routes =
props.routes ?? config.routes ?? (config.route ? [config.route] : []) ?? [];
for (const route of routes) {
if (typeof route !== "string" && route.custom_domain) {
if (route.pattern.includes("*")) {
throw new UserError(
`Cannot use "${route.pattern}" as a Custom Domain; wildcard operators (*) are not allowed`
);
}
if (route.pattern.includes("/")) {
throw new UserError(
`Cannot use "${route.pattern}" as a Custom Domain; paths are not allowed`
);
}
}
}
validateRoutes(routes, Boolean(props.experimentalAssetsOptions));

const jsxFactory = props.jsxFactory || config.jsx_factory;
const jsxFragment = props.jsxFragment || config.jsx_fragment;
Expand Down
Loading

0 comments on commit 8ee1fb1

Please sign in to comment.