From f30352f874790b9de0bd0eba985a21aef23e158e Mon Sep 17 00:00:00 2001 From: James Glenn <47917431+JR-G@users.noreply.github.com> Date: Wed, 29 Jan 2025 21:51:13 +0000 Subject: [PATCH] feat: validate values for `cache-control` and `content-type` headers in dev mode (#13114) * Add header validator * Validate headers * Test route for the invalid headers * changeset * Capture all IANA top level content types * chore: Slight improvements before merge * ugh lint --------- Co-authored-by: S. Elliott Johnson --- .changeset/rich-pants-beam.md | 5 + packages/kit/src/runtime/server/respond.js | 6 ++ .../src/runtime/server/validate-headers.js | 64 ++++++++++++ .../runtime/server/validate-headers.spec.js | 99 +++++++++++++++++++ .../src/routes/headers/invalid/+server.js | 9 ++ 5 files changed, 183 insertions(+) create mode 100644 .changeset/rich-pants-beam.md create mode 100644 packages/kit/src/runtime/server/validate-headers.js create mode 100644 packages/kit/src/runtime/server/validate-headers.spec.js create mode 100644 packages/kit/test/apps/dev-only/src/routes/headers/invalid/+server.js diff --git a/.changeset/rich-pants-beam.md b/.changeset/rich-pants-beam.md new file mode 100644 index 000000000000..bc87729ebebd --- /dev/null +++ b/.changeset/rich-pants-beam.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': minor +--- + +feat: validate values for `cache-control` and `content-type` headers in dev mode diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 5f5ae7b795e4..d95ba3514166 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -33,8 +33,10 @@ import { INVALIDATED_PARAM, TRAILING_SLASH_PARAM } from '../shared.js'; import { get_public_env } from './env_module.js'; import { load_page_nodes } from './page/load_page_nodes.js'; import { get_page_config } from '../../utils/route_config.js'; +import { validateHeaders } from './validate-headers.js'; /* global __SVELTEKIT_ADAPTER_NAME__ */ +/* global __SVELTEKIT_DEV__ */ /** @type {import('types').RequiredResolveOptions['transformPageChunk']} */ const default_transform = ({ html }) => html; @@ -186,6 +188,10 @@ export async function respond(request, options, manifest, state) { request, route: { id: route?.id ?? null }, setHeaders: (new_headers) => { + if (__SVELTEKIT_DEV__) { + validateHeaders(new_headers); + } + for (const key in new_headers) { const lower = key.toLowerCase(); const value = new_headers[key]; diff --git a/packages/kit/src/runtime/server/validate-headers.js b/packages/kit/src/runtime/server/validate-headers.js new file mode 100644 index 000000000000..6da978574108 --- /dev/null +++ b/packages/kit/src/runtime/server/validate-headers.js @@ -0,0 +1,64 @@ +/** @type {Set} */ +const VALID_CACHE_CONTROL_DIRECTIVES = new Set([ + 'max-age', + 'public', + 'private', + 'no-cache', + 'no-store', + 'must-revalidate', + 'proxy-revalidate', + 's-maxage', + 'immutable', + 'stale-while-revalidate', + 'stale-if-error', + 'no-transform', + 'only-if-cached', + 'max-stale', + 'min-fresh' +]); + +const CONTENT_TYPE_PATTERN = + /^(application|audio|example|font|haptics|image|message|model|multipart|text|video|x-[a-z]+)\/[-+.\w]+$/i; + +/** @type {Record void>} */ +const HEADER_VALIDATORS = { + 'cache-control': (value) => { + const error_suffix = `(While parsing "${value}".)`; + const parts = value.split(',').map((part) => part.trim()); + if (parts.some((part) => !part)) { + throw new Error(`\`cache-control\` header contains empty directives. ${error_suffix}`); + } + + const directives = parts.map((part) => part.split('=')[0].toLowerCase()); + const invalid = directives.find((directive) => !VALID_CACHE_CONTROL_DIRECTIVES.has(directive)); + if (invalid) { + throw new Error( + `Invalid cache-control directive "${invalid}". Did you mean one of: ${[...VALID_CACHE_CONTROL_DIRECTIVES].join(', ')}? ${error_suffix}` + ); + } + }, + + 'content-type': (value) => { + const type = value.split(';')[0].trim(); + const error_suffix = `(While parsing "${value}".)`; + if (!CONTENT_TYPE_PATTERN.test(type)) { + throw new Error(`Invalid content-type value "${type}". ${error_suffix}`); + } + } +}; + +/** + * @param {Record} headers + */ +export function validateHeaders(headers) { + for (const [key, value] of Object.entries(headers)) { + const validator = HEADER_VALIDATORS[key.toLowerCase()]; + try { + validator?.(value); + } catch (error) { + if (error instanceof Error) { + console.warn(`[SvelteKit] ${error.message}`); + } + } + } +} diff --git a/packages/kit/src/runtime/server/validate-headers.spec.js b/packages/kit/src/runtime/server/validate-headers.spec.js new file mode 100644 index 000000000000..0a043314a0c5 --- /dev/null +++ b/packages/kit/src/runtime/server/validate-headers.spec.js @@ -0,0 +1,99 @@ +import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { validateHeaders } from './validate-headers.js'; + +describe('validateHeaders', () => { + const console_warn_spy = vi.spyOn(console, 'warn'); + + beforeEach(() => { + vi.resetAllMocks(); + }); + + describe('cache-control header', () => { + test('accepts valid directives', () => { + validateHeaders({ 'cache-control': 'public, max-age=3600' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); + + test('rejects invalid directives', () => { + validateHeaders({ 'cache-control': 'public, maxage=3600' }); + expect(console_warn_spy).toHaveBeenCalledWith( + expect.stringContaining('Invalid cache-control directive "maxage"') + ); + }); + + test('rejects empty directives', () => { + validateHeaders({ 'cache-control': 'public,, max-age=3600' }); + expect(console_warn_spy).toHaveBeenCalledWith( + expect.stringContaining('`cache-control` header contains empty directives') + ); + + validateHeaders({ 'cache-control': 'public, , max-age=3600' }); + expect(console_warn_spy).toHaveBeenCalledWith( + expect.stringContaining('`cache-control` header contains empty directives') + ); + }); + + test('accepts multiple cache-control values', () => { + validateHeaders({ 'cache-control': 'max-age=3600, s-maxage=7200' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); + }); + + describe('content-type header', () => { + test('accepts standard content types', () => { + validateHeaders({ 'content-type': 'application/json' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); + + test('accepts content types with parameters', () => { + validateHeaders({ 'content-type': 'text/html; charset=utf-8' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + + validateHeaders({ 'content-type': 'application/javascript; charset=utf-8' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); + + test('accepts vendor-specific content types', () => { + validateHeaders({ 'content-type': 'x-custom/whatever' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); + + test('rejects malformed content types', () => { + validateHeaders({ 'content-type': 'invalid-content-type' }); + expect(console_warn_spy).toHaveBeenCalledWith( + expect.stringContaining('Invalid content-type value "invalid-content-type"') + ); + }); + + test('rejects invalid content type categories', () => { + validateHeaders({ 'content-type': 'invalid/type; invalid=param' }); + expect(console_warn_spy).toHaveBeenCalledWith( + expect.stringContaining('Invalid content-type value "invalid/type"') + ); + + validateHeaders({ 'content-type': 'bad/type; charset=utf-8' }); + expect(console_warn_spy).toHaveBeenCalledWith( + expect.stringContaining('Invalid content-type value "bad/type"') + ); + }); + + test('handles case-insensitive content-types', () => { + validateHeaders({ 'content-type': 'TEXT/HTML; charset=utf-8' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); + }); + + test('allows unknown headers', () => { + validateHeaders({ 'x-custom-header': 'some-value' }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); + + test('handles multiple headers simultaneously', () => { + validateHeaders({ + 'cache-control': 'max-age=3600', + 'content-type': 'text/html', + 'x-custom': 'value' + }); + expect(console_warn_spy).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/kit/test/apps/dev-only/src/routes/headers/invalid/+server.js b/packages/kit/test/apps/dev-only/src/routes/headers/invalid/+server.js new file mode 100644 index 000000000000..91b98c75e0c3 --- /dev/null +++ b/packages/kit/test/apps/dev-only/src/routes/headers/invalid/+server.js @@ -0,0 +1,9 @@ +/** @type {import("@sveltejs/kit").RequestHandler} */ +export function GET({ setHeaders }) { + setHeaders({ + 'cache-control': 'totally-invalid', + 'content-type': 'not-a-real-type' + }); + + return new Response('Testing invalid headers'); +}