fix: adding quotes when necessary for unquoted sources (#247)

webpack-contrib · Mar 5, 2020 · d0b0150 · d0b0150
1 parent e3727ab
commit d0b0150
Show file tree

Hide file tree

Showing 13 changed files with 788 additions and 180 deletions.
diff --git a/src/plugins/attribute-plugin.js b/src/plugins/attribute-plugin.js
@@ -415,15 +415,20 @@ export default (options) =>
       {
         attributesMeta: {},
         onattribute(name, value) {
-          this.attributesMeta[name] = {
-            // eslint-disable-next-line no-underscore-dangle
-            startIndex: parser._tokenizer._index - value.length,
-          };
+          // eslint-disable-next-line no-underscore-dangle
+          const endIndex = parser._tokenizer._index;
+          const startIndex = endIndex - value.length;
+          const unquoted = html[endIndex] !== '"' && html[endIndex] !== "'";
+
+          this.attributesMeta[name] = { startIndex, unquoted };
         },
         onopentag(tag, attributes) {
           Object.keys(attributes).forEach((attribute) => {
             const value = attributes[attribute];
-            const valueStartIndex = this.attributesMeta[attribute].startIndex;
+            const {
+              startIndex: valueStartIndex,
+              unquoted,
+            } = this.attributesMeta[attribute];
 
             // TODO use code frame for errors
 
@@ -458,7 +463,7 @@ export default (options) =>
 
                 const startIndex = valueStartIndex + source.startIndex;
 
-                sources.push({ startIndex, value: source.value });
+                sources.push({ startIndex, value: source.value, unquoted });
               });
 
               return;
@@ -484,7 +489,7 @@ export default (options) =>
 
             const startIndex = valueStartIndex + source.startIndex;
 
-            sources.push({ startIndex, value: source.value });
+            sources.push({ startIndex, value: source.value, unquoted });
           });
 
           this.attributesMeta = {};
@@ -510,7 +515,8 @@ export default (options) =>
     let index = 0;
 
     for (const source of sources) {
-      const uri = parse(source.value);
+      const { value, startIndex, unquoted } = source;
+      const uri = parse(value);
 
       if (typeof uri.hash !== 'undefined') {
         uri.hash = null;
@@ -526,16 +532,17 @@ export default (options) =>
           type: 'attribute',
           replacementName,
           source: decodeURIComponent(source.value),
+          unquoted,
         },
       });
 
       // eslint-disable-next-line no-param-reassign
       html =
-        html.substr(0, source.startIndex + offset) +
+        html.substr(0, startIndex + offset) +
         replacementName +
-        html.substr(source.startIndex + source.value.length + offset);
+        html.substr(startIndex + value.length + offset);
 
-      offset += replacementName.length - source.value.length;
+      offset += replacementName.length - value.length;
       index += 1;
     }
 

diff --git a/src/runtime/getUrl.js b/src/runtime/getUrl.js
@@ -1,14 +1,14 @@
-module.exports = (url) => {
+module.exports = (url, maybeNeedQuotes) => {
   // eslint-disable-next-line no-underscore-dangle, no-param-reassign
   url = url && url.__esModule ? url.default : url;
 
   if (typeof url !== 'string') {
     return url;
   }
 
-  // if (/[\t\n\f\r "'=<>`]/.test(url)) {
-  //  return `"${url}"`;
-  // }
+  if (maybeNeedQuotes && /[\t\n\f\r "'=<>`]/.test(url)) {
+    return `"${url}"`;
+  }
 
   return url;
 };
diff --git a/src/utils.js b/src/utils.js
@@ -71,11 +71,14 @@ export function getExportCode(html, replacers, options) {
   }
 
   for (const replacer of replacers) {
-    const { replacementName } = replacer;
+    const { replacementName, unquoted } = replacer;
 
     exportCode = exportCode.replace(
       new RegExp(replacementName, 'g'),
-      () => `" + ___HTML_LOADER_GET_URL_IMPORT___(${replacementName}) + "`
+      () =>
+        `" + ___HTML_LOADER_GET_URL_IMPORT___(${replacementName}${
+          unquoted ? ', true' : ''
+        }) + "`
     );
   }
 

diff --git a/test/__snapshots__/attributes-option.test.js.snap b/test/__snapshots__/attributes-option.test.js.snap
diff --git a/test/__snapshots__/esModule-option.test.js.snap b/test/__snapshots__/esModule-option.test.js.snap
diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap
diff --git a/test/__snapshots__/minimize-option.test.js.snap b/test/__snapshots__/minimize-option.test.js.snap
diff --git a/test/__snapshots__/root-option.test.js.snap b/test/__snapshots__/root-option.test.js.snap
diff --git a/test/fixtures/alias-image.png b/test/fixtures/alias-image.png
diff --git a/test/fixtures/simple.html b/test/fixtures/simple.html
@@ -210,3 +210,17 @@ <h2>An Ordered HTML List</h2>
 <script>
   function test() {}
 </script>
+
+<img src=image.png />
+<img src="image image.png" />
+<img src='image image.png' />
+<img src=~aliasImageWithSpace />
+<img srcset=image.png />
+<img srcset="image%20image.png" />
+<img srcset='image%20image.png' />
+<img srcset=~aliasImageWithSpace />
+<img srcset="image.png 480w, image%20image.png 640w, ~aliasImageWithSpace 800w" sizes="(max-width: 600px) 480px, 800px" src="image.png" alt="Elva dressed as a fairy">
+<img src      =       image.png />
+<img src      =      ~aliasImageWithSpace      />
+<img src      =      "~aliasImageWithSpace"      />
+<img src      =      '~aliasImageWithSpace'      />
diff --git a/test/helpers/getCompiler.js b/test/helpers/getCompiler.js
@@ -54,7 +54,11 @@ export default (fixture, loaderOptions = {}, config = {}) => {
     },
     resolve: {
       alias: {
-        aliasImg: path.resolve(__dirname, '../fixtures/alias-image.png'),
+        aliasImg: path.resolve(__dirname, '../fixtures/image.png'),
+        aliasImageWithSpace: path.resolve(
+          __dirname,
+          '../fixtures/image image.png'
+        ),
       },
     },
     plugins: [],

diff --git a/test/runtime/__snapshots__/getUrl.test.js.snap b/test/runtime/__snapshots__/getUrl.test.js.snap
@@ -37,3 +37,33 @@ exports[`getUrl should work 14`] = `"image<image.png"`;
 exports[`getUrl should work 15`] = `"image\`image.png"`;
 
 exports[`getUrl should work 16`] = `"image.png"`;
+
+exports[`getUrl should work 17`] = `"\\"image	image.png\\""`;
+
+exports[`getUrl should work 18`] = `
+"\\"image
+image.png\\""
+`;
+
+exports[`getUrl should work 19`] = `"\\"imageimage.png\\""`;
+
+exports[`getUrl should work 20`] = `
+"\\"image
+image.png\\""
+`;
+
+exports[`getUrl should work 21`] = `"\\"image image.png\\""`;
+
+exports[`getUrl should work 22`] = `"\\"image\\"image.png\\""`;
+
+exports[`getUrl should work 23`] = `"\\"image'image.png\\""`;
+
+exports[`getUrl should work 24`] = `"\\"image=image.png\\""`;
+
+exports[`getUrl should work 25`] = `"\\"image>image.png\\""`;
+
+exports[`getUrl should work 26`] = `"\\"image<image.png\\""`;
+
+exports[`getUrl should work 27`] = `"\\"image\`image.png\\""`;
+
+exports[`getUrl should work 28`] = `"image.png"`;
diff --git a/test/runtime/getUrl.test.js b/test/runtime/getUrl.test.js
@@ -18,6 +18,18 @@ describe('getUrl', () => {
     expect(getUrl('image>image.png')).toMatchSnapshot();
     expect(getUrl('image<image.png')).toMatchSnapshot();
     expect(getUrl('image`image.png')).toMatchSnapshot();
+    expect(getUrl('image.png', true)).toMatchSnapshot();
+    expect(getUrl('image\timage.png', true)).toMatchSnapshot();
+    expect(getUrl('image\nimage.png', true)).toMatchSnapshot();
+    expect(getUrl('image\fimage.png', true)).toMatchSnapshot();
+    expect(getUrl('image\rimage.png', true)).toMatchSnapshot();
+    expect(getUrl('image image.png', true)).toMatchSnapshot();
+    expect(getUrl('image"image.png', true)).toMatchSnapshot();
+    expect(getUrl("image'image.png", true)).toMatchSnapshot();
+    expect(getUrl('image=image.png', true)).toMatchSnapshot();
+    expect(getUrl('image>image.png', true)).toMatchSnapshot();
+    expect(getUrl('image<image.png', true)).toMatchSnapshot();
+    expect(getUrl('image`image.png', true)).toMatchSnapshot();
     expect(
       getUrl({ default: 'image.png', __esModule: true })
     ).toMatchSnapshot();