-
Notifications
You must be signed in to change notification settings - Fork 28.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add support for wildcard patterns in image.domains #18730
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -86,7 +86,23 @@ export async function imageOptimizer( | |||||
return { finished: true } | ||||||
} | ||||||
|
||||||
if (!domains.includes(hrefParsed.hostname)) { | ||||||
if ( | ||||||
!domains.some((domain) => { | ||||||
if (domain.includes('*')) { | ||||||
const domainExpr = new RegExp( | ||||||
'^' + | ||||||
domain | ||||||
.split(/\*+/) | ||||||
.map((s) => s.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&')) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this regex to escape regex? MDN has a slightly different example https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping I wonder if we can do this asterisk matching without regex and avoid escaping. |
||||||
.join('.*') + | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think this should match one or more, right? |
||||||
'$' | ||||||
) | ||||||
return domainExpr.test(hrefParsed.hostname) | ||||||
} else { | ||||||
return hrefParsed.hostname === domain | ||||||
} | ||||||
}) | ||||||
) { | ||||||
res.statusCode = 400 | ||||||
res.end('"url" parameter is not allowed') | ||||||
return { finished: true } | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
module.exports = { | ||
images: { | ||
domains: ['*.placeholder.com'], | ||
}, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import React from 'react' | ||
import Image from 'next/image' | ||
|
||
const Invalid = () => { | ||
return ( | ||
<div> | ||
<Image src="https://via.invalid.com/500" width={500} height={500} /> | ||
<p id="stubtext">This is a page with errors</p> | ||
</div> | ||
) | ||
} | ||
|
||
export default Invalid |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import React from 'react' | ||
import Image from 'next/image' | ||
|
||
const Page = () => { | ||
return ( | ||
<div> | ||
<Image src="https://via.placeholder.com/500" width={500} height={500} /> | ||
<p>This is a valid domain</p> | ||
</div> | ||
) | ||
} | ||
|
||
export default Page |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* eslint-env jest */ | ||
|
||
import { join } from 'path' | ||
import { renderViaHTTP, findPort, launchApp, killApp } from 'next-test-utils' | ||
|
||
jest.setTimeout(1000 * 60 * 2) | ||
|
||
const appDir = join(__dirname, '..') | ||
let appPort | ||
let app | ||
|
||
describe('Image Component Domain Pattern', () => { | ||
describe('next dev', () => { | ||
beforeAll(async () => { | ||
appPort = await findPort() | ||
app = await launchApp(appDir, appPort) | ||
}) | ||
afterAll(() => killApp(app)) | ||
|
||
it('should render the valid Image usage and not print error', async () => { | ||
const html = await renderViaHTTP(appPort, '/valid', {}) | ||
expect(html).toMatch(/This is a valid domain/) | ||
expect(html).not.toMatch(/hostname .* is not configured under images/) | ||
}) | ||
|
||
it('should print error when invalid Image usage', async () => { | ||
const html = await renderViaHTTP(appPort, '/invalid', {}) | ||
expect(html).toMatch(/hostname .* is not configured under images/) | ||
expect(html).not.toMatch(/This is a page with errors/) | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to allow multiple asterisks?