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.

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

fix(assets): make srcset parsing HTML spec compliant (#16323) #18242

Merged
merged 6 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions packages/vite/src/node/__tests__/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,70 @@ describe('processSrcSetSync', () => {
processSrcSetSync('https://anydomain/image.jpg', ({ url }) => url),
).toBe(source)
})

test('should not break URLs with commas in srcSet', async () => {
const source = `
\thttps://example.com/dpr_1,f_auto,fl_progressive,q_auto,w_100/v1/img 1x,
\thttps://example.com/dpr_2,f_auto,fl_progressive,q_auto,w_100/v1/img\t\t2x
`
const result =
'https://example.com/dpr_1,f_auto,fl_progressive,q_auto,w_100/v1/img 1x, https://example.com/dpr_2,f_auto,fl_progressive,q_auto,w_100/v1/img 2x'
expect(processSrcSetSync(source, ({ url }) => url)).toBe(result)
})

test('should not break URLs with commas in image-set-options', async () => {
const source = `url(https://example.com/dpr_1,f_auto,fl_progressive,q_auto,w_100/v1/img) 1x,
url("https://example.com/dpr_2,f_auto,fl_progressive,q_auto,w_100/v1/img")\t\t2x
`
const result =
'url(https://example.com/dpr_1,f_auto,fl_progressive,q_auto,w_100/v1/img) 1x, url("https://example.com/dpr_2,f_auto,fl_progressive,q_auto,w_100/v1/img") 2x'
expect(processSrcSetSync(source, ({ url }) => url)).toBe(result)
})

test('should parse image-set-options with resolution', async () => {
const source = ` "foo.png" 1x,
"foo-2x.png" 2x,
"foo-print.png" 600dpi`
const result = '"foo.png" 1x, "foo-2x.png" 2x, "foo-print.png" 600dpi'
expect(processSrcSetSync(source, ({ url }) => url)).toBe(result)
})

test('should parse image-set-options with type', async () => {
const source = ` "foo.avif" type("image/avif"),
"foo.jpg" type("image/jpeg") `
const result = '"foo.avif" type("image/avif"), "foo.jpg" type("image/jpeg")'
expect(processSrcSetSync(source, ({ url }) => url)).toBe(result)
})

test('should parse image-set-options with linear-gradient', async () => {
const source = `linear-gradient(cornflowerblue, white) 1x,
url("detailed-gradient.png") 3x`
const result =
'linear-gradient(cornflowerblue, white) 1x, url("detailed-gradient.png") 3x'
expect(processSrcSetSync(source, ({ url }) => url)).toBe(result)
})

test('should parse image-set-options with resolution and type specified', async () => {
const source = `url("picture.png")\t1x\t type("image/jpeg"), url("picture.png")\t type("image/jpeg")\t2x`
const result =
'url("picture.png") 1x type("image/jpeg"), url("picture.png") type("image/jpeg") 2x'
expect(processSrcSetSync(source, ({ url }) => url)).toBe(result)
})

test('should capture whole image set options', async () => {
const source = `linear-gradient(cornflowerblue, white) 1x,
url("detailed-gradient.png") 3x`
const expected = [
'linear-gradient(cornflowerblue, white)',
'url("detailed-gradient.png")',
]
const result: string[] = []
processSrcSetSync(source, ({ url }) => {
result.push(url)
return url
})
expect(result).toEqual(expected)
})
})

describe('flattenId', () => {
Expand Down
73 changes: 31 additions & 42 deletions packages/vite/src/node/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -720,36 +720,50 @@ interface ImageCandidate {
url: string
descriptor: string
}
const escapedSpaceCharacters = /(?: |\\t|\\n|\\f|\\r)+/g
const imageSetUrlRE = /^(?:[\w-]+\(.*?\)|'.*?'|".*?"|\S*)/

function joinSrcset(ret: ImageCandidate[]) {
return ret
.map(({ url, descriptor }) => url + (descriptor ? ` ${descriptor}` : ''))
.join(', ')
}

// NOTE: The returned `url` should perhaps be decoded so all handled URLs within Vite are consistently decoded.
// However, this may also require a refactor for `cssReplacer` to accept decoded URLs instead.
function splitSrcSetDescriptor(srcs: string): ImageCandidate[] {
return splitSrcSet(srcs)
.map((s) => {
const src = s.replace(escapedSpaceCharacters, ' ').trim()
const url = imageSetUrlRE.exec(src)?.[0] ?? ''
/**
rarous marked this conversation as resolved.
Show resolved Hide resolved
This regex represents a loose rule of an “image candidate string” and "image set options".

return {
url,
descriptor: src.slice(url.length).trim(),
}
})
.filter(({ url }) => !!url)
@see https://html.spec.whatwg.org/multipage/images.html#srcset-attribute
@see https://drafts.csswg.org/css-images-4/#image-set-notation

The Regex has named capturing groups `url` and `descriptor`.
The `url` group can be:
* any CSS function
* CSS string (single or double-quoted)
* URL string (unquoted)
The `descriptor` is anything after the space and before the comma.
*/
const imageCandidateRegex =
/(?:^|\s)(?<url>[\w-]+\([^)]*\)|"[^"]*"|'[^']*'|[^,]\S*[^,])\s*(?:\s(?<descriptor>\w[^,]+))?(?:,|$)/g
const escapedSpaceCharacters = /(?: |\\t|\\n|\\f|\\r)+/g

export function parseSrcset(string: string): ImageCandidate[] {
const matches = string
.trim()
.replace(escapedSpaceCharacters, ' ')
.replace(/\r?\n/, '')
.replace(/,\s+/, ', ')
.replaceAll(/\s+/g, ' ')
.matchAll(imageCandidateRegex)
return Array.from(matches, ({ groups }) => ({
url: groups?.url?.trim() ?? '',
descriptor: groups?.descriptor?.trim() ?? '',
})).filter(({ url }) => !!url)
}

export function processSrcSet(
srcs: string,
replacer: (arg: ImageCandidate) => Promise<string>,
): Promise<string> {
return Promise.all(
splitSrcSetDescriptor(srcs).map(async ({ url, descriptor }) => ({
parseSrcset(srcs).map(async ({ url, descriptor }) => ({
sapphi-red marked this conversation as resolved.
Show resolved Hide resolved
url: await replacer({ url, descriptor }),
descriptor,
})),
Expand All @@ -761,38 +775,13 @@ export function processSrcSetSync(
replacer: (arg: ImageCandidate) => string,
): string {
return joinSrcset(
splitSrcSetDescriptor(srcs).map(({ url, descriptor }) => ({
parseSrcset(srcs).map(({ url, descriptor }) => ({
url: replacer({ url, descriptor }),
descriptor,
})),
)
}

const cleanSrcSetRE =
/(?:url|image|gradient|cross-fade)\([^)]*\)|"([^"]|(?<=\\)")*"|'([^']|(?<=\\)')*'|data:\w+\/[\w.+-]+;base64,[\w+/=]+|\?\S+,/g
function splitSrcSet(srcs: string) {
const parts: string[] = []
/**
* There could be a ',' inside of:
* - url(data:...)
* - linear-gradient(...)
* - "data:..."
* - data:...
* - query parameter ?...
*/
const cleanedSrcs = srcs.replace(cleanSrcSetRE, blankReplacer)
let startIndex = 0
let splitIndex: number
do {
splitIndex = cleanedSrcs.indexOf(',', startIndex)
parts.push(
srcs.slice(startIndex, splitIndex !== -1 ? splitIndex : undefined),
)
startIndex = splitIndex + 1
} while (splitIndex !== -1)
return parts
}

const windowsDriveRE = /^[A-Z]:/
const replaceWindowsDriveRE = /^([A-Z]):\//
const linuxAbsolutePathRE = /^\/[^/]/
Expand Down