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.

Sign up for GitHub

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

Jump to bottom

wip: improve remotePatterns wildcard matching #37026

Closed
wants to merge 1 commit into from
Closed

wip: improve remotePatterns wildcard matching #37026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 18 additions & 8 deletions packages/next/shared/lib/match-remote-pattern.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,15 @@ export function matchRemotePattern(pattern: RemotePattern, url: URL): boolean {
}
break
}
if (patternParts[i] === '*') {
// Single asterisk means "match this part" so we can
// continue to the next part of the loop
continue
const patternPart = patternParts[i] || ''
const actualPart = actualParts[i] || ''
if (patternPart.includes('*') && !patternPart.includes('**')) {
// Single asterisk means "zero or more matches" similar to regex
// so we can test for a match and continue to the next part of the loop
// TODO: Don't use regex since its not safe
if (new RegExp(patternPart.replaceAll('*', '.*')).test(actualPart)) {
continue
}
}
if (patternParts[i] !== actualParts[i]) {
return false
Expand Down Expand Up @@ -59,10 +64,15 @@ export function matchRemotePattern(pattern: RemotePattern, url: URL): boolean {
}
break
}
if (patternParts[i] === '*') {
// Single asterisk means "match this subdomain" so we can
// continue to the next part of the loop
continue
const patternPart = patternParts[i] || ''
const actualPart = actualParts[i] || ''
if (patternPart.includes('*') && !patternPart.includes('**')) {
// Single asterisk means "zero or more matches" similar to regex
// so we can test for a match and continue to the next part of the loop
// TODO: Don't use regex since its not safe
if (new RegExp(patternPart.replaceAll('*', '.*')).test(actualPart)) {
continue
}
}
if (patternParts[i] !== actualParts[i]) {
return false
Expand Down
125 changes: 123 additions & 2 deletions test/unit/image-optimizer/match-remote-pattern.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
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,6 +198,79 @@ 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/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/act456/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/act456/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)
})

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)
Expand Down