Skip to content

Commit

Permalink
Fix image optimization encoding url (#28045)
Browse files Browse the repository at this point in the history
- Fixes #27973 
- Reverts #27671 


The problem with PR #27671 is that it was encoding too often when it really only needed to solve the bug for `next build && next start` since `next dev` was already working.

This PR uses the alternative solution mentioned here #27210 (comment)
  • Loading branch information
styfle committed Aug 13, 2021
1 parent 1552b83 commit b11bd49
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 6 deletions.
8 changes: 3 additions & 5 deletions packages/next/server/image-optimizer.ts
Expand Up @@ -69,22 +69,20 @@ export async function imageOptimizer(
}

const { headers } = req
const { url: decodedUrl, w, q } = parsedUrl.query
const { url, w, q } = parsedUrl.query
const mimeType = getSupportedMimeType(MODERN_TYPES, headers.accept)
let href: string

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

const url = encodeURI(decodedUrl)

let isAbsolute: boolean

if (url.startsWith('/')) {
Expand Down
9 changes: 8 additions & 1 deletion packages/next/server/next-server.ts
Expand Up @@ -1158,7 +1158,14 @@ export default class Server {
pathParts.splice(0, basePathParts.length)
}

const path = `/${pathParts.join('/')}`
let path = `/${pathParts.join('/')}`

if (!publicFiles.has(path)) {
// In `next-dev-server.ts`, we ensure encoded paths match
// decoded paths on the filesystem. So we need do the
// opposite here: make sure decoded paths match encoded.
path = encodeURI(path)
}

if (publicFiles.has(path)) {
await this.serveStatic(
Expand Down
5 changes: 5 additions & 0 deletions test/integration/image-component/unicode/next.config.js
@@ -0,0 +1,5 @@
module.exports = {
images: {
domains: ['image-optimization-test.vercel.app'],
},
}
33 changes: 33 additions & 0 deletions test/integration/image-component/unicode/pages/index.js
@@ -0,0 +1,33 @@
import React from 'react'
import Image from 'next/image'
import img from '../public/äöü.png'

const Page = () => {
return (
<div>
<h1>Unicode Image URL</h1>
<Image id="static" src={img} />
<Image id="internal" src="/äöü.png" width={400} height={400} />
<Image
id="external"
src="https://image-optimization-test.vercel.app/äöü.png"
width={400}
height={400}
/>
<Image
id="internal-space"
src="/hello%20world.jpg"
width={200}
height={200}
/>
<Image
id="external-space"
src="https://image-optimization-test.vercel.app/hello%20world.jpg"
width={200}
height={200}
/>
</div>
)
}

export default Page
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
96 changes: 96 additions & 0 deletions test/integration/image-component/unicode/test/index.test.js
@@ -0,0 +1,96 @@
/* eslint-env jest */

import {
findPort,
killApp,
launchApp,
nextBuild,
nextStart,
} from 'next-test-utils'
import webdriver from 'next-webdriver'
//import fetch from 'node-fetch'
import { join } from 'path'

jest.setTimeout(1000 * 60)

const appDir = join(__dirname, '../')

let appPort
let app
let browser

function runTests() {
it('should load static unicode image', async () => {
const src = await browser.elementById('static').getAttribute('src')
expect(src).toMatch(
/_next%2Fstatic%2Fimage%2Fpublic%2F%C3%A4%C3%B6%C3%BC(.+)png/
)
const res = await fetch(src)
expect(res.status).toBe(200)
})

it('should load internal unicode image', async () => {
const src = await browser.elementById('internal').getAttribute('src')
expect(src).toMatch('/_next/image?url=%2F%C3%A4%C3%B6%C3%BC.png')
const res = await fetch(src)
expect(res.status).toBe(200)
})

it('should load external unicode image', async () => {
const src = await browser.elementById('external').getAttribute('src')
expect(src).toMatch(
'/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2F%C3%A4%C3%B6%C3%BC.png'
)
const res = await fetch(src)
expect(res.status).toBe(200)
})

it('should load internal image with space', async () => {
const src = await browser.elementById('internal-space').getAttribute('src')
expect(src).toMatch('/_next/image?url=%2Fhello%2520world.jpg')
const res = await fetch(src)
expect(res.status).toBe(200)
})

it('should load external image with space', async () => {
const src = await browser.elementById('external-space').getAttribute('src')
expect(src).toMatch(
'/_next/image?url=https%3A%2F%2Fimage-optimization-test.vercel.app%2Fhello%2520world.jpg'
)
const res = await fetch(src)
expect(res.status).toBe(200)
})
}

describe('Image Component Unicode Image URL', () => {
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
browser = await webdriver(appPort, '/')
})
afterAll(() => {
killApp(app)
if (browser) {
browser.close()
}
})
runTests()
})

describe('server mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
browser = await webdriver(appPort, '/')
})
afterAll(() => {
killApp(app)
if (browser) {
browser.close()
}
})
runTests()
})
})

0 comments on commit b11bd49

Please sign in to comment.