From 4d473d4067b337e11ecb51b458767731736b5234 Mon Sep 17 00:00:00 2001
From: Bogdan Chadkin <trysound@yandex.ru>
Date: Wed, 3 Mar 2021 12:25:33 +0300
Subject: [PATCH] Forbid invalid <style> type attribute

This fixes one more case in regression tests.
Also I added support for description in test cases.
This will let make tests more atomic.
---
 plugins/inlineStyles.js          | 370 +++++++++++++++++--------------
 test/plugins/_index.js           |  14 +-
 test/plugins/inlineStyles.20.svg |  31 +++
 3 files changed, 238 insertions(+), 177 deletions(-)
 create mode 100644 test/plugins/inlineStyles.20.svg

diff --git a/plugins/inlineStyles.js b/plugins/inlineStyles.js
index 605740b78..af94f5e3f 100644
--- a/plugins/inlineStyles.js
+++ b/plugins/inlineStyles.js
@@ -5,17 +5,16 @@ exports.type = 'full';
 exports.active = true;
 
 exports.params = {
-    onlyMatchedOnce: true,
-    removeMatchedSelectors: true,
-    useMqs: ['', 'screen'],
-    usePseudos: ['']
+  onlyMatchedOnce: true,
+  removeMatchedSelectors: true,
+  useMqs: ['', 'screen'],
+  usePseudos: [''],
 };
 
 exports.description = 'inline styles (additional options)';
 
-
 var csstree = require('css-tree'),
-    cssTools = require('../lib/css-tools');
+  cssTools = require('../lib/css-tools');
 
 /**
  * Moves + merges styles from style elements to element styles
@@ -41,202 +40,229 @@ var csstree = require('css-tree'),
  *
  * @author strarsis <strarsis@gmail.com>
  */
-exports.fn = function(document, opts) {
-
-    // collect <style/>s
-    var styleEls = document.querySelectorAll('style');
+exports.fn = function (document, opts) {
+  // collect <style/>s
+  var styleEls = document.querySelectorAll('style');
 
-    //no <styles/>s, nothing to do
-    if (styleEls === null) {
-        return document;
+  //no <styles/>s, nothing to do
+  if (styleEls === null) {
+    return document;
+  }
+
+  var styles = [],
+    selectors = [];
+
+  for (var styleEl of styleEls) {
+    // values other than the empty string or text/css are not used
+    if (
+      styleEl.hasAttr('type') &&
+      styleEl.attr('type').value !== '' &&
+      styleEl.attr('type').value !== 'text/css'
+    ) {
+      continue;
     }
-
-    var styles = [],
-        selectors = [];
-
-    for (var styleEl of styleEls) {
-        if (styleEl.isEmpty() || styleEl.closestElem('foreignObject')) {
-            // skip empty <style/>s or <foreignObject> content.
-            continue;
-        }
-
-        var cssStr = cssTools.getCssStr(styleEl);
-
-        // collect <style/>s and their css ast
-        var cssAst = {};
-        try {
-            cssAst = csstree.parse(cssStr, {
-                parseValue: false,
-                parseCustomProperty: false
-            });
-        } catch (parseError) {
-            // console.warn('Warning: Parse error of styles of <style/> element, skipped. Error details: ' + parseError);
-            continue;
-        }
-
-        styles.push({
-            styleEl: styleEl,
-            cssAst: cssAst
-        });
-
-        selectors = selectors.concat(cssTools.flattenToSelectors(cssAst));
+    // skip empty <style/>s or <foreignObject> content.
+    if (styleEl.isEmpty() || styleEl.closestElem('foreignObject')) {
+      continue;
     }
 
+    var cssStr = cssTools.getCssStr(styleEl);
+
+    // collect <style/>s and their css ast
+    var cssAst = {};
+    try {
+      cssAst = csstree.parse(cssStr, {
+        parseValue: false,
+        parseCustomProperty: false,
+      });
+    } catch (parseError) {
+      // console.warn('Warning: Parse error of styles of <style/> element, skipped. Error details: ' + parseError);
+      continue;
+    }
 
-    // filter for mediaqueries to be used or without any mediaquery
-    var selectorsMq = cssTools.filterByMqs(selectors, opts.useMqs);
-
-
-    // filter for pseudo elements to be used
-    var selectorsPseudo = cssTools.filterByPseudos(selectorsMq, opts.usePseudos);
-
-    // remove PseudoClass from its SimpleSelector for proper matching
-    cssTools.cleanPseudos(selectorsPseudo);
+    styles.push({
+      styleEl: styleEl,
+      cssAst: cssAst,
+    });
 
+    selectors = selectors.concat(cssTools.flattenToSelectors(cssAst));
+  }
 
-    // stable sort selectors
-    var sortedSelectors = cssTools.sortSelectors(selectorsPseudo).reverse();
+  // filter for mediaqueries to be used or without any mediaquery
+  var selectorsMq = cssTools.filterByMqs(selectors, opts.useMqs);
 
+  // filter for pseudo elements to be used
+  var selectorsPseudo = cssTools.filterByPseudos(selectorsMq, opts.usePseudos);
 
-    var selector,
-        selectedEl;
+  // remove PseudoClass from its SimpleSelector for proper matching
+  cssTools.cleanPseudos(selectorsPseudo);
 
-    // match selectors
-    for (selector of sortedSelectors) {
-        var selectorStr = csstree.generate(selector.item.data),
-            selectedEls = null;
+  // stable sort selectors
+  var sortedSelectors = cssTools.sortSelectors(selectorsPseudo).reverse();
 
-        try {
-            selectedEls = document.querySelectorAll(selectorStr);
-        } catch (selectError) {
-            // console.warn('Warning: Syntax error when trying to select \n\n' + selectorStr + '\n\n, skipped. Error details: ' + selectError);
-            continue;
-        }
+  var selector, selectedEl;
 
-        if (selectedEls === null) {
-            // nothing selected
-            continue;
-        }
+  // match selectors
+  for (selector of sortedSelectors) {
+    var selectorStr = csstree.generate(selector.item.data),
+      selectedEls = null;
 
-        selector.selectedEls = selectedEls;
+    try {
+      selectedEls = document.querySelectorAll(selectorStr);
+    } catch (selectError) {
+      // console.warn('Warning: Syntax error when trying to select \n\n' + selectorStr + '\n\n, skipped. Error details: ' + selectError);
+      continue;
     }
 
+    if (selectedEls === null) {
+      // nothing selected
+      continue;
+    }
 
-    // apply <style/> styles to matched elements
-    for (selector of sortedSelectors) {
-        if(!selector.selectedEls) {
-            continue;
-        }
-
-        if (opts.onlyMatchedOnce && selector.selectedEls !== null && selector.selectedEls.length > 1) {
-            // skip selectors that match more than once if option onlyMatchedOnce is enabled
-            continue;
-        }
-
-        // apply <style/> to matched elements
-        for (selectedEl of selector.selectedEls) {
-            if (selector.rule === null) {
-                continue;
-            }
-
-            // merge declarations
-            csstree.walk(selector.rule, {visit: 'Declaration', enter: function(styleCsstreeDeclaration) {
-
-                // existing inline styles have higher priority
-                // no inline styles, external styles,                                    external styles used
-                // inline styles,    external styles same   priority as inline styles,   inline   styles used
-                // inline styles,    external styles higher priority than inline styles, external styles used
-                var styleDeclaration = cssTools.csstreeToStyleDeclaration(styleCsstreeDeclaration);
-                if (selectedEl.style.getPropertyValue(styleDeclaration.name) !== null &&
-                    selectedEl.style.getPropertyPriority(styleDeclaration.name) >= styleDeclaration.priority) {
-                    return;
-                }
-                selectedEl.style.setProperty(styleDeclaration.name, styleDeclaration.value, styleDeclaration.priority);
-            }});
-        }
+    selector.selectedEls = selectedEls;
+  }
 
-        if (opts.removeMatchedSelectors && selector.selectedEls !== null && selector.selectedEls.length > 0) {
-            // clean up matching simple selectors if option removeMatchedSelectors is enabled
-            selector.rule.prelude.children.remove(selector.item);
-        }
+  // apply <style/> styles to matched elements
+  for (selector of sortedSelectors) {
+    if (!selector.selectedEls) {
+      continue;
     }
 
-
-    if (!opts.removeMatchedSelectors) {
-        return document; // no further processing required
+    if (
+      opts.onlyMatchedOnce &&
+      selector.selectedEls !== null &&
+      selector.selectedEls.length > 1
+    ) {
+      // skip selectors that match more than once if option onlyMatchedOnce is enabled
+      continue;
     }
 
+    // apply <style/> to matched elements
+    for (selectedEl of selector.selectedEls) {
+      if (selector.rule === null) {
+        continue;
+      }
+
+      // merge declarations
+      csstree.walk(selector.rule, {
+        visit: 'Declaration',
+        enter: function (styleCsstreeDeclaration) {
+          // existing inline styles have higher priority
+          // no inline styles, external styles,                                    external styles used
+          // inline styles,    external styles same   priority as inline styles,   inline   styles used
+          // inline styles,    external styles higher priority than inline styles, external styles used
+          var styleDeclaration = cssTools.csstreeToStyleDeclaration(
+            styleCsstreeDeclaration
+          );
+          if (
+            selectedEl.style.getPropertyValue(styleDeclaration.name) !== null &&
+            selectedEl.style.getPropertyPriority(styleDeclaration.name) >=
+              styleDeclaration.priority
+          ) {
+            return;
+          }
+          selectedEl.style.setProperty(
+            styleDeclaration.name,
+            styleDeclaration.value,
+            styleDeclaration.priority
+          );
+        },
+      });
+    }
 
-    // clean up matched class + ID attribute values
-    for (selector of sortedSelectors) {
-        if(!selector.selectedEls) {
-            continue;
-        }
+    if (
+      opts.removeMatchedSelectors &&
+      selector.selectedEls !== null &&
+      selector.selectedEls.length > 0
+    ) {
+      // clean up matching simple selectors if option removeMatchedSelectors is enabled
+      selector.rule.prelude.children.remove(selector.item);
+    }
+  }
 
-        if (opts.onlyMatchedOnce && selector.selectedEls !== null && selector.selectedEls.length > 1) {
-            // skip selectors that match more than once if option onlyMatchedOnce is enabled
-            continue;
-        }
+  if (!opts.removeMatchedSelectors) {
+    return document; // no further processing required
+  }
 
-        for (selectedEl of selector.selectedEls) {
-            // class
-            var firstSubSelector = selector.item.data.children.first();
-            if(firstSubSelector.type === 'ClassSelector') {
-                selectedEl.class.remove(firstSubSelector.name);
-            }
-            // clean up now empty class attributes
-            if(typeof selectedEl.class.item(0) === 'undefined') {
-                selectedEl.removeAttr('class');
-            }
-
-            // ID
-            if(firstSubSelector.type === 'IdSelector') {
-                selectedEl.removeAttr('id', firstSubSelector.name);
-            }
-        }
+  // clean up matched class + ID attribute values
+  for (selector of sortedSelectors) {
+    if (!selector.selectedEls) {
+      continue;
     }
 
+    if (
+      opts.onlyMatchedOnce &&
+      selector.selectedEls !== null &&
+      selector.selectedEls.length > 1
+    ) {
+      // skip selectors that match more than once if option onlyMatchedOnce is enabled
+      continue;
+    }
 
-    // clean up now empty elements
-    for (var style of styles) {
-        csstree.walk(style.cssAst, {visit: 'Rule', enter: function(node, item, list) {
-            // clean up <style/> atrules without any rulesets left
-            if (node.type === 'Atrule' &&
-                // only Atrules containing rulesets
-                node.block !== null &&
-                node.block.children.isEmpty()) {
-                list.remove(item);
-                return;
-            }
-
-            // clean up <style/> rulesets without any css selectors left
-            if (node.type === 'Rule' &&
-                node.prelude.children.isEmpty()) {
-                list.remove(item);
-            }
-        }});
-
-
-        if (style.cssAst.children.isEmpty()) {
-            // clean up now emtpy <style/>s
-            var styleParentEl = style.styleEl.parentNode;
-            styleParentEl.spliceContent(styleParentEl.content.indexOf(style.styleEl), 1);
-
-            if (styleParentEl.elem === 'defs' &&
-                styleParentEl.content.length === 0) {
-                // also clean up now empty <def/>s
-                var defsParentEl = styleParentEl.parentNode;
-                defsParentEl.spliceContent(defsParentEl.content.indexOf(styleParentEl), 1);
-            }
-
-            continue;
+    for (selectedEl of selector.selectedEls) {
+      // class
+      var firstSubSelector = selector.item.data.children.first();
+      if (firstSubSelector.type === 'ClassSelector') {
+        selectedEl.class.remove(firstSubSelector.name);
+      }
+      // clean up now empty class attributes
+      if (typeof selectedEl.class.item(0) === 'undefined') {
+        selectedEl.removeAttr('class');
+      }
+
+      // ID
+      if (firstSubSelector.type === 'IdSelector') {
+        selectedEl.removeAttr('id', firstSubSelector.name);
+      }
+    }
+  }
+
+  // clean up now empty elements
+  for (var style of styles) {
+    csstree.walk(style.cssAst, {
+      visit: 'Rule',
+      enter: function (node, item, list) {
+        // clean up <style/> atrules without any rulesets left
+        if (
+          node.type === 'Atrule' &&
+          // only Atrules containing rulesets
+          node.block !== null &&
+          node.block.children.isEmpty()
+        ) {
+          list.remove(item);
+          return;
         }
 
-
-        // update existing, left over <style>s
-        cssTools.setCssStr(style.styleEl, csstree.generate(style.cssAst));
+        // clean up <style/> rulesets without any css selectors left
+        if (node.type === 'Rule' && node.prelude.children.isEmpty()) {
+          list.remove(item);
+        }
+      },
+    });
+
+    if (style.cssAst.children.isEmpty()) {
+      // clean up now emtpy <style/>s
+      var styleParentEl = style.styleEl.parentNode;
+      styleParentEl.spliceContent(
+        styleParentEl.content.indexOf(style.styleEl),
+        1
+      );
+
+      if (styleParentEl.elem === 'defs' && styleParentEl.content.length === 0) {
+        // also clean up now empty <def/>s
+        var defsParentEl = styleParentEl.parentNode;
+        defsParentEl.spliceContent(
+          defsParentEl.content.indexOf(styleParentEl),
+          1
+        );
+      }
+
+      continue;
     }
 
+    // update existing, left over <style>s
+    cssTools.setCssStr(style.styleEl, csstree.generate(style.cssAst));
+  }
 
-    return document;
+  return document;
 };
diff --git a/test/plugins/_index.js b/test/plugins/_index.js
index 7bbc90b4c..d40546db3 100644
--- a/test/plugins/_index.js
+++ b/test/plugins/_index.js
@@ -22,19 +22,23 @@ describe('plugins tests', function () {
 
       it(name + '.' + index, function () {
         return readFile(file).then(function (data) {
-          var splitted = normalize(data).split(/\s*@@@\s*/),
-            orig = splitted[0],
-            should = splitted[1],
-            params = splitted[2];
+          // remove description
+          const items = normalize(data).split(/\s*===\s*/);
+          const test = items.length === 2 ? items[1] : items[0];
+          // extract test case
+          const [original, should, params] = test.split(/\s*@@@\s*/);
           const plugin = {
             name,
             params: params ? JSON.parse(params) : {},
           };
-          const result = optimize(orig, {
+          const result = optimize(original, {
             path: file,
             plugins: [plugin],
             js2svg: { pretty: true },
           });
+          if (result.error != null) {
+            expect.fail(result.error);
+          }
           //FIXME: results.data has a '\n' at the end while it should not
           expect(normalize(result.data)).to.equal(should);
         });
diff --git a/test/plugins/inlineStyles.20.svg b/test/plugins/inlineStyles.20.svg
new file mode 100644
index 000000000..5597b500f
--- /dev/null
+++ b/test/plugins/inlineStyles.20.svg
@@ -0,0 +1,31 @@
+<style> may have type attribute. Values other than the empty string
+or text/css are ignored.
+
+===
+
+<svg>
+  <style type="text/invalid">
+    .invalid { fill: red; }
+  </style>
+  <style type="text/css">
+    .css { fill: green; }
+  </style>
+  <style type="">
+    .empty { fill: blue; }
+  </style>
+  <rect x="0" y="0" width="100" height="100" class="invalid" />
+  <rect x="0" y="0" width="100" height="100" class="css" />
+  <rect x="0" y="0" width="100" height="100" class="empty" />
+</svg>
+
+@@@
+
+<svg>
+    <style type="text/invalid">
+        .invalid { fill: red; }
+    </style>
+    <rect x="0" y="0" width="100" height="100" class="invalid"/>
+    <rect x="0" y="0" width="100" height="100" style="fill:green"/>
+    <rect x="0" y="0" width="100" height="100" style="fill:blue"/>
+</svg>
+