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

Disallow unknown exports #7878

Merged
merged 11 commits into from
Dec 8, 2022
5 changes: 5 additions & 0 deletions .changeset/friendly-jobs-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] disallow unknown exports from `+(layout|page)(.server)?.js` and `+server.js` files
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
dummdidumm marked this conversation as resolved.
Show resolved Hide resolved
63 changes: 40 additions & 23 deletions packages/kit/src/core/prerender/prerender.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ import { logger } from '../utils.js';
import { load_config } from '../config/index.js';
import { get_route_segments } from '../../utils/routing.js';
import { get_option } from '../../runtime/server/utils.js';
import {
validate_common_exports,
validate_page_server_exports,
validate_server_exports
} from '../../utils/exports.js';

const [, , client_out_dir, manifest_path, results_path, verbose, env] = process.argv;

Expand Down Expand Up @@ -357,34 +362,46 @@ export async function prerender() {
}

for (const route of manifest._.routes) {
try {
if (route.endpoint) {
const mod = await route.endpoint();
if (mod.prerender !== undefined) {
if (mod.prerender && (mod.POST || mod.PATCH || mod.PUT || mod.DELETE)) {
throw new Error(
`Cannot prerender a +server file with POST, PATCH, PUT, or DELETE (${route.id})`
);
}

prerender_map.set(route.id, mod.prerender);
if (route.endpoint) {
const mod = await route.endpoint();
if (mod.prerender !== undefined) {
validate_server_exports(mod, route.id);

if (mod.prerender && (mod.POST || mod.PATCH || mod.PUT || mod.DELETE)) {
throw new Error(
`Cannot prerender a +server file with POST, PATCH, PUT, or DELETE (${route.id})`
);
}

prerender_map.set(route.id, mod.prerender);
}
}

if (route.page) {
const nodes = await Promise.all(
[...route.page.layouts, route.page.leaf].map((n) => {
if (n !== undefined) return manifest._.nodes[n]();
})
);

const layouts = nodes.slice(0, -1);
const page = nodes.at(-1);

if (route.page) {
const nodes = await Promise.all(
[...route.page.layouts, route.page.leaf].map((n) => {
if (n !== undefined) return manifest._.nodes[n]();
})
);
const prerender = get_option(nodes, 'prerender') ?? false;
for (const layout of layouts) {
if (layout) {
validate_common_exports(layout.server, route.id);
validate_common_exports(layout.shared, route.id);
}
}

prerender_map.set(route.id, prerender);
if (page) {
validate_page_server_exports(page.server, route.id);
validate_common_exports(page.shared, route.id);
}
} catch (e) {
// We failed to import the module, which indicates it can't be prerendered
// TODO should we catch these? It's almost certainly a bug in the app
console.error(e);

const prerender = get_option(nodes, 'prerender') ?? false;

prerender_map.set(route.id, prerender);
}
}

Expand Down
7 changes: 6 additions & 1 deletion packages/kit/src/exports/vite/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ export async function dev(vite, vite_config, svelte_config) {
// @ts-expect-error
globalThis.__SVELTEKIT_BROWSER__ = false;

// @ts-expect-error
globalThis.__SVELTEKIT_DEV__ = true;

sync.init(svelte_config, vite_config.mode);

/** @type {import('types').Respond} */
Expand Down Expand Up @@ -102,6 +105,7 @@ export async function dev(vite, vite_config, svelte_config) {
module_nodes.push(module_node);

result.shared = module;
result.shared_id = node.shared;
}

if (node.server) {
Expand Down Expand Up @@ -163,7 +167,8 @@ export async function dev(vite, vite_config, svelte_config) {
const url = path.resolve(cwd, endpoint.file);
return await vite.ssrLoadModule(url);
}
: null
: null,
endpoint_id: endpoint?.file
};
})
),
Expand Down
5 changes: 5 additions & 0 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { stores } from './singletons.js';
import { unwrap_promises } from '../../utils/promises.js';
import * as devalue from 'devalue';
import { INDEX_KEY, PRELOAD_PRIORITIES, SCROLL_KEY } from './constants.js';
import { validate_common_exports } from '../../utils/exports.js';

const routes = parse(nodes, server_loads, dictionary, matchers);

Expand Down Expand Up @@ -551,6 +552,10 @@ export function create_client({ target, base }) {

const node = await loader();

if (__SVELTEKIT_DEV__) {
validate_common_exports(node.shared);
}

if (node.shared?.load) {
/** @param {string[]} deps */
function depends(...deps) {
Expand Down
26 changes: 26 additions & 0 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ import { INVALIDATED_HEADER, redirect_json_response, render_data } from './data/
import { add_cookies_to_headers, get_cookies } from './cookie.js';
import { create_fetch } from './fetch.js';
import { Redirect } from '../control.js';
import {
validate_common_exports,
validate_page_server_exports,
validate_server_exports
} from '../../utils/exports.js';

/* global __SVELTEKIT_ADAPTER_NAME__ */

Expand Down Expand Up @@ -185,10 +190,31 @@ export async function respond(request, options, state) {
options.manifest._.nodes[route.page.leaf]()
]);

if (__SVELTEKIT_DEV__) {
const layouts = nodes.slice(0, -1);
const page = nodes.at(-1);

for (const layout of layouts) {
if (layout) {
validate_common_exports(layout.server, /** @type {string} */ (layout.server_id));
validate_common_exports(layout.shared, /** @type {string} */ (layout.shared_id));
}
}

if (page) {
validate_page_server_exports(page.server, /** @type {string} */ (page.server_id));
validate_common_exports(page.shared, /** @type {string} */ (page.shared_id));
}
}

trailing_slash = get_option(nodes, 'trailingSlash');
} else if (route.endpoint) {
const node = await route.endpoint();
trailing_slash = node.trailingSlash;

if (__SVELTEKIT_DEV__) {
validate_server_exports(node, /** @type {string} */ (route.endpoint_id));
}
}

const normalized = normalize_path(url.pathname, trailing_slash ?? 'never');
Expand Down
54 changes: 54 additions & 0 deletions packages/kit/src/utils/exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* @param {string[]} expected
*/
function validator(expected) {
const set = new Set(expected);

/**
* @param {any} module
* @param {string} [route_id]
*/
function validate(module, route_id) {
if (!module) return;

for (const key in module) {
if (key[0] !== '_' && !set.has(key)) {
const valid = expected.join(', ');
throw new Error(
`Invalid export '${key}'${
route_id ? ` in ${route_id}` : ''
} (valid exports are ${valid}, or anything with a '_' prefix)`
);
}
}
}

return validate;
}

export const validate_common_exports = validator([
'load',
'prerender',
'csr',
'ssr',
'trailingSlash'
]);

export const validate_page_server_exports = validator([
'load',
'prerender',
'csr',
'ssr',
'actions',
'trailingSlash'
]);

export const validate_server_exports = validator([
'GET',
'POST',
'PATCH',
'PUT',
'DELETE',
'prerender',
'trailingSlash'
]);
58 changes: 58 additions & 0 deletions packages/kit/src/utils/exports.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { test } from 'uvu';
import * as assert from 'uvu/assert';
import {
validate_common_exports,
validate_page_server_exports,
validate_server_exports
} from './exports.js';

test('validates +layout.server.js, +layout.js, +page.js', () => {
validate_common_exports({
load: () => {}
});

validate_common_exports({
_unknown: () => {}
});

assert.throws(() => {
validate_common_exports({
actions: {}
});
}, /Invalid export 'actions' \(valid exports are load, prerender, csr, ssr, trailingSlash\)/);
});

test('validates +page.server.js', () => {
validate_page_server_exports({
load: () => {},
actions: {}
});

validate_page_server_exports({
_unknown: () => {}
});

assert.throws(() => {
validate_page_server_exports({
answer: 42
});
}, /Invalid export 'answer' \(valid exports are load, prerender, csr, ssr, actions, trailingSlash\)/);
});

test('validates +server.js', () => {
validate_server_exports({
GET: () => {}
});

validate_server_exports({
_unknown: () => {}
});

assert.throws(() => {
validate_server_exports({
answer: 42
});
}, /Invalid export 'answer' \(valid exports are GET, POST, PATCH, PUT, DELETE, prerender, trailingSlash\)/);
});

test.run();
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { building } from '$app/environment';
import { json } from '@sveltejs/kit';

// @ts-expect-error
thisvariableisnotdefined;
if (!building) {
// @ts-expect-error
thisvariableisnotdefined;
}

export function GET() {
return json({
Expand Down
2 changes: 2 additions & 0 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ export interface SSRNode {

// store this in dev so we can print serialization errors
server_id?: string;
shared_id?: string;
}

export type SSRNodeLoader = () => Promise<SSRNode>;
Expand Down Expand Up @@ -346,6 +347,7 @@ export interface SSRRoute {
params: RouteParam[];
page: PageNodeIndexes | null;
endpoint: (() => Promise<SSREndpoint>) | null;
endpoint_id?: string;
}

export interface SSRState {
Expand Down