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

Add support for Image Optimizer #17749

Merged
merged 51 commits into from Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
0d61e66
[image-optimizer] Initial commit
styfle Oct 8, 2020
19c4680
Fix several bugs
styfle Oct 9, 2020
7920ad5
Fix relative urls
styfle Oct 9, 2020
b5ad58d
Add initial test fixture
styfle Oct 9, 2020
689f7c1
Add tests
styfle Oct 9, 2020
6d2c58d
Add missing header
styfle Oct 9, 2020
3257808
Update per review comments
styfle Oct 10, 2020
b5e258d
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 10, 2020
db041fc
Remove test repo
styfle Oct 10, 2020
bbdfd60
Run lint
styfle Oct 10, 2020
b5f1dfc
Regenerate yarn.lock
styfle Oct 12, 2020
7e33323
Remove sharp from pnp test
styfle Oct 12, 2020
23ea3bf
Fix sharp install with env hack
styfle Oct 12, 2020
60b7221
Use env property
styfle Oct 12, 2020
05c3cc3
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 12, 2020
4118aa2
Use exact version
styfle Oct 12, 2020
0eeba5c
Prevent accidental octal or hexidecimal
styfle Oct 12, 2020
cd4e387
Add cache expiration
styfle Oct 12, 2020
44bc0d9
Regenerate yarn.lock
styfle Oct 12, 2020
7e5233d
Add unit tests for getMaxAge()
styfle Oct 12, 2020
170ce0f
Fix typo
styfle Oct 12, 2020
88f82ac
Fix cache expiration and add a test
styfle Oct 12, 2020
38273f6
Add more tests for max-age header
styfle Oct 12, 2020
14b0538
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 12, 2020
e1d6586
Fallback to png and jpeg
styfle Oct 13, 2020
d24cf43
Add file extension for unsupported image types
styfle Oct 13, 2020
5093267
Add tests for other file types
styfle Oct 13, 2020
081cbb5
Clean up tests
styfle Oct 13, 2020
2dd646d
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 13, 2020
254df71
should proxy-pass unsupported images
styfle Oct 13, 2020
a53ce3e
Apply suggestions from JJ
styfle Oct 14, 2020
866142c
Add parsedUrl parameter
styfle Oct 14, 2020
6d591ae
Update cache fs test
styfle Oct 14, 2020
d7b0ccc
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 14, 2020
4233849
Add defaults per timneutkens
styfle Oct 14, 2020
55dc28d
Perform local reads for relative urls
styfle Oct 14, 2020
532a164
Run prettier
styfle Oct 14, 2020
b20cfd2
Change fs back to fetch()
styfle Oct 14, 2020
fa4c1e0
Fix tests
styfle Oct 14, 2020
5ecacb5
Apply suggestions from JJ
styfle Oct 15, 2020
6f63854
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 15, 2020
e64f680
Friendly error when sharp is missing
styfle Oct 15, 2020
2776d90
Lint
styfle Oct 15, 2020
e377ecf
Update sharp to 0.26.2
styfle Oct 15, 2020
301512e
Remove "experimental" and write images-manifest.json
styfle Oct 15, 2020
f6670e5
Remove one more experimental
styfle Oct 15, 2020
59a26c0
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 15, 2020
b2e11f0
Add docs for installing sharp
styfle Oct 15, 2020
6e0c905
Throw error when sharp was not found
styfle Oct 15, 2020
c53aec6
Merge branch 'canary' into rfc17141-next-image-api
styfle Oct 15, 2020
17cddcb
Add test for invalid image
styfle Oct 15, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/next/build/index.ts
Expand Up @@ -29,6 +29,7 @@ import {
CLIENT_STATIC_FILES_PATH,
EXPORT_DETAIL,
EXPORT_MARKER,
IMAGES_MANIFEST,
PAGES_MANIFEST,
PHASE_PRODUCTION_BUILD,
PRERENDER_MANIFEST,
Expand Down Expand Up @@ -1083,6 +1084,15 @@ export default async function build(
)
}

await promises.writeFile(
path.join(distDir, IMAGES_MANIFEST),
JSON.stringify({
version: 1,
images: config.images,
}),
'utf8'
)

await promises.writeFile(
path.join(distDir, EXPORT_MARKER),
JSON.stringify({
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/lib/constants.ts
Expand Up @@ -8,6 +8,7 @@ export const EXPORT_MARKER = 'export-marker.json'
export const EXPORT_DETAIL = 'export-detail.json'
export const PRERENDER_MANIFEST = 'prerender-manifest.json'
export const ROUTES_MANIFEST = 'routes-manifest.json'
export const IMAGES_MANIFEST = 'images-manifest.json'
export const DEV_CLIENT_PAGES_MANIFEST = '_devPagesManifest.json'
export const REACT_LOADABLE_MANIFEST = 'react-loadable-manifest.json'
export const FONT_MANIFEST = 'font-manifest.json'
Expand Down
49 changes: 47 additions & 2 deletions packages/next/next-server/server/config.ts
Expand Up @@ -23,7 +23,11 @@ const defaultConfig: { [key: string]: any } = {
target: 'server',
poweredByHeader: true,
compress: true,
images: { hosts: { default: { path: 'defaultconfig' } } },
images: {
sizes: [320, 420, 768, 1024, 1200],
domains: [],
hosts: { default: { path: 'defaultconfig' } },
},
devIndicators: {
buildActivity: true,
autoPrerender: true,
Expand Down Expand Up @@ -208,6 +212,47 @@ function assignDefaults(userConfig: { [key: string]: any }) {
}
}

if (result?.images) {
const { images } = result
if (typeof images !== 'object') {
throw new Error(
`Specified images should be an object received ${typeof images}`
)
}
ijjk marked this conversation as resolved.
Show resolved Hide resolved
if (images.domains) {
if (!Array.isArray(images.domains)) {
throw new Error(
`Specified images.domains should be an Array received ${typeof images.domains}`
)
}
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(
', '
)})`
)
}
}
if (images.sizes) {
if (!Array.isArray(images.sizes)) {
throw new Error(
`Specified images.sizes should be an Array received ${typeof images.sizes}`
)
}
const invalid = images.sizes.filter((d: unknown) => typeof d !== 'number')
if (invalid.length > 0) {
throw new Error(
`Specified images.sizes should be an Array of numbers received invalid values (${invalid.join(
', '
)})`
)
}
}
}

if (result.experimental?.i18n) {
const { i18n } = result.experimental
const i18nType = typeof i18n
Expand All @@ -218,7 +263,7 @@ function assignDefaults(userConfig: { [key: string]: any }) {

if (!Array.isArray(i18n.locales)) {
throw new Error(
`Specified i18n.locales should be an Array received ${typeof i18n.lcoales}`
`Specified i18n.locales should be an Array received ${typeof i18n.locales}`
styfle marked this conversation as resolved.
Show resolved Hide resolved
)
}

Expand Down
240 changes: 240 additions & 0 deletions packages/next/next-server/server/image-optimizer.ts
@@ -0,0 +1,240 @@
import { UrlWithParsedQuery } from 'url'
import { IncomingMessage, ServerResponse } from 'http'
import { join } from 'path'
import { mediaType } from '@hapi/accept'
import { createReadStream, promises } from 'fs'
import { createHash } from 'crypto'
import Server from './next-server'
import { getContentType, getExtension } from './serve-static'
import { fileExists } from '../../lib/file-exists'

let sharp: typeof import('sharp')
//const AVIF = 'image/avif'
const WEBP = 'image/webp'
const PNG = 'image/png'
const JPEG = 'image/jpeg'
const MIME_TYPES = [/* AVIF, */ WEBP, PNG, JPEG]
const CACHE_VERSION = 1

export async function imageOptimizer(
server: Server,
req: IncomingMessage,
res: ServerResponse,
parsedUrl: UrlWithParsedQuery
) {
const { nextConfig, distDir } = server
const { sizes = [], domains = [] } = nextConfig?.images || {}
const { headers } = req
const { url, w, q } = parsedUrl.query
const proto = headers['x-forwarded-proto'] || 'http'
const host = headers['x-forwarded-host'] || headers.host
const mimeType = mediaType(headers.accept, MIME_TYPES) || ''

if (!url) {
res.statusCode = 400
res.end('"url" parameter is required')
return { finished: true }
} else if (Array.isArray(url)) {
res.statusCode = 400
res.end('"url" parameter cannot be an array')
return { finished: true }
}

let absoluteUrl: URL
try {
absoluteUrl = new URL(url)
} catch (_error) {
// url was not absolute so assuming relative url
try {
absoluteUrl = new URL(url, `${proto}://${host}`)
} catch (__error) {
res.statusCode = 400
res.end('"url" parameter is invalid')
return { finished: true }
}
}

if (!['http:', 'https:'].includes(absoluteUrl.protocol)) {
res.statusCode = 400
res.end('"url" parameter is invalid')
return { finished: true }
}

if (!server.renderOpts.dev && !domains.includes(absoluteUrl.hostname)) {
res.statusCode = 400
res.end('"url" parameter is not allowed')
return { finished: true }
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to validate that absoluteUrl.pathname is not /_next/image/? to avoid infinite loop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you need to pass parameters, so it cannot be infinite and not a big problem.


if (!w) {
res.statusCode = 400
res.end('"w" parameter (width) is required')
return { finished: true }
} else if (Array.isArray(w)) {
res.statusCode = 400
res.end('"w" parameter (width) cannot be an array')
return { finished: true }
}

if (!q) {
res.statusCode = 400
res.end('"q" parameter (quality) is required')
return { finished: true }
} else if (Array.isArray(q)) {
res.statusCode = 400
res.end('"q" parameter (quality) cannot be an array')
return { finished: true }
}
ijjk marked this conversation as resolved.
Show resolved Hide resolved

const width = parseInt(w, 10)

if (!width || isNaN(width)) {
res.statusCode = 400
res.end('"w" parameter (width) must be a number greater than 0')
return { finished: true }
}

if (!sizes.includes(width)) {
res.statusCode = 400
res.end(`"w" parameter (width) of ${width} is not allowed`)
return { finished: true }
}

const quality = parseInt(q)

if (isNaN(quality) || quality < 1 || quality > 100) {
nkzawa marked this conversation as resolved.
Show resolved Hide resolved
res.statusCode = 400
res.end('"q" parameter (quality) must be a number between 1 and 100')
return { finished: true }
}

const { href } = absoluteUrl
const hash = getHash([CACHE_VERSION, href, width, quality, mimeType])
const imagesDir = join(distDir, 'cache', 'images')
const hashDir = join(imagesDir, hash)
const now = Date.now()

if (await fileExists(hashDir, 'directory')) {
const files = await promises.readdir(hashDir)
for (let file of files) {
const [filename, extension] = file.split('.')
const expireAt = Number(filename)
const contentType = getContentType(extension)
if (now < expireAt) {
if (contentType) {
res.setHeader('Content-Type', contentType)
}
createReadStream(join(hashDir, file)).pipe(res)
return { finished: true }
} else {
await promises.unlink(join(hashDir, file))
}
}
}

const upstreamRes = await fetch(href)

if (!upstreamRes.ok) {
server.logError(
new Error(`Unexpected status ${upstreamRes.status} from upstream ${href}`)
)
return { finished: true }
ijjk marked this conversation as resolved.
Show resolved Hide resolved
}

const upstreamBuffer = Buffer.from(await upstreamRes.arrayBuffer())
const upstreamType = upstreamRes.headers.get('Content-Type')
const maxAge = getMaxAge(upstreamRes.headers.get('Cache-Control'))
const expireAt = maxAge * 1000 + now
let contentType: string

if (mimeType) {
contentType = mimeType
} else if (upstreamType?.startsWith('image/') && getExtension(upstreamType)) {
contentType = upstreamType
} else {
contentType = JPEG
}

if (!sharp) {
try {
// eslint-disable-next-line import/no-extraneous-dependencies
sharp = require('sharp')
} catch (error) {
error.message +=
'\nDid you forget to add it to "dependencies" in `package.json`?'
ijjk marked this conversation as resolved.
Show resolved Hide resolved
server.logError(error)
}
}

const transformer = sharp(upstreamBuffer).resize(width)

//if (contentType === AVIF) {
// Soon https://github.com/lovell/sharp/issues/2289
//}
if (contentType === WEBP) {
transformer.webp({ quality })
} else if (contentType === PNG) {
transformer.png({ quality })
} else if (contentType === JPEG) {
transformer.jpeg({ quality })
}

try {
const optimizedBuffer = await transformer.toBuffer()
await promises.mkdir(hashDir, { recursive: true })
const extension = getExtension(contentType)
const filename = join(hashDir, `${expireAt}.${extension}`)
await promises.writeFile(filename, optimizedBuffer)
res.setHeader('Content-Type', contentType)
res.end(optimizedBuffer)
} catch (error) {
server.logError(error)
if (upstreamType) {
res.setHeader('Content-Type', upstreamType)
}
res.end(upstreamBuffer)
}

return { finished: true }
}

function getHash(items: (string | number | undefined)[]) {
const hash = createHash('sha256')
for (let item of items) {
hash.update(String(item))
}
// See https://en.wikipedia.org/wiki/Base64#Filenames
return hash.digest('base64').replace(/\//g, '-')
}

function parseCacheControl(str: string | null): Map<string, string> {
const map = new Map<string, string>()
if (!str) {
return map
}
for (let directive of str.split(',')) {
let [key, value] = directive.trim().split('=')
key = key.toLowerCase()
if (value) {
value = value.toLowerCase()
}
map.set(key, value)
}
return map
}

export function getMaxAge(str: string | null): number {
const minimum = 60
const map = parseCacheControl(str)
if (map) {
let age = map.get('s-maxage') || map.get('max-age') || ''
if (age.startsWith('"') && age.endsWith('"')) {
age = age.slice(1, -1)
}
const n = parseInt(age, 10)
if (!isNaN(n)) {
return Math.max(n, minimum)
}
}
return minimum
}
11 changes: 10 additions & 1 deletion packages/next/next-server/server/next-server.ts
Expand Up @@ -79,6 +79,7 @@ import accept from '@hapi/accept'
import { normalizeLocalePath } from '../lib/i18n/normalize-locale-path'
import { detectLocaleCookie } from '../lib/i18n/detect-locale-cookie'
import * as Log from '../../build/output/log'
import { imageOptimizer } from './image-optimizer'
import { detectDomainLocale } from '../lib/i18n/detect-domain-locale'
import cookie from 'next/dist/compiled/cookie'

Expand Down Expand Up @@ -271,7 +272,7 @@ export default class Server {
return PHASE_PRODUCTION_SERVER
}

private logError(err: Error): void {
public logError(err: Error): void {
if (this.onErrorMiddleware) {
this.onErrorMiddleware({ err })
}
Expand Down Expand Up @@ -471,6 +472,7 @@ export default class Server {
useFileSystemPublicRoutes: boolean
dynamicRoutes: DynamicRoutes | undefined
} {
const server: Server = this
const publicRoutes = fs.existsSync(this.publicDir)
? this.generatePublicRoutes()
: []
Expand Down Expand Up @@ -588,6 +590,13 @@ export default class Server {
}
},
},
{
match: route('/_next/image'),
type: 'route',
name: '_next/image catchall',
fn: (req, res, _params, parsedUrl) =>
imageOptimizer(server, req, res, parsedUrl),
},
{
match: route('/_next/:path*'),
type: 'route',
Expand Down