Skip to content

Commit

Permalink
Update handling for relative files in image-optimizer (#17998)
Browse files Browse the repository at this point in the history 
This updates the new image optimizer endpoint to instead of relying on the `host` and `proto` headers for relative files to use the internal route handling in `next-server` to load files from the public directory. The existing tests for relative files with the endpoint should cover these changes
  • Loading branch information
@ijjk
ijjk committed Oct 19, 2020
1 parent 0d3671c commit 114de06
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 63 deletions.
109 changes: 81 additions & 28 deletions packages/next/next-server/server/image-optimizer.ts
View file
@@ -1,4 +1,4 @@
import { UrlWithParsedQuery } from 'url'
import nodeUrl, { UrlWithParsedQuery } from 'url'
import { IncomingMessage, ServerResponse } from 'http'
import { join } from 'path'
import { mediaType } from '@hapi/accept'
Expand All @@ -9,6 +9,7 @@ import { getContentType, getExtension } from './serve-static'
import { fileExists } from '../../lib/file-exists'
// @ts-ignore no types for is-animated
import isAnimated from 'next/dist/compiled/is-animated'
import Stream from 'stream'

let sharp: typeof import('sharp')
//const AVIF = 'image/avif'
Expand All @@ -30,9 +31,8 @@ export async function imageOptimizer(
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) || ''
let href: string

if (!url) {
res.statusCode = 400
Expand All @@ -44,30 +44,35 @@ export async function imageOptimizer(
return { finished: true }
}

let absoluteUrl: URL
try {
absoluteUrl = new URL(url)
} catch (_error) {
// url was not absolute so assuming relative url
let isAbsolute: boolean

if (url.startsWith('/')) {
href = url
isAbsolute = false
} else {
let hrefParsed: URL

try {
absoluteUrl = new URL(url, `${proto}://${host}`)
} catch (__error) {
hrefParsed = new URL(url)
href = hrefParsed.toString()
isAbsolute = true
} 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 (!['http:', 'https:'].includes(hrefParsed.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 }
if (!domains.includes(hrefParsed.hostname)) {
res.statusCode = 400
res.end('"url" parameter is not allowed')
return { finished: true }
}
}

if (!w) {
Expand Down Expand Up @@ -112,7 +117,6 @@ export async function imageOptimizer(
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)
Expand All @@ -136,17 +140,66 @@ export async function imageOptimizer(
}
}

const upstreamRes = await fetch(href)
let upstreamBuffer: Buffer
let upstreamType: string | null
let maxAge: number

if (isAbsolute) {
const upstreamRes = await fetch(href)

if (!upstreamRes.ok) {
res.statusCode = upstreamRes.status
res.end('"url" parameter is valid but upstream response is invalid')
return { finished: true }
}

if (!upstreamRes.ok) {
res.statusCode = upstreamRes.status
res.end('"url" parameter is valid but upstream response is invalid')
return { finished: true }
upstreamBuffer = Buffer.from(await upstreamRes.arrayBuffer())
upstreamType = upstreamRes.headers.get('Content-Type')
maxAge = getMaxAge(upstreamRes.headers.get('Cache-Control'))
} else {
try {
const _req: any = {
headers: req.headers,
method: req.method,
url: href,
}
const resBuffers: Buffer[] = []
const mockRes: any = new Stream.Writable()

mockRes.write = (chunk: Buffer | string) => {
resBuffers.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk))
}
mockRes._write = (chunk: Buffer | string) => {
mockRes.write(chunk)
}

const mockHeaders: Record<string, string | string[]> = {}

mockRes.writeHead = (_status: any, _headers: any) =>
Object.assign(mockHeaders, _headers)
mockRes.getHeader = (name: string) => mockHeaders[name.toLowerCase()]
mockRes.getHeaders = () => mockHeaders
mockRes.getHeaderNames = () => Object.keys(mockHeaders)
mockRes.setHeader = (name: string, value: string | string[]) =>
(mockHeaders[name.toLowerCase()] = value)
mockRes._implicitHeader = () => {}
mockRes.finished = false
mockRes.statusCode = 200

await server.getRequestHandler()(_req, mockRes, nodeUrl.parse(href, true))
res.statusCode = mockRes.statusCode

upstreamBuffer = Buffer.concat(resBuffers)
upstreamType = mockRes.getHeader('Content-Type')
maxAge = getMaxAge(mockRes.getHeader('Cache-Control'))
} catch (err) {
res.statusCode = 500
res.end('"url" parameter is valid but upstream response is invalid')
return { finished: true }
}
}

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

Expand Down
90 changes: 55 additions & 35 deletions test/integration/image-optimizer/test/index.test.js
View file
Expand Up @@ -35,7 +35,7 @@ async function fsToJson(dir, output = {}) {
return output
}

function runTests({ w, isDev }) {
function runTests({ w, isDev, domains }) {
it('should return home page', async () => {
const res = await fetchViaHTTP(appPort, '/', null, {})
expect(await res.text()).toMatch(/Image Optimizer Home/m)
Expand Down Expand Up @@ -131,16 +131,14 @@ function runTests({ w, isDev }) {
)
})

if (!isDev) {
it('should fail when domain is not defined in next.config.js', async () => {
const url = `http://vercel.com/button`
const query = { url, w, q: 100 }
const opts = { headers: { accept: 'image/webp' } }
const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
expect(res.status).toBe(400)
expect(await res.text()).toBe(`"url" parameter is not allowed`)
})
}
it('should fail when domain is not defined in next.config.js', async () => {
const url = `http://vercel.com/button`
const query = { url, w, q: 100 }
const opts = { headers: { accept: 'image/webp' } }
const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
expect(res.status).toBe(400)
expect(await res.text()).toBe(`"url" parameter is not allowed`)
})

it('should fail when width is not in next.config.js', async () => {
const query = { url: '/test.png', w: 1000, q: 100 }
Expand Down Expand Up @@ -216,14 +214,16 @@ function runTests({ w, isDev }) {
expect(res.headers.get('Content-Type')).toBe('image/webp')
})

it('should resize absolute url from localhost', async () => {
const url = `http://localhost:${appPort}/test.png`
const query = { url, w, q: 80 }
const opts = { headers: { accept: 'image/webp' } }
const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
expect(res.status).toBe(200)
expect(res.headers.get('Content-Type')).toBe('image/webp')
})
if (domains.includes('localhost')) {
it('should resize absolute url from localhost', async () => {
const url = `http://localhost:${appPort}/test.png`
const query = { url, w, q: 80 }
const opts = { headers: { accept: 'image/webp' } }
const res = await fetchViaHTTP(appPort, '/_next/image', query, opts)
expect(res.status).toBe(200)
expect(res.headers.get('Content-Type')).toBe('image/webp')
})
}

it('should fail when url has file protocol', async () => {
const url = `file://localhost:${appPort}/test.png`
Expand All @@ -243,15 +243,17 @@ function runTests({ w, isDev }) {
expect(await res.text()).toBe(`"url" parameter is invalid`)
})

it('should fail when url fails to load an image', async () => {
const url = `http://localhost:${appPort}/not-an-image`
const query = { w, url, q: 100 }
const res = await fetchViaHTTP(appPort, '/_next/image', query, {})
expect(res.status).toBe(404)
expect(await res.text()).toBe(
`"url" parameter is valid but upstream response is invalid`
)
})
if (domains.includes('localhost')) {
it('should fail when url fails to load an image', async () => {
const url = `http://localhost:${appPort}/not-an-image`
const query = { w, url, q: 100 }
const res = await fetchViaHTTP(appPort, '/_next/image', query, {})
expect(res.status).toBe(404)
expect(await res.text()).toBe(
`"url" parameter is valid but upstream response is invalid`
)
})
}

it('should use cached image file when parameters are the same', async () => {
await fs.remove(imagesDir)
Expand Down Expand Up @@ -288,6 +290,9 @@ function runTests({ w, isDev }) {
}

describe('Image Optimizer', () => {
// domains for testing
const domains = ['localhost', 'example.com']

describe('dev support w/o next.config.js', () => {
const size = 768 // defaults defined in server/config.ts
beforeAll(async () => {
Expand All @@ -299,7 +304,7 @@ describe('Image Optimizer', () => {
await fs.remove(imagesDir)
})

runTests({ w: size, isDev: true })
runTests({ w: size, isDev: true, domains: [] })
})

describe('dev support with next.config.js', () => {
Expand All @@ -308,7 +313,7 @@ describe('Image Optimizer', () => {
const json = JSON.stringify({
images: {
sizes: [size],
domains: ['localhost', 'example.com'],
domains,
},
})
nextConfig.replace('{ /* replaceme */ }', json)
Expand All @@ -321,7 +326,22 @@ describe('Image Optimizer', () => {
await fs.remove(imagesDir)
})

runTests({ w: size, isDev: true })
runTests({ w: size, isDev: true, domains })
})

describe('Server support w/o next.config.js', () => {
const size = 768 // defaults defined in server/config.ts
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(async () => {
await killApp(app)
await fs.remove(imagesDir)
})

runTests({ w: size, isDev: false, domains: [] })
})

describe('Server support with next.config.js', () => {
Expand All @@ -330,7 +350,7 @@ describe('Image Optimizer', () => {
const json = JSON.stringify({
images: {
sizes: [128],
domains: ['localhost', 'example.com'],
domains,
},
})
nextConfig.replace('{ /* replaceme */ }', json)
Expand All @@ -344,7 +364,7 @@ describe('Image Optimizer', () => {
await fs.remove(imagesDir)
})

runTests({ w: size, isDev: false })
runTests({ w: size, isDev: false, domains })
})

describe('Serverless support with next.config.js', () => {
Expand All @@ -354,7 +374,7 @@ describe('Image Optimizer', () => {
target: 'experimental-serverless-trace',
images: {
sizes: [size],
domains: ['localhost', 'example.com'],
domains,
},
})
nextConfig.replace('{ /* replaceme */ }', json)
Expand All @@ -368,6 +388,6 @@ describe('Image Optimizer', () => {
await fs.remove(imagesDir)
})

runTests({ w: size, isDev: false })
runTests({ w: size, isDev: false, domains })
})
})

0 comments on commit 114de06

Please sign in to comment.