From 483258c250606c852d0c18573f850e294c3aca38 Mon Sep 17 00:00:00 2001 From: Shinyaigeek Date: Sun, 20 Mar 2022 14:33:31 +0900 Subject: [PATCH 1/8] Feature: escape deps url on CSSPackager --- packages/core/utils/src/replaceBundleReferences.js | 3 +++ packages/transformers/css/src/CSSTransformer.js | 13 ++++++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/core/utils/src/replaceBundleReferences.js b/packages/core/utils/src/replaceBundleReferences.js index 8161ac299ad..de299322667 100644 --- a/packages/core/utils/src/replaceBundleReferences.js +++ b/packages/core/utils/src/replaceBundleReferences.js @@ -191,6 +191,9 @@ export function getURLReplacement({ let placeholder = dependency.meta?.placeholder ?? dependency.id; invariant(typeof placeholder === 'string'); + + to = to.replace(/"/g, '\\"'); + return { from: placeholder, to, diff --git a/packages/transformers/css/src/CSSTransformer.js b/packages/transformers/css/src/CSSTransformer.js index 58f1638f331..44e4f7268d4 100644 --- a/packages/transformers/css/src/CSSTransformer.js +++ b/packages/transformers/css/src/CSSTransformer.js @@ -83,12 +83,15 @@ export default (new Transformer({ symbols: new Map([['*', {local: '*', isWeak: true, loc}]]), }); } else if (dep.type === 'url') { - asset.addURLDependency(dep.url, { - loc, - meta: { - placeholder: dep.placeholder, + asset.addURLDependency( + dep.url.replace(/\\"/g, '"').replace(/\\'/g, "'"), + { + loc, + meta: { + placeholder: dep.placeholder, + }, }, - }); + ); } } } From b36d7c23e528039288a2ca1441d173bc839878d7 Mon Sep 17 00:00:00 2001 From: Shinyaigeek Date: Sun, 20 Mar 2022 14:48:38 +0900 Subject: [PATCH 2/8] Update: escape single quote --- packages/core/utils/src/replaceBundleReferences.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/utils/src/replaceBundleReferences.js b/packages/core/utils/src/replaceBundleReferences.js index de299322667..ce5c9db8a76 100644 --- a/packages/core/utils/src/replaceBundleReferences.js +++ b/packages/core/utils/src/replaceBundleReferences.js @@ -61,7 +61,7 @@ export function replaceURLReferences({ if (resolved == null) { replacements.set(placeholder, { from: placeholder, - to: dependency.specifier, + to: dependency.specifier.replace(/"/g, '\\"').replace(/'/g, "\\'"), }); continue; } @@ -192,7 +192,7 @@ export function getURLReplacement({ let placeholder = dependency.meta?.placeholder ?? dependency.id; invariant(typeof placeholder === 'string'); - to = to.replace(/"/g, '\\"'); + to = to.replace(/"/g, '\\"').replace(/'/g, "\\'"); return { from: placeholder, From e11d4006f627a36161a55838f4dd61f3b4fa68e9 Mon Sep 17 00:00:00 2001 From: Shinyaigeek Date: Sun, 20 Mar 2022 14:50:20 +0900 Subject: [PATCH 3/8] Chore: add comment for CSSTransformer --- packages/transformers/css/src/CSSTransformer.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/transformers/css/src/CSSTransformer.js b/packages/transformers/css/src/CSSTransformer.js index 44e4f7268d4..fc79bad889f 100644 --- a/packages/transformers/css/src/CSSTransformer.js +++ b/packages/transformers/css/src/CSSTransformer.js @@ -84,6 +84,7 @@ export default (new Transformer({ }); } else if (dep.type === 'url') { asset.addURLDependency( + // when URL is escaped, parcel resolver cannot resolve URL, so we need to unescape it in transformer dep.url.replace(/\\"/g, '"').replace(/\\'/g, "'"), { loc, From c2fafb3777348656cb70a0904965e7b0a68eab4e Mon Sep 17 00:00:00 2001 From: Shinyaigeek Date: Sun, 20 Mar 2022 15:25:02 +0900 Subject: [PATCH 4/8] Update: not replace single quote --- packages/core/utils/src/replaceBundleReferences.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/utils/src/replaceBundleReferences.js b/packages/core/utils/src/replaceBundleReferences.js index ce5c9db8a76..ba32090266f 100644 --- a/packages/core/utils/src/replaceBundleReferences.js +++ b/packages/core/utils/src/replaceBundleReferences.js @@ -61,7 +61,7 @@ export function replaceURLReferences({ if (resolved == null) { replacements.set(placeholder, { from: placeholder, - to: dependency.specifier.replace(/"/g, '\\"').replace(/'/g, "\\'"), + to: dependency.specifier.replace(/"/g, '\\"'), }); continue; } @@ -192,7 +192,7 @@ export function getURLReplacement({ let placeholder = dependency.meta?.placeholder ?? dependency.id; invariant(typeof placeholder === 'string'); - to = to.replace(/"/g, '\\"').replace(/'/g, "\\'"); + to = to.replace(/"/g, '\\"'); return { from: placeholder, From 6c954f181a78ca0c24a98fc879a4c6622f1e8d18 Mon Sep 17 00:00:00 2001 From: Shinyaigeek Date: Sun, 20 Mar 2022 15:28:16 +0900 Subject: [PATCH 5/8] Chore: add test case for CSS quote handling --- packages/core/integration-tests/test/css.js | 8 ++++++++ .../test/integration/css-url-quote/index.css | 5 +++++ .../test/integration/css-url-quote/index.js | 5 +++++ 3 files changed, 18 insertions(+) create mode 100644 packages/core/integration-tests/test/integration/css-url-quote/index.css create mode 100644 packages/core/integration-tests/test/integration/css-url-quote/index.js diff --git a/packages/core/integration-tests/test/css.js b/packages/core/integration-tests/test/css.js index dcb3adab669..d55ac9bbc03 100644 --- a/packages/core/integration-tests/test/css.js +++ b/packages/core/integration-tests/test/css.js @@ -244,6 +244,14 @@ describe('css', () => { ); }); + it('should handle quote in CSS URL correctly', async function () { + await bundle(path.join(__dirname, '/integration/css-url-quote/index.css')); + + let css = await outputFS.readFile(path.join(distDir, 'index.css'), 'utf8'); + + assert(css.includes('url("data:image/svg+xml;utf8,with quote \\"");')); + }); + it('should ignore url() with IE behavior specifiers', async function () { let b = await bundle( path.join(__dirname, '/integration/css-url-behavior/index.css'), diff --git a/packages/core/integration-tests/test/integration/css-url-quote/index.css b/packages/core/integration-tests/test/integration/css-url-quote/index.css new file mode 100644 index 00000000000..a69fce1fdd8 --- /dev/null +++ b/packages/core/integration-tests/test/integration/css-url-quote/index.css @@ -0,0 +1,5 @@ +.quotes { + background-image: url('data:image/svg+xml;utf8,with quote "'); + width: 100px; + height: 100px; +} diff --git a/packages/core/integration-tests/test/integration/css-url-quote/index.js b/packages/core/integration-tests/test/integration/css-url-quote/index.js new file mode 100644 index 00000000000..14400a424cd --- /dev/null +++ b/packages/core/integration-tests/test/integration/css-url-quote/index.js @@ -0,0 +1,5 @@ +require('./index.css'); + +module.exports = function () { + return 2; +}; From 43da65b22361fed6514970d2b73d0dfe8fbceb8c Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sun, 20 Mar 2022 12:41:52 -0400 Subject: [PATCH 6/8] Refactor and also escape urls in other file types --- packages/core/integration-tests/test/css.js | 6 +++++- .../test/integration/css-url-quote/index.css | 2 +- packages/core/utils/src/replaceBundleReferences.js | 11 +++++++---- packages/packagers/css/src/CSSPackager.js | 7 ++++++- packages/packagers/html/src/HTMLPackager.js | 1 + packages/packagers/js/src/index.js | 1 + packages/packagers/raw-url/src/RawUrlPackager.js | 1 + packages/packagers/svg/src/SVGPackager.js | 1 + packages/packagers/xml/src/XMLPackager.js | 1 + 9 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/core/integration-tests/test/css.js b/packages/core/integration-tests/test/css.js index d55ac9bbc03..f0b10a054bc 100644 --- a/packages/core/integration-tests/test/css.js +++ b/packages/core/integration-tests/test/css.js @@ -249,7 +249,11 @@ describe('css', () => { let css = await outputFS.readFile(path.join(distDir, 'index.css'), 'utf8'); - assert(css.includes('url("data:image/svg+xml;utf8,with quote \\"");')); + assert( + css.includes( + 'url("data:image/svg+xml;utf8,with quote \\" and escape \\\\");', + ), + ); }); it('should ignore url() with IE behavior specifiers', async function () { diff --git a/packages/core/integration-tests/test/integration/css-url-quote/index.css b/packages/core/integration-tests/test/integration/css-url-quote/index.css index a69fce1fdd8..0da4f256ed0 100644 --- a/packages/core/integration-tests/test/integration/css-url-quote/index.css +++ b/packages/core/integration-tests/test/integration/css-url-quote/index.css @@ -1,5 +1,5 @@ .quotes { - background-image: url('data:image/svg+xml;utf8,with quote "'); + background-image: url('data:image/svg+xml;utf8,with quote " and escape \\'); width: 100px; height: 100px; } diff --git a/packages/core/utils/src/replaceBundleReferences.js b/packages/core/utils/src/replaceBundleReferences.js index ba32090266f..e31ef2b098b 100644 --- a/packages/core/utils/src/replaceBundleReferences.js +++ b/packages/core/utils/src/replaceBundleReferences.js @@ -33,6 +33,7 @@ export function replaceURLReferences({ bundleGraph, contents, map, + getReplacement, relative = true, }: {| bundle: NamedBundle, @@ -40,6 +41,7 @@ export function replaceURLReferences({ contents: string, relative?: boolean, map?: ?SourceMap, + getReplacement: string => string, |}): {|+contents: string, +map: ?SourceMap|} { let replacements = new Map(); let urlDependencies = []; @@ -61,7 +63,7 @@ export function replaceURLReferences({ if (resolved == null) { replacements.set(placeholder, { from: placeholder, - to: dependency.specifier.replace(/"/g, '\\"'), + to: getReplacement(dependency.specifier), }); continue; } @@ -79,6 +81,7 @@ export function replaceURLReferences({ fromBundle: bundle, toBundle: resolved, relative, + getReplacement, }), ); } @@ -156,11 +159,13 @@ export function getURLReplacement({ fromBundle, toBundle, relative, + getReplacement, }: {| dependency: Dependency, fromBundle: NamedBundle, toBundle: NamedBundle, relative: boolean, + getReplacement?: string => string, |}): {|from: string, to: string|} { let to; @@ -192,11 +197,9 @@ export function getURLReplacement({ let placeholder = dependency.meta?.placeholder ?? dependency.id; invariant(typeof placeholder === 'string'); - to = to.replace(/"/g, '\\"'); - return { from: placeholder, - to, + to: getReplacement ? getReplacement(to) : to, }; } diff --git a/packages/packagers/css/src/CSSPackager.js b/packages/packagers/css/src/CSSPackager.js index 23be7bae163..ae2fbbef49b 100644 --- a/packages/packagers/css/src/CSSPackager.js +++ b/packages/packagers/css/src/CSSPackager.js @@ -129,6 +129,7 @@ export default (new Packager({ bundleGraph, contents, map, + getReplacement: escapeString, })); return replaceInlineReferences({ @@ -138,7 +139,7 @@ export default (new Packager({ getInlineBundleContents, getInlineReplacement: (dep, inlineType, contents) => ({ from: getSpecifier(dep), - to: contents, + to: escapeString(contents), }), map, }); @@ -153,6 +154,10 @@ export function getSpecifier(dep: Dependency): string { return dep.id; } +function escapeString(contents: string): string { + return contents.replace(/(["\\])/g, '\\$1'); +} + async function processCSSModule( options, logger, diff --git a/packages/packagers/html/src/HTMLPackager.js b/packages/packagers/html/src/HTMLPackager.js index b30faf3a110..f92089f5f12 100644 --- a/packages/packagers/html/src/HTMLPackager.js +++ b/packages/packagers/html/src/HTMLPackager.js @@ -74,6 +74,7 @@ export default (new Packager({ bundleGraph, contents: html, relative: false, + getReplacement: contents => contents.replace(/"/g, '"'), }); return replaceInlineReferences({ diff --git a/packages/packagers/js/src/index.js b/packages/packagers/js/src/index.js index 58c96fa4fb8..73f1ebc0aa6 100644 --- a/packages/packagers/js/src/index.js +++ b/packages/packagers/js/src/index.js @@ -72,6 +72,7 @@ export default (new Packager({ bundleGraph, contents, map, + getReplacement: s => JSON.stringify(s).slice(1, -1), })); } diff --git a/packages/packagers/raw-url/src/RawUrlPackager.js b/packages/packagers/raw-url/src/RawUrlPackager.js index cd73ce2bac2..e6641bf415b 100644 --- a/packages/packagers/raw-url/src/RawUrlPackager.js +++ b/packages/packagers/raw-url/src/RawUrlPackager.js @@ -17,6 +17,7 @@ export default (new Packager({ bundleGraph, contents: await assets[0].getCode(), relative: false, + getReplacement: s => s, }); return {contents}; }, diff --git a/packages/packagers/svg/src/SVGPackager.js b/packages/packagers/svg/src/SVGPackager.js index a2dd81e9004..3ee06bd7d20 100644 --- a/packages/packagers/svg/src/SVGPackager.js +++ b/packages/packagers/svg/src/SVGPackager.js @@ -58,6 +58,7 @@ export default (new Packager({ bundleGraph, contents: svg, relative: false, + getReplacement: contents => contents.replace(/"/g, '"'), }); return replaceInlineReferences({ diff --git a/packages/packagers/xml/src/XMLPackager.js b/packages/packagers/xml/src/XMLPackager.js index 7e267a2fb41..290c0c73ad9 100644 --- a/packages/packagers/xml/src/XMLPackager.js +++ b/packages/packagers/xml/src/XMLPackager.js @@ -68,6 +68,7 @@ export default (new Packager({ bundleGraph, contents: code, relative: false, + getReplacement: contents => contents.replace(/"/g, '"'), }); return replaceInlineReferences({ From 03ad4ea8c3b646a1209aab3114af447aff9424e1 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sun, 20 Mar 2022 12:44:45 -0400 Subject: [PATCH 7/8] Revert transformer change --- packages/transformers/css/src/CSSTransformer.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/transformers/css/src/CSSTransformer.js b/packages/transformers/css/src/CSSTransformer.js index fc79bad889f..58f1638f331 100644 --- a/packages/transformers/css/src/CSSTransformer.js +++ b/packages/transformers/css/src/CSSTransformer.js @@ -83,16 +83,12 @@ export default (new Transformer({ symbols: new Map([['*', {local: '*', isWeak: true, loc}]]), }); } else if (dep.type === 'url') { - asset.addURLDependency( - // when URL is escaped, parcel resolver cannot resolve URL, so we need to unescape it in transformer - dep.url.replace(/\\"/g, '"').replace(/\\'/g, "'"), - { - loc, - meta: { - placeholder: dep.placeholder, - }, + asset.addURLDependency(dep.url, { + loc, + meta: { + placeholder: dep.placeholder, }, - ); + }); } } } From 39d40fe26b6210e0b5623dc60b93a95081103b30 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sun, 20 Mar 2022 14:05:06 -0400 Subject: [PATCH 8/8] Make it not a breaking change --- packages/core/utils/src/replaceBundleReferences.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/utils/src/replaceBundleReferences.js b/packages/core/utils/src/replaceBundleReferences.js index e31ef2b098b..25eaf2c996c 100644 --- a/packages/core/utils/src/replaceBundleReferences.js +++ b/packages/core/utils/src/replaceBundleReferences.js @@ -33,7 +33,7 @@ export function replaceURLReferences({ bundleGraph, contents, map, - getReplacement, + getReplacement = s => s, relative = true, }: {| bundle: NamedBundle, @@ -41,7 +41,7 @@ export function replaceURLReferences({ contents: string, relative?: boolean, map?: ?SourceMap, - getReplacement: string => string, + getReplacement?: string => string, |}): {|+contents: string, +map: ?SourceMap|} { let replacements = new Map(); let urlDependencies = [];