From 0b248f81a13cb55cbf83cbc681a37c23daf2711b Mon Sep 17 00:00:00 2001 From: Steven Date: Fri, 24 Feb 2023 16:08:24 -0500 Subject: [PATCH] fix: improve next.config.js validation for `images` prop (#46326) This PR removes the custom validation in favor of json schema validation. Previously Next.js would print warnings on schema errors and only throw when custom validation failed (albeit with a nasty stack trace). ### Before image ### After image --------- --- packages/next/src/server/config-schema.ts | 20 +- packages/next/src/server/config.ts | 212 ++---------------- packages/next/src/telemetry/flush-and-exit.ts | 11 + .../image-optimizer/test/index.test.ts | 103 ++++++++- 4 files changed, 142 insertions(+), 204 deletions(-) create mode 100644 packages/next/src/telemetry/flush-and-exit.ts diff --git a/packages/next/src/server/config-schema.ts b/packages/next/src/server/config-schema.ts index b1d698289945f..4584a68331474 100644 --- a/packages/next/src/server/config-schema.ts +++ b/packages/next/src/server/config-schema.ts @@ -582,6 +582,7 @@ const configSchema = { type: 'string', }, port: { + maxLength: 5, type: 'string', }, protocol: { @@ -590,8 +591,10 @@ const configSchema = { type: 'string', }, }, + required: ['hostname'] as any, type: 'object', }, + maxItems: 50, type: 'array', }, unoptimized: { @@ -610,9 +613,12 @@ const configSchema = { }, deviceSizes: { items: { - type: 'number', + type: 'integer', + minimum: 1, + maximum: 10000, }, minItems: 1, + maxItems: 25, type: 'array', }, disableStaticImages: { @@ -622,6 +628,7 @@ const configSchema = { items: { type: 'string', }, + maxItems: 50, type: 'array', }, formats: { @@ -629,13 +636,17 @@ const configSchema = { enum: ['image/avif', 'image/webp'], // automatic typing does not like enum type: 'string', } as any, + maxItems: 4, type: 'array', }, imageSizes: { items: { - type: 'number', + type: 'integer', + minimum: 1, + maximum: 10000, }, - minItems: 1, + minItems: 0, + maxItems: 25, type: 'array', }, loader: { @@ -648,7 +659,8 @@ const configSchema = { type: 'string', }, minimumCacheTTL: { - type: 'number', + type: 'integer', + minimum: 0, }, path: { minLength: 1, diff --git a/packages/next/src/server/config.ts b/packages/next/src/server/config.ts index cac4a8a11991c..fa9b427a09ed9 100644 --- a/packages/next/src/server/config.ts +++ b/packages/next/src/server/config.ts @@ -25,13 +25,10 @@ import { NextConfig, } from './config-shared' import { loadWebpackHook } from './config-utils' -import { - ImageConfig, - imageConfigDefault, - VALID_LOADERS, -} from '../shared/lib/image-config' +import { ImageConfig, imageConfigDefault } from '../shared/lib/image-config' import { loadEnvConfig } from '@next/env' import { gte as semverGte } from 'next/dist/compiled/semver' +import { flushAndExit } from '../telemetry/flush-and-exit' export { DomainLocale, NextConfig, normalizeConfig } from './config-shared' @@ -350,128 +347,12 @@ function assignDefaults( if (config.assetPrefix?.startsWith('http')) { images.domains.push(new URL(config.assetPrefix).hostname) } - - if (images.domains.length > 50) { - throw new Error( - `Specified images.domains exceeds length of 50, received length (${images.domains.length}), please reduce the length of the array to continue.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - const invalid = images.domains.filter( - (d: unknown) => typeof d !== 'string' - ) - if (invalid.length > 0) { - throw new Error( - `Specified images.domains should be an Array of strings received invalid values (${invalid.join( - ', ' - )}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - } - - const remotePatterns = result?.images?.remotePatterns - if (remotePatterns) { - if (!Array.isArray(remotePatterns)) { - throw new Error( - `Specified images.remotePatterns should be an Array received ${typeof remotePatterns}.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - if (remotePatterns.length > 50) { - throw new Error( - `Specified images.remotePatterns exceeds length of 50, received length (${remotePatterns.length}), please reduce the length of the array to continue.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - const validProps = new Set(['protocol', 'hostname', 'pathname', 'port']) - const requiredProps = ['hostname'] - const invalidPatterns = remotePatterns.filter( - (d: unknown) => - !d || - typeof d !== 'object' || - Object.entries(d).some( - ([k, v]) => !validProps.has(k) || typeof v !== 'string' - ) || - requiredProps.some((k) => !(k in d)) - ) - if (invalidPatterns.length > 0) { - throw new Error( - `Invalid images.remotePatterns values:\n${invalidPatterns - .map((item) => JSON.stringify(item)) - .join( - '\n' - )}\n\nremotePatterns value must follow format { protocol: 'https', hostname: 'example.com', port: '', pathname: '/imgs/**' }.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - } - - if (images.deviceSizes) { - const { deviceSizes } = images - if (!Array.isArray(deviceSizes)) { - throw new Error( - `Specified images.deviceSizes should be an Array received ${typeof deviceSizes}.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - if (deviceSizes.length > 25) { - throw new Error( - `Specified images.deviceSizes exceeds length of 25, received length (${deviceSizes.length}), please reduce the length of the array to continue.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - const invalid = deviceSizes.filter((d: unknown) => { - return typeof d !== 'number' || d < 1 || d > 10000 - }) - - if (invalid.length > 0) { - throw new Error( - `Specified images.deviceSizes should be an Array of numbers that are between 1 and 10000, received invalid values (${invalid.join( - ', ' - )}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - } - if (images.imageSizes) { - const { imageSizes } = images - if (!Array.isArray(imageSizes)) { - throw new Error( - `Specified images.imageSizes should be an Array received ${typeof imageSizes}.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - if (imageSizes.length > 25) { - throw new Error( - `Specified images.imageSizes exceeds length of 25, received length (${imageSizes.length}), please reduce the length of the array to continue.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - const invalid = imageSizes.filter((d: unknown) => { - return typeof d !== 'number' || d < 1 || d > 10000 - }) - - if (invalid.length > 0) { - throw new Error( - `Specified images.imageSizes should be an Array of numbers that are between 1 and 10000, received invalid values (${invalid.join( - ', ' - )}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } } if (!images.loader) { images.loader = 'default' } - if (!VALID_LOADERS.includes(images.loader)) { - throw new Error( - `Specified images.loader should be one of (${VALID_LOADERS.join( - ', ' - )}), received invalid value (${ - images.loader - }).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - if ( images.loader !== 'default' && images.loader !== 'custom' && @@ -512,69 +393,6 @@ function assignDefaults( images.loader = 'custom' images.loaderFile = absolutePath } - - if ( - images.minimumCacheTTL && - (!Number.isInteger(images.minimumCacheTTL) || images.minimumCacheTTL < 0) - ) { - throw new Error( - `Specified images.minimumCacheTTL should be an integer 0 or more received (${images.minimumCacheTTL}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - if (images.formats) { - const { formats } = images - if (!Array.isArray(formats)) { - throw new Error( - `Specified images.formats should be an Array received ${typeof formats}.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - if (formats.length < 1 || formats.length > 2) { - throw new Error( - `Specified images.formats must be length 1 or 2, received length (${formats.length}), please reduce the length of the array to continue.\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - const invalid = formats.filter((f) => { - return f !== 'image/avif' && f !== 'image/webp' - }) - - if (invalid.length > 0) { - throw new Error( - `Specified images.formats should be an Array of mime type strings, received invalid values (${invalid.join( - ', ' - )}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - } - - if ( - typeof images.dangerouslyAllowSVG !== 'undefined' && - typeof images.dangerouslyAllowSVG !== 'boolean' - ) { - throw new Error( - `Specified images.dangerouslyAllowSVG should be a boolean received (${images.dangerouslyAllowSVG}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - if ( - typeof images.contentSecurityPolicy !== 'undefined' && - typeof images.contentSecurityPolicy !== 'string' - ) { - throw new Error( - `Specified images.contentSecurityPolicy should be a string received (${images.contentSecurityPolicy}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } - - const unoptimized = result?.images?.unoptimized - if ( - typeof unoptimized !== 'undefined' && - typeof unoptimized !== 'boolean' - ) { - throw new Error( - `Specified images.unoptimized should be a boolean, received (${unoptimized}).\nSee more info here: https://nextjs.org/docs/messages/invalid-images-config` - ) - } } warnOptionHasBeenMovedOutOfExperimental( @@ -945,21 +763,37 @@ export default async function loadConfig( const validateResult = validateConfig(userConfig) if (!silent && validateResult.errors) { - curLog.warn(`Invalid next.config.js options detected: `) - // Only load @segment/ajv-human-errors when invalid config is detected const { AggregateAjvError } = require('next/dist/compiled/@segment/ajv-human-errors') as typeof import('next/dist/compiled/@segment/ajv-human-errors') const aggregatedAjvErrors = new AggregateAjvError(validateResult.errors, { fieldLabels: 'js', }) + + let shouldExit = false + let messages = [`Invalid ${configFileName} options detected: `] + for (const error of aggregatedAjvErrors) { - console.error(` - ${error.message}`) + messages.push(` ${error.message}`) + if (error.message.startsWith('The value at .images.')) { + shouldExit = true + } } - console.error( - '\nSee more info here: https://nextjs.org/docs/messages/invalid-next-config' + messages.push( + 'See more info here: https://nextjs.org/docs/messages/invalid-next-config' ) + + if (shouldExit) { + for (const message of messages) { + curLog.error(message) + } + await flushAndExit(1) + } else { + for (const message of messages) { + curLog.warn(message) + } + } } if (Object.keys(userConfig).length === 0) { diff --git a/packages/next/src/telemetry/flush-and-exit.ts b/packages/next/src/telemetry/flush-and-exit.ts new file mode 100644 index 0000000000000..23180c5276f48 --- /dev/null +++ b/packages/next/src/telemetry/flush-and-exit.ts @@ -0,0 +1,11 @@ +import { traceGlobals } from '../trace/shared' + +export async function flushAndExit(code: number) { + let telemetry = traceGlobals.get('telemetry') as + | InstanceType + | undefined + if (telemetry) { + await telemetry.flush() + } + process.exit(code) +} diff --git a/test/integration/image-optimizer/test/index.test.ts b/test/integration/image-optimizer/test/index.test.ts index 811acf904f669..481ffaf8c57a2 100644 --- a/test/integration/image-optimizer/test/index.test.ts +++ b/test/integration/image-optimizer/test/index.test.ts @@ -43,7 +43,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Specified images.domains exceeds length of 50, received length (51), please reduce the length of the array to continue' + 'The value at .images.domains must have 50 or fewer items but it has 51.' ) }) @@ -70,7 +70,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Specified images.remotePatterns exceeds length of 50, received length (51), please reduce the length of the array to continue' + 'The value at .images.remotePatterns must have 50 or fewer items but it has 51.' ) }) @@ -95,7 +95,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Invalid images.remotePatterns values:\n{"hostname":"example.com","foo":"bar"}' + 'The value at .images.remotePatterns[0] has an unexpected property, foo, which is not in the list of allowed properties (hostname, pathname, port, protocol).' ) }) @@ -120,7 +120,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Invalid images.remotePatterns values:\n{"protocol":"https"}' + "The value at .images.remotePatterns[0] is missing the required field 'hostname'." ) }) @@ -145,7 +145,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Specified images.deviceSizes exceeds length of 25, received length (51), please reduce the length of the array to continue' + 'The value at .images.deviceSizes must have 25 or fewer items but it has 51.' ) }) @@ -170,7 +170,10 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Specified images.deviceSizes should be an Array of numbers that are between 1 and 10000, received invalid values (0, 12000)' + 'The value at .images.deviceSizes[0] must be equal to or greater than 1.' + ) + expect(stderr).toContain( + 'The value at .images.deviceSizes[1] must be equal to or less than 10000.' ) }) @@ -195,7 +198,10 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Specified images.imageSizes should be an Array of numbers that are between 1 and 10000, received invalid values (0, 12000)' + 'The value at .images.imageSizes[0] must be equal to or greater than 1.' + ) + expect(stderr).toContain( + 'The value at .images.imageSizes[3] must be equal to or less than 10000.' ) }) @@ -220,7 +226,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - 'Specified images.loader should be one of (default, imgix, cloudinary, akamai, custom), received invalid value (notreal)' + 'The value at .images.loader must be one of: "default", "imgix", "cloudinary", "akamai", or "custom".' ) }) @@ -245,7 +251,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - `Specified images.formats should be an Array of mime type strings, received invalid values (jpeg)` + `The value at .images.formats[1] must be one of: "image/avif" or "image/webp".` ) }) @@ -345,7 +351,7 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - `Specified images.dangerouslyAllowSVG should be a boolean` + `The value at .images.dangerouslyAllowSVG must be a boolean but it was a string.` ) }) @@ -370,7 +376,82 @@ describe('Image Optimizer', () => { await nextConfig.restore() expect(stderr).toContain( - `Specified images.contentSecurityPolicy should be a string` + `The value at .images.contentSecurityPolicy must be a string but it was a number.` + ) + }) + + it('should error when images.contentDispositionType is not valid', async () => { + await nextConfig.replace( + '{ /* replaceme */ }', + JSON.stringify({ + images: { + contentDispositionType: 'nope', + }, + }) + ) + let stderr = '' + + app = await launchApp(appDir, await findPort(), { + onStderr(msg) { + stderr += msg || '' + }, + }) + await waitFor(1000) + await killApp(app).catch(() => {}) + await nextConfig.restore() + + expect(stderr).toContain( + `The value at .images.contentDispositionType must be one of: "inline" or "attachment".` + ) + }) + + it('should error when images.minimumCacheTTL is not valid', async () => { + await nextConfig.replace( + '{ /* replaceme */ }', + JSON.stringify({ + images: { + minimumCacheTTL: -1, + }, + }) + ) + let stderr = '' + + app = await launchApp(appDir, await findPort(), { + onStderr(msg) { + stderr += msg || '' + }, + }) + await waitFor(1000) + await killApp(app).catch(() => {}) + await nextConfig.restore() + + expect(stderr).toContain( + `The value at .images.minimumCacheTTL must be equal to or greater than 0.` + ) + }) + + it('should error when images.unoptimized is not a boolean', async () => { + await nextConfig.replace( + '{ /* replaceme */ }', + JSON.stringify({ + images: { + unoptimized: 'yup', + }, + }) + ) + let stderr = '' + + app = await launchApp(appDir, await findPort(), { + onStderr(msg) { + stderr += msg || '' + }, + }) + await waitFor(1000) + await killApp(app).catch(() => {}) + await nextConfig.restore() + + expect(stderr).toContain( + `The value at .images.unoptimized must be a boolean but it was a string.` ) }) })