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: sanitize asset filenames #9737

Merged
merged 3 commits into from Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
19 changes: 18 additions & 1 deletion packages/vite/src/node/plugins/asset.ts
Expand Up @@ -324,7 +324,7 @@ export function assetFileNamesToFileName(
return hash

case '[name]':
return name
return sanitizeFileName(name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use output.sanitizeFileName here to support custom functions by the user? (and also avoid the need to replicate the default in our code)
https://rollupjs.org/guide/en/#outputsanitizefilename

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussing with @dominikg in Vite Land, we don't have access to Rollup's default. We can also leave support for custom sanitize functions for another PR. I think we can merge this PR and then work on extending support later.

}
throw new Error(
`invalid placeholder ${placeholder} in assetFileNames "${assetFileNames}"`
Expand All @@ -335,6 +335,23 @@ export function assetFileNamesToFileName(
return fileName
}

// taken from https://github.com/rollup/rollup/blob/a8647dac0fe46c86183be8596ef7de25bc5b4e4b/src/utils/sanitizeFileName.ts
// https://datatracker.ietf.org/doc/html/rfc2396
// eslint-disable-next-line no-control-regex
const INVALID_CHAR_REGEX = /[\x00-\x1F\x7F<>*#"{}|^[\]`;?:&=+$,]/g
const DRIVE_LETTER_REGEX = /^[a-z]:/i
function sanitizeFileName(name: string): string {
const match = DRIVE_LETTER_REGEX.exec(name)
const driveLetter = match ? match[0] : ''

// A `:` is only allowed as part of a windows drive letter (ex: C:\foo)
// Otherwise, avoid them because they can refer to NTFS alternate data streams.
return (
driveLetter +
name.substr(driveLetter.length).replace(INVALID_CHAR_REGEX, '_')
)
}

export const publicAssetUrlCache = new WeakMap<
ResolvedConfig,
// hash -> url
Expand Down
3 changes: 3 additions & 0 deletions playground/assets-sanitize/+circle.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 20 additions & 0 deletions playground/assets-sanitize/__tests__/assets-sanitize.spec.ts
@@ -0,0 +1,20 @@
import { expect, test } from 'vitest'
import { getBg, isBuild, listAssets, page } from '~utils'

if (!isBuild) {
test('importing asset with special char in filename works', async () => {
expect(await getBg('.circle-bg')).toContain('+circle.svg')
expect(await page.textContent('.circle-bg')).toMatch('+circle.svg')
})
} else {
const expected_asset_name_RE = /_circle\.[\w]+\.svg/
test('importing asset with special char in filename works', async () => {
expect(await getBg('.circle-bg')).toMatch(expected_asset_name_RE)
expect(await page.textContent('.circle-bg')).toMatch(expected_asset_name_RE)
})
test('asset with special char in filename gets sanitized', async () => {
const svgs = listAssets().filter((a) => a.endsWith('.svg'))
expect(svgs[0]).toMatch(expected_asset_name_RE)
expect(svgs.length).toBe(1)
})
}
6 changes: 6 additions & 0 deletions playground/assets-sanitize/index.html
@@ -0,0 +1,6 @@
<script type="module" src="./index.js"></script>
<h1>test element below should show a circle and it's url</h1>
<div
class="circle-bg"
style="background-repeat: no-repeat; padding-left: 2rem"
></div>
4 changes: 4 additions & 0 deletions playground/assets-sanitize/index.js
@@ -0,0 +1,4 @@
import svg from './+circle.svg'
const el = document.body.querySelector('.circle-bg')
el.style.backgroundImage = `url(${svg})`
el.textContent = svg
11 changes: 11 additions & 0 deletions playground/assets-sanitize/package.json
@@ -0,0 +1,11 @@
{
"name": "test-assets-sanitize",
"private": true,
"version": "0.0.0",
"scripts": {
"dev": "vite",
"build": "vite build",
"debug": "node --inspect-brk ../../packages/vite/bin/vite",
"preview": "vite preview"
}
}
11 changes: 11 additions & 0 deletions playground/assets-sanitize/vite.config.js
@@ -0,0 +1,11 @@
const { defineConfig } = require('vite')

module.exports = defineConfig({
build: {
//speed up build
minify: false,
target: 'esnext',
assetsInlineLimit: 0,
manifest: true
}
})