Skip to content

Commit

Permalink
Fix experimental remotePatterns wildcard (#37137)
Browse files Browse the repository at this point in the history 
- Follow up to #36245 
- Closes #37026
  • Loading branch information
@styfle
styfle committed May 25, 2022
1 parent 82a9d21 commit 0d95886
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 68 deletions.
12 changes: 10 additions & 2 deletions packages/next/build/index.ts
View file
Expand Up @@ -2,7 +2,7 @@ import type { webpack5 as webpack } from 'next/dist/compiled/webpack/webpack'
import { loadEnvConfig } from '@next/env'
import chalk from 'next/dist/compiled/chalk'
import crypto from 'crypto'
import { isMatch } from 'next/dist/compiled/micromatch'
import { isMatch, makeRe } from 'next/dist/compiled/micromatch'
import { promises, writeFileSync } from 'fs'
import { Worker as JestWorker } from 'next/dist/compiled/jest-worker'
import { Worker } from '../lib/worker'
Expand Down Expand Up @@ -111,6 +111,7 @@ import { lockfilePatchPromise, teardownTraceSubscriber } from './swc'
import { injectedClientEntries } from './webpack/plugins/client-entry-plugin'
import { getNamedRouteRegex } from '../shared/lib/router/utils/route-regex'
import { flatReaddir } from '../lib/flat-readdir'
import { RemotePattern } from '../shared/lib/image-config'

export type SsgRoute = {
initialRevalidateSeconds: number | false
Expand Down Expand Up @@ -2215,8 +2216,15 @@ export default async function build(
const images = { ...config.images }
const { deviceSizes, imageSizes } = images
;(images as any).sizes = [...deviceSizes, ...imageSizes]
;(images as any).remotePatterns =
;(images as any).remotePatterns = (
config?.experimental?.images?.remotePatterns || []
).map((p: RemotePattern) => ({
// Should be the same as matchRemotePattern()
protocol: p.protocol,
hostname: makeRe(p.hostname).source,
port: p.port,
pathname: makeRe(p.pathname ?? '**').source,
}))

await promises.writeFile(
path.join(distDir, IMAGES_MANIFEST),
Expand Down
57 changes: 8 additions & 49 deletions packages/next/shared/lib/match-remote-pattern.ts
View file
@@ -1,4 +1,5 @@
import type { RemotePattern } from './image-config'
import { makeRe } from 'next/dist/compiled/micromatch'

export function matchRemotePattern(pattern: RemotePattern, url: URL): boolean {
if (pattern.protocol !== undefined) {
Expand All @@ -12,63 +13,21 @@ export function matchRemotePattern(pattern: RemotePattern, url: URL): boolean {
return false
}
}
if (pattern.pathname !== undefined) {
const patternParts = pattern.pathname.split('/')
const actualParts = url.pathname.split('/')
const len = Math.max(patternParts.length, actualParts.length)
for (let i = 0; i < len; i++) {
if (patternParts[i] === '**' && actualParts[i] !== undefined) {
// Double asterisk means "match everything until the end of the path"
// so we can break the loop early. But we throw
// if the double asterisk is not the last part.
if (patternParts.length - 1 > i) {
throw new Error(
`Pattern can only contain ** at end of pathname but found "${pattern.pathname}"`
)
}
break
}
if (patternParts[i] === '*') {
// Single asterisk means "match this part" so we can
// continue to the next part of the loop
continue
}
if (patternParts[i] !== actualParts[i]) {
return false
}
}
}

if (pattern.hostname === undefined) {
throw new Error(
`Pattern should define hostname but found\n${JSON.stringify(pattern)}`
)
} else {
const patternParts = pattern.hostname.split('.').reverse()
const actualParts = url.hostname.split('.').reverse()
const len = Math.max(patternParts.length, actualParts.length)
for (let i = 0; i < len; i++) {
if (patternParts[i] === '**' && actualParts[i] !== undefined) {
// Double asterisk means "match every subdomain"
// so we can break the loop early. But we throw
// if the double asterisk is not the last part.
if (patternParts.length - 1 > i) {
throw new Error(
`Pattern can only contain ** at start of hostname but found "${pattern.hostname}"`
)
}
break
}
if (patternParts[i] === '*') {
// Single asterisk means "match this subdomain" so we can
// continue to the next part of the loop
continue
}
if (patternParts[i] !== actualParts[i]) {
return false
}
if (!makeRe(pattern.hostname).test(url.hostname)) {
return false
}
}

if (!makeRe(pattern.pathname ?? '**').test(url.pathname)) {
return false
}

return true
}

Expand Down
147 changes: 130 additions & 17 deletions test/unit/image-optimizer/match-remote-pattern.test.ts
View file
Expand Up @@ -102,7 +102,7 @@ describe('matchRemotePattern', () => {
expect(m(p, new URL('ftp://example.com/path/to/file'))).toBe(false)
})

it('should match hostname pattern with single asterisk', () => {
it('should match hostname pattern with single asterisk by itself', () => {
const p = { hostname: 'avatars.*.example.com' } as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
Expand All @@ -115,6 +115,54 @@ describe('matchRemotePattern', () => {
expect(m(p, new URL('https://more.avatars.iad1.example.com'))).toBe(false)
})

it('should match hostname pattern with single asterisk at beginning', () => {
const p = { hostname: 'avatars.*1.example.com' } as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
expect(m(p, new URL('https://sub.example.com'))).toBe(false)
expect(m(p, new URL('https://example.com.uk'))).toBe(false)
expect(m(p, new URL('https://sub.example.com.uk'))).toBe(false)
expect(m(p, new URL('https://avatars.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.sfo1.example.com'))).toBe(true)
expect(m(p, new URL('https://avatars.iad1.example.com'))).toBe(true)
expect(m(p, new URL('https://more.avatars.iad1.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.sfo2.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.iad2.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.1.example.com'))).toBe(true)
})

it('should match hostname pattern with single asterisk in middle', () => {
const p = { hostname: 'avatars.*a*.example.com' } as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
expect(m(p, new URL('https://sub.example.com'))).toBe(false)
expect(m(p, new URL('https://example.com.uk'))).toBe(false)
expect(m(p, new URL('https://sub.example.com.uk'))).toBe(false)
expect(m(p, new URL('https://avatars.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.sfo1.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.iad1.example.com'))).toBe(true)
expect(m(p, new URL('https://more.avatars.iad1.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.sfo2.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.iad2.example.com'))).toBe(true)
expect(m(p, new URL('https://avatars.a.example.com'))).toBe(true)
})

it('should match hostname pattern with single asterisk at end', () => {
const p = { hostname: 'avatars.ia*.example.com' } as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
expect(m(p, new URL('https://sub.example.com'))).toBe(false)
expect(m(p, new URL('https://example.com.uk'))).toBe(false)
expect(m(p, new URL('https://sub.example.com.uk'))).toBe(false)
expect(m(p, new URL('https://avatars.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.sfo1.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.iad1.example.com'))).toBe(true)
expect(m(p, new URL('https://more.avatars.iad1.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.sfo2.example.com'))).toBe(false)
expect(m(p, new URL('https://avatars.iad2.example.com'))).toBe(true)
expect(m(p, new URL('https://avatars.ia.example.com'))).toBe(true)
})

it('should match hostname pattern with double asterisk', () => {
const p = { hostname: '**.example.com' } as const
expect(m(p, new URL('https://com'))).toBe(false)
Expand All @@ -129,7 +177,7 @@ describe('matchRemotePattern', () => {
expect(m(p, new URL('https://more.avatars.iad1.example.com'))).toBe(true)
})

it('should match pathname pattern with single asterisk', () => {
it('should match pathname pattern with single asterisk by itself', () => {
const p = {
hostname: 'example.com',
pathname: '/act123/*/pic.jpg',
Expand All @@ -150,13 +198,89 @@ describe('matchRemotePattern', () => {
expect(m(p, new URL('https://example.com/team/pic.jpg'))).toBe(false)
})

it('should match pathname pattern with single asterisk at the beginning', () => {
const p = {
hostname: 'example.com',
pathname: '/act123/*4/pic.jpg',
} as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
expect(m(p, new URL('https://sub.example.com'))).toBe(false)
expect(m(p, new URL('https://example.com.uk'))).toBe(false)
expect(m(p, new URL('https://example.com/act123'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/pic'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/picsjpg'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/usr5/pic.jpg'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/team4/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act456/team5/pic.jpg'))).toBe(
false
)
expect(m(p, new URL('https://example.com/team/pic.jpg'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/4/pic.jpg'))).toBe(true)
})

it('should match pathname pattern with single asterisk in the middle', () => {
const p = {
hostname: 'example.com',
pathname: '/act123/*sr*/pic.jpg',
} as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
expect(m(p, new URL('https://sub.example.com'))).toBe(false)
expect(m(p, new URL('https://example.com.uk'))).toBe(false)
expect(m(p, new URL('https://example.com/act123'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/pic'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/picsjpg'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/usr5/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/team4/pic.jpg'))).toBe(
false
)
expect(m(p, new URL('https://example.com/act123/team5/pic.jpg'))).toBe(
false
)
expect(m(p, new URL('https://example.com/team/pic.jpg'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/sr/pic.jpg'))).toBe(true)
})

it('should match pathname pattern with single asterisk at the end', () => {
const p = {
hostname: 'example.com',
pathname: '/act123/usr*/pic.jpg',
} as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
expect(m(p, new URL('https://sub.example.com'))).toBe(false)
expect(m(p, new URL('https://example.com.uk'))).toBe(false)
expect(m(p, new URL('https://example.com/act123'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/pic'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/picsjpg'))).toBe(false)
expect(m(p, new URL('https://example.com/act123/usr4/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/usr5/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/usr/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/team4/pic.jpg'))).toBe(
false
)
expect(m(p, new URL('https://example.com/act456/team5/pic.jpg'))).toBe(
false
)
expect(m(p, new URL('https://example.com/team/pic.jpg'))).toBe(false)
expect(m(p, new URL('https://sub.example.com/act123/usr6/pic.jpg'))).toBe(
false
)
})

it('should match pathname pattern with double asterisk', () => {
const p = { hostname: 'example.com', pathname: '/act123/**' } as const
expect(m(p, new URL('https://com'))).toBe(false)
expect(m(p, new URL('https://example.com'))).toBe(false)
expect(m(p, new URL('https://sub.example.com'))).toBe(false)
expect(m(p, new URL('https://example.com.uk'))).toBe(false)
expect(m(p, new URL('https://example.com/act123'))).toBe(false)
expect(m(p, new URL('https://example.com/act123'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/usr4'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/usr4/pic'))).toBe(true)
expect(m(p, new URL('https://example.com/act123/usr4/picsjpg'))).toBe(true)
Expand All @@ -166,6 +290,9 @@ describe('matchRemotePattern', () => {
expect(m(p, new URL('https://example.com/act123/team/pic.jpg'))).toBe(true)
expect(m(p, new URL('https://example.com/act456/team/pic.jpg'))).toBe(false)
expect(m(p, new URL('https://example.com/team/pic.jpg'))).toBe(false)
expect(m(p, new URL('https://sub.example.com/act123/team/pic.jpg'))).toBe(
false
)
})

it('should throw when hostname is missing', () => {
Expand All @@ -176,20 +303,6 @@ describe('matchRemotePattern', () => {
)
})

it('should throw when hostname has double asterisk in the middle', () => {
const p = { hostname: 'example.**.com' } as const
expect(() => m(p, new URL('https://example.com'))).toThrow(
'Pattern can only contain ** at start of hostname but found "example.**.com"'
)
})

it('should throw when pathname has double asterisk in the middle', () => {
const p = { hostname: 'example.com', pathname: '/**/img' } as const
expect(() => m(p, new URL('https://example.com'))).toThrow(
'Pattern can only contain ** at end of pathname but found "/**/img"'
)
})

it('should properly work with hasMatch', () => {
const url = new URL('https://example.com')
expect(hasMatch([], [], url)).toBe(false)
Expand Down

0 comments on commit 0d95886

Please sign in to comment.