From 30a43ec794a1b71f37c645f1f1b915090d54bf3e Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Mon, 16 Oct 2023 21:10:49 -0400 Subject: [PATCH] fix #3410: quote asset references in url tokens --- CHANGELOG.md | 4 +++ internal/bundler_tests/bundler_css_test.go | 34 +++++++++++++++++++ .../bundler_tests/snapshots/snapshots_css.txt | 28 +++++++++++++-- .../snapshots/snapshots_default.txt | 18 +++++----- .../snapshots/snapshots_loader.txt | 16 ++++----- internal/css_printer/css_printer.go | 7 ++++ scripts/js-api-tests.js | 2 +- 7 files changed, 88 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3840779938..1467bd43303 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ .foo{font:16px Menlo} ``` +* Fix bundling CSS with asset names containing spaces ([#3410](https://github.com/evanw/esbuild/issues/3410)) + + Assets referenced via CSS `url()` tokens may cause esbuild to generate invalid output when bundling if the file name contains spaces (e.g. `url(image 2.png)`). With this release, esbuild will now quote all bundled asset references in `url()` tokens to avoid this problem. This only affects assets loaded using the `file` and `copy` loaders. + * Update to Unicode 15.1.0 The character tables that determine which characters form valid JavaScript identifiers have been updated from Unicode version 15.0.0 to the newly-released Unicode version 15.1.0. I'm not putting an example in the release notes because all of the new characters will likely just show up as little squares since fonts haven't been updated yet. But you can read https://www.unicode.org/versions/Unicode15.1.0/#Summary for more information about the changes. diff --git a/internal/bundler_tests/bundler_css_test.go b/internal/bundler_tests/bundler_css_test.go index 3fc140efc91..3f5aa28fd38 100644 --- a/internal/bundler_tests/bundler_css_test.go +++ b/internal/bundler_tests/bundler_css_test.go @@ -2617,3 +2617,37 @@ func TestCSSCaseInsensitivity(t *testing.T) { }, }) } + +func TestCSSAssetPathsWithSpacesBundle(t *testing.T) { + css_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.css": ` + a { + background: url(foo.copy); + background: url(foo.file); + } + + /*! The URLs for "foo 2" files must have quotes in the final CSS */ + b { + background: url('foo 2.copy'); + background: url('foo 2.file'); + } + `, + "/foo.file": `...`, + "/foo.copy": `...`, + "/foo 2.file": `...`, + "/foo 2.copy": `...`, + }, + entryPaths: []string{"/entry.css"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.css", + MinifySyntax: true, + ExtensionToLoader: map[string]config.Loader{ + ".css": config.LoaderCSS, + ".file": config.LoaderFile, + ".copy": config.LoaderCopy, + }, + }, + }) +} diff --git a/internal/bundler_tests/snapshots/snapshots_css.txt b/internal/bundler_tests/snapshots/snapshots_css.txt index e7b7d5eb347..330d687fda9 100644 --- a/internal/bundler_tests/snapshots/snapshots_css.txt +++ b/internal/bundler_tests/snapshots/snapshots_css.txt @@ -63,6 +63,28 @@ body { color: blue; } +================================================================================ +TestCSSAssetPathsWithSpacesBundle +---------- /foo-AKINYSFH.copy ---------- +... +---------- /foo-AKINYSFH.file ---------- +... +---------- /foo 2-AKINYSFH.copy ---------- +... +---------- /foo 2-AKINYSFH.file ---------- +... +---------- /out.css ---------- +/* entry.css */ +a { + background: url("./foo-AKINYSFH.copy"); + background: url("./foo-AKINYSFH.file"); +} +/*! The URLs for "foo 2" files must have quotes in the final CSS */ +b { + background: url("./foo 2-AKINYSFH.copy"); + background: url("./foo 2-AKINYSFH.file"); +} + ================================================================================ TestCSSAtImport ---------- /out.css ---------- @@ -1659,7 +1681,7 @@ TestCSSCaseInsensitivity body { BACKGROUND-color: red; width: 50Px; - background-IMAGE: url(./image-AKINYSFH.png); + background-IMAGE: url("./image-AKINYSFH.png"); } } } @@ -2214,12 +2236,12 @@ This is some data. ---------- /out/entry.css ---------- /* one.css */ a { - background: url(./example-GDKWWYFY.data); + background: url("./example-GDKWWYFY.data"); } /* two.css */ b { - background: url(./example-GDKWWYFY.data); + background: url("./example-GDKWWYFY.data"); } /* entry.css */ diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index f30fa33c6fe..cd3227f8f4b 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -3148,8 +3148,8 @@ tNjAtOTVMMTU1IDEwMGwtNDcuNSA0Ny4\ 1IiBmaWxsPSJub25lIiBzdHJva2U9IiM\ xOTE5MTkiIHN0cm9rZS13aWR0aD0iMjQ\ iLz4KPC9zdmc+Cg=="); - cursor: url(./x-TZ25B4WH.file); - cursor: url(./x-UF3O47Y3.copy); + cursor: url("./x-TZ25B4WH.file"); + cursor: url("./x-UF3O47Y3.copy"); cursor: url("data:text/plain;c\ harset=utf-8,...lots of long dat\ a...lots of long data..."); @@ -4110,10 +4110,10 @@ a { background: url(data:image/svg+xml,); } b { - background: url(./file-NVISQQTV.file); + background: url("./file-NVISQQTV.file"); } c { - background: url(./copy-O3Y5SCJE.copy); + background: url("./copy-O3Y5SCJE.copy"); } d { background: url(extern.png); @@ -4357,10 +4357,10 @@ d { "entryPoint": "project/entry.css", "inputs": { "project/entry.css": { - "bytesInOutput": 183 + "bytesInOutput": 187 } }, - "bytes": 230 + "bytes": 234 } } } @@ -4394,7 +4394,7 @@ import("./3333333333333333333333333333333333333333333333333333333333333333333333 ---------- /out/bytesInOutput should be at least 99.css ---------- /* project/bytesInOutput should be at least 99.css */ a { - background: url(./444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444-55DNWN2R.file); + background: url("./444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444-55DNWN2R.file"); } ---------- metafile.json ---------- { @@ -4561,10 +4561,10 @@ a { "entryPoint": "project/bytesInOutput should be at least 99.css", "inputs": { "project/bytesInOutput should be at least 99.css": { - "bytesInOutput": 142 + "bytesInOutput": 144 } }, - "bytes": 196 + "bytes": 198 } } } diff --git a/internal/bundler_tests/snapshots/snapshots_loader.txt b/internal/bundler_tests/snapshots/snapshots_loader.txt index a8437380822..5bcb9ab6171 100644 --- a/internal/bundler_tests/snapshots/snapshots_loader.txt +++ b/internal/bundler_tests/snapshots/snapshots_loader.txt @@ -311,7 +311,7 @@ console.log(x); ---------- /out/src/entry.css ---------- /* Users/user/project/src/entry.css */ body { - background: url(../assets/some.file); + background: url("../assets/some.file"); } ================================================================================ @@ -321,7 +321,7 @@ stuff ---------- /out/src/entry.css ---------- /* Users/user/project/src/entry.css */ body { - background: url(../some-BYATPJRB.file); + background: url("../some-BYATPJRB.file"); } ================================================================================ @@ -666,7 +666,7 @@ x ---------- /out/entries/entry.css ---------- /* src/shared/common.css */ div { - background: url(../common-LSAMBFUD.png); + background: url("../common-LSAMBFUD.png"); } /* src/entries/entry.css */ @@ -674,7 +674,7 @@ div { ---------- /out/entries/other/entry.css ---------- /* src/shared/common.css */ div { - background: url(../../common-LSAMBFUD.png); + background: url("../../common-LSAMBFUD.png"); } /* src/entries/other/entry.css */ @@ -704,7 +704,7 @@ x ---------- /out/entries/entry.css ---------- /* src/entries/entry.css */ div { - background: url(https://example.com/images/image-LSAMBFUD.png); + background: url("https://example.com/images/image-LSAMBFUD.png"); } ================================================================================ @@ -725,7 +725,7 @@ x ---------- /out/entries/entry.css ---------- /* src/entries/entry.css */ div { - background: url(https://example.com/image-LSAMBFUD.png); + background: url("https://example.com/image-LSAMBFUD.png"); } ================================================================================ @@ -746,7 +746,7 @@ x ---------- /out/entries/entry.css ---------- /* src/entries/entry.css */ div { - background: url(../images/image-LSAMBFUD.png); + background: url("../images/image-LSAMBFUD.png"); } ================================================================================ @@ -767,7 +767,7 @@ x ---------- /out/entries/entry.css ---------- /* src/entries/entry.css */ div { - background: url(../image-LSAMBFUD.png); + background: url("../image-LSAMBFUD.png"); } ================================================================================ diff --git a/internal/css_printer/css_printer.go b/internal/css_printer/css_printer.go index 08f805a3e17..2f63184ce04 100644 --- a/internal/css_printer/css_printer.go +++ b/internal/css_printer/css_printer.go @@ -1025,6 +1025,13 @@ func (p *printer) printTokens(tokens []css_ast.Token, opts printTokensOpts) bool var flags printQuotedFlags if record.Flags.Has(ast.ContainsUniqueKey) { flags |= printQuotedNoWrap + + // If the caller will be substituting a path here later using string + // substitution, then we can't be sure that it will form a valid URL + // token when unquoted (e.g. it may contain spaces). So we need to + // quote the unique key here just in case. For more info see this + // issue: https://github.com/evanw/esbuild/issues/3410 + tryToAvoidQuote = false } else if p.options.LineLimit > 0 && p.currentLineLength()+len(text) >= p.options.LineLimit { tryToAvoidQuote = false } diff --git a/scripts/js-api-tests.js b/scripts/js-api-tests.js index 3c5d822c2a1..cf1a23876a4 100644 --- a/scripts/js-api-tests.js +++ b/scripts/js-api-tests.js @@ -945,7 +945,7 @@ export { assert.strictEqual(value.outputFiles, void 0) assert.strictEqual(await readFileAsync(output, 'utf8'), `/* scripts/.js-api-tests/fileLoaderCSS/in.css */ body { - background: url(https://www.example.com/assets/data-BYATPJRB.bin); + background: url("https://www.example.com/assets/data-BYATPJRB.bin"); } `) },