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

vitejs · Oct 7, 2024 · 0e6d4a5 · 0e6d4a5
1 parent 007773b
commit 0e6d4a5
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 42 deletions.
diff --git a/packages/vite/src/node/__tests__/utils.spec.ts b/packages/vite/src/node/__tests__/utils.spec.ts
@@ -365,6 +365,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', () => {

diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts
@@ -716,36 +716,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] ?? ''
+/**
+ 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 }) => ({
       url: await replacer({ url, descriptor }),
       descriptor,
     })),
@@ -757,38 +771,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 = /^\/[^/]/