Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Validate Vercel cron paths #9145

Merged
merged 15 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tiny-deers-complain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/adapter-vercel': minor
---

feat: validate that Vercel cron paths match an API path
59 changes: 59 additions & 0 deletions packages/adapter-vercel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ const plugin = function (defaults = {}) {
builder.rimraf(dir);
builder.rimraf(tmp);

if (fs.existsSync('vercel.json')) {
const vercel_file = fs.readFileSync('vercel.json', 'utf-8');
const vercel_config = JSON.parse(vercel_file);
validate_vercel_json(builder, vercel_config);
}

const files = fileURLToPath(new URL('./files', import.meta.url).href);

const dirs = {
Expand Down Expand Up @@ -497,4 +503,57 @@ async function create_function_bundle(builder, entry, dir, config) {
write(`${dir}/package.json`, JSON.stringify({ type: 'module' }));
}

/**
*
* @param {import('@sveltejs/kit').Builder} builder
* @param {any} vercel_config
*/
function validate_vercel_json(builder, vercel_config) {
if (builder.routes.length > 0 && !builder.routes[0].api) {
// bail — we're on an older SvelteKit version that doesn't
// populate `route.api.methods`, so we can't check
// to see if cron paths are valid
return;
}

const crons = /** @type {Array<unknown>} */ (
Array.isArray(vercel_config?.crons) ? vercel_config.crons : []
);

/** For a route to be considered 'valid', it must be an API route with a GET handler */
const valid_routes = builder.routes.filter((route) => route.api.methods.includes('GET'));

/** @type {Array<string>} */
const unmatched_paths = [];

for (const cron of crons) {
if (typeof cron !== 'object' || cron === null || !('path' in cron)) {
continue;
}

const { path } = cron;
if (typeof path !== 'string') {
continue;
}

for (const route of valid_routes) {
if (route.pattern.test(path)) {
continue;
}
}

unmatched_paths.push(path);
}

builder.log.warn(
`\nvercel.json defines cron tasks that use paths that do not correspond to an API route with a GET handler (ignore this if the request is handled in your \`handle\` hook):`
);

for (const path of unmatched_paths) {
console.log(` - ${path}`);
}

console.log('');
}

export default plugin;
9 changes: 5 additions & 4 deletions packages/kit/src/core/adapt/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ export function create_builder({
* we expose a stable type that adapters can use to group/filter routes
*/
const routes = route_data.map((route) => {
const methods =
/** @type {import('types').HttpMethod[]} */
(server_metadata.routes.get(route.id)?.methods);
const config = server_metadata.routes.get(route.id)?.config;
const { config, methods, page, api } = /** @type {import('types').ServerMetadataRoute} */ (
server_metadata.routes.get(route.id)
);

/** @type {import('types').RouteDefinition} */
const facade = {
id: route.id,
api,
page,
segments: get_route_segments(route.id).map((segment) => ({
dynamic: segment.includes('['),
rest: segment.includes('[...'),
Expand Down
33 changes: 21 additions & 12 deletions packages/kit/src/core/postbuild/analyse.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,11 @@ async function analyse({ manifest_path, env }) {

// analyse routes
for (const route of manifest._.routes) {
/** @type {Set<import('types').HttpMethod>} */
const methods = new Set();
/** @type {Array<'GET' | 'POST'>} */
const page_methods = [];

/** @type {import('types').HttpMethod[]} */
const api_methods = [];

/** @type {import('types').PrerenderOption | undefined} */
let prerender = undefined;
Expand All @@ -84,12 +87,12 @@ async function analyse({ manifest_path, env }) {
prerender = mod.prerender;
}

if (mod.GET) methods.add('GET');
if (mod.POST) methods.add('POST');
if (mod.PUT) methods.add('PUT');
if (mod.PATCH) methods.add('PATCH');
if (mod.DELETE) methods.add('DELETE');
if (mod.OPTIONS) methods.add('OPTIONS');
if (mod.GET) api_methods.push('GET');
if (mod.POST) api_methods.push('POST');
if (mod.PUT) api_methods.push('PUT');
if (mod.PATCH) api_methods.push('PATCH');
if (mod.DELETE) api_methods.push('DELETE');
if (mod.OPTIONS) api_methods.push('OPTIONS');

config = mod.config;
}
Expand All @@ -112,8 +115,8 @@ async function analyse({ manifest_path, env }) {
}

if (page) {
methods.add('GET');
if (page.server?.actions) methods.add('POST');
page_methods.push('GET');
if (page.server?.actions) page_methods.push('POST');

validate_page_server_exports(page.server, page.server_id);
validate_common_exports(page.universal, page.universal_id);
Expand All @@ -133,9 +136,15 @@ async function analyse({ manifest_path, env }) {
}

metadata.routes.set(route.id, {
prerender,
config,
methods: Array.from(methods)
methods: Array.from(new Set([...page_methods, ...api_methods])),
page: {
methods: page_methods
},
api: {
methods: api_methods
},
prerender
});
}

Expand Down
10 changes: 8 additions & 2 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ export interface RequestEvent<
*/
locals: App.Locals;
/**
* The parameters of the current page or endpoint - e.g. for a route like `/blog/[slug]`, a `{ slug: string }` object
* The parameters of the current route - e.g. for a route like `/blog/[slug]`, a `{ slug: string }` object
*/
params: Params;
/**
Expand Down Expand Up @@ -936,7 +936,7 @@ export interface RequestEvent<
*/
setHeaders(headers: Record<string, string>): void;
/**
* The URL of the current page or endpoint.
* The requested URL.
*/
url: URL;
/**
Expand Down Expand Up @@ -983,6 +983,12 @@ export interface ResolveOptions {

export interface RouteDefinition<Config = any> {
id: string;
api: {
methods: HttpMethod[];
};
page: {
methods: Extract<HttpMethod, 'GET' | 'POST'>[];
};
pattern: RegExp;
prerender: PrerenderOption;
segments: RouteSegment[];
Expand Down
21 changes: 13 additions & 8 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,16 +263,21 @@ export interface ServerErrorNode {
status?: number;
}

export interface ServerMetadataRoute {
config: any;
api: {
methods: HttpMethod[];
};
page: {
methods: Array<'GET' | 'POST'>;
};
methods: HttpMethod[];
prerender: PrerenderOption | undefined;
}

export interface ServerMetadata {
nodes: Array<{ has_server_load: boolean }>;
routes: Map<
string,
{
prerender: PrerenderOption | undefined;
methods: HttpMethod[];
config: any;
}
>;
routes: Map<string, ServerMetadataRoute>;
}

export interface SSRComponent {
Expand Down