Repository: svg-sprite/svg-sprite (js)

Issue:
Valid SVG files are erroneously marked as invalid
An example SVG file that is not able to be processed is [this file](https://github.com/mozilla/pdf.js/blob/6ddc297170a48475b2fdb882efd895d0236474c3/web/images/toolbarButton-pageUp.svg?short_path=aa0160a).
The regex in the following line is not able to match multi-line attribute values of the svg element:
https://github.com/svg-sprite/svg-sprite/blob/7edd0063b0fcf91c436a0d13ec6e01ce01af3299/lib/svg-sprite/shape.js#L324

In the aforementioned example, the `viewBox` attribute of the `svg` element has such a multi-line attribute value:
```svg
<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 
16">
```

Processing this file with the command  `node bin/svg-sprite.js -l debug /path/to/valid/svg` will produce the following log entries:
```
2021-04-05 21:04:48 - debug: Started logging
2021-04-05 21:04:48 - debug: Prepared general options
2021-04-05 21:04:48 - debug: Prepared `shape` options
2021-04-05 21:04:48 - debug: Prepared `svg` options
2021-04-05 21:04:48 - debug: Prepared `mode` options
2021-04-05 21:04:48 - debug: Prepared `variables` options
2021-04-05 21:04:48 - verbose: Initialized spriter configuration
2021-04-05 21:04:48 - debug: Created processing queue instance
2021-04-05 21:04:48 - info: Created spriter instance
2021-04-05 21:04:48 - debug: Added "toolbarButton-pageUp.svg" to processing queue
2021-04-05 21:04:48 - error: Skipping "toolbarButton-pageUp.svg" (Invalid SVG file)
2021-04-05 21:04:48 - info: Compiling 0 shapes ...
2021-04-05 21:04:48 - debug: Created layouter instance
2021-04-05 21:04:48 - info: Finished  sprite compilation
2021-04-05 21:04:48 - info: Created
```

Even though an error occurred while processing the svg file, the error code of the command is 0. The output of the command with flag `-l debug` must be manually inspected to notice that there was an issue.

Pull request:
fix: fixing valid svg checking
+ some JSDoc fixes

Suggestion: moving from `mocha` to `jest`. Current problem is with inability of mocking modules. For example in `test/svg-shape.test.js` I have to copy-paste test input strings from  `test/fix-xml-string.test.js` instead of mocking `lib/svg-sprite/fix-xml-string.js` behavior. 
But there is two problems: 
1. Jest is a BIG package
2. Current test dependency. All test a designed to run step-by-step, but jest is designed to run parallel tests with random orde

Fixes #454

Patch:
diff --git a/lib/svg-sprite/fix-xml-string.js b/lib/svg-sprite/fix-xml-string.js
new file mode 100644
index 0000000..9323808
--- /dev/null
+++ b/lib/svg-sprite/fix-xml-string.js
@@ -0,0 +1,22 @@
+const { DOMParser } = require('@xmldom/xmldom');
+
+module.exports = svgString => {
+    let domParserError = false;
+    const errorHandler = () => {
+        domParserError = true;
+    };
+
+    const fixedSVG = new DOMParser({ errorHandler })
+        .parseFromString(svgString)
+        .toString()
+        .replace(/(\s)(\s+)/g, ' ');
+
+    if (!domParserError) {
+        return fixedSVG;
+    }
+
+    const error = new Error('Invalid XML string');
+    error.name = 'XMLFixingError';
+    error.errno = 1_400_141_400;
+    throw error;
+};
diff --git a/lib/svg-sprite/shape.js b/lib/svg-sprite/shape.js
index cad355f..abbcfc8 100644
--- a/lib/svg-sprite/shape.js
+++ b/lib/svg-sprite/shape.js
@@ -20,6 +20,7 @@ const { CssSelectorParser } = require('css-selector-parser');
 const async = require('async');
 const csso = require('csso');
 const calculateSvgDimensions = require('./calculate-svg-dimensions.js');
+const fixXMLString = require('./fix-xml-string.js');
 
 /**
  * Default callback for shape ID generation
@@ -314,13 +315,29 @@ SVGShape.prototype.setSVG = function(svg) {
  */
 SVGShape.prototype._initSVG = function() {
     // Basic check for basic SVG file structure
-    const svgStart = this.svg.current.match(/<svg(?:\s+[a-z\d-:]+=("|').*?\1)*\s*(?:(\/)|(>[^]*<\/svg))>/i);
+    const validSVGRegExp = /<svg(?:\s+[a-z\d-:]+=(["']).*?\1)*\s*(?:(\/)|(>[^]*<\/svg))>/i;
+    let svgStart = this.svg.current.match(validSVGRegExp);
 
     if (!svgStart) {
-        const e = new Error('Invalid SVG file');
-        e.name = 'ArgumentError';
-        e.errno = 1_429_395_394;
-        throw e;
+        const throwError = () => {
+            const e = new Error('Invalid SVG file');
+            e.name = 'ArgumentError';
+            e.errno = 1_429_395_394;
+            throw e;
+        };
+
+        try {
+            const fixedXMLString = fixXMLString(this.svg.current);
+            svgStart = fixedXMLString.match(validSVGRegExp);
+
+            if (!svgStart) {
+                return throwError();
+            }
+
+            this.svg.current = fixedXMLString;
+        } catch {
+            throwError();
+        }
     }
 
     // Resolve XML entities
@@ -431,7 +448,7 @@ SVGShape.prototype.setDimensions = function(width, height) {
  * Return the shape's viewBox (and set it if it doesn't exist yet)
  *
  * @param {Number} width        Width
- * @param {Height} height       Height
+ * @param {Number} height       Height
  * @return {Array}              Viewbox
  */
 SVGShape.prototype.getViewbox = function(width, height) {
@@ -886,13 +903,10 @@ SVGShape.prototype.distribute = function() {
 
 /**
  * Module export (constructor wrapper)
- *
- * @param {String} svg          SVG content
- * @param {String} name         Name part or the file path
- * @param {String} file         Absolute file path
- * @param {Object} config       SVG shape configuration
- * @return {SVGShape}           SVGShape instance
+ * @param {File} file                   Vinyl file
+ * @param {SVGSpriter} spriter          Spriter instance
+ * @return {SVGShape}                   SVGShape instance
  */
-module.exports = function(svg, name) {
-    return new SVGShape(svg, name);
+module.exports = function(file, spriter) {
+    return new SVGShape(file, spriter);
 };


Test patch:
diff --git a/test/fix-xml-string.test.js b/test/fix-xml-string.test.js
new file mode 100644
index 0000000..bc42dfe
--- /dev/null
+++ b/test/fix-xml-string.test.js
@@ -0,0 +1,39 @@
+const assert = require('assert').strict;
+const fixXMLString = require('../lib/svg-sprite/fix-xml-string.js');
+
+describe('testing fixing svg string', () => {
+    it('should return valid svg file on svg with multiline attribute values', () => {
+        assert.equal(fixXMLString(`<svg viewBox="0 0 0 16
+                                     16"></svg>`), '<svg viewBox="0 0 0 16 16"/>');
+    });
+
+    it('should return valid svg file on svg with multiline attribute values', () => {
+        assert.equal(fixXMLString(`<svg fill="r
+                                                            e
+                                                            d"
+                                                            viewBox="0 0 0 16
+                                                                                                 16"></svg>`),
+        '<svg fill="r e d" viewBox="0 0 0 16 16"/>');
+    });
+
+    it('should return valid svg file on svg with multiline attribute values', () => {
+        assert.equal(fixXMLString(`<svg fill="r
+                                                            e
+                                                            d"
+                                                            viewBox="0
+                                                            0
+                                                            16
+                                                            16"></svg>`),
+        '<svg fill="r e d" viewBox="0 0 16 16"/>');
+    });
+
+    it('should throw an error on invalid file', () => {
+        assert.throws(() => {
+            fixXMLString('<svg viewBox=></svg>');
+        }, Error);
+    });
+
+    it('should return same string on valid svg', () => {
+        assert.equal(fixXMLString('<svg viewBox="0 0 0 16 16"></svg>'), '<svg viewBox="0 0 0 16 16"/>');
+    });
+});
diff --git a/test/svg-shape.test.js b/test/svg-shape.test.js
new file mode 100644
index 0000000..dde9d31
--- /dev/null
+++ b/test/svg-shape.test.js
@@ -0,0 +1,101 @@
+const assert = require('assert').strict;
+const { Buffer } = require('buffer');
+const path = require('path');
+const fs = require('fs');
+const File = require('vinyl');
+const glob = require('glob');
+const getShape = require('../lib/svg-sprite/shape.js');
+const SVGSpriter = require('../lib/svg-sprite.js');
+
+describe('testing SVGShape initialization', () => {
+    let spriter;
+
+    beforeEach(() => {
+        spriter = new SVGSpriter({
+            shape: {
+                dest: 'svg'
+            }
+        });
+    });
+
+    it('should not throw an error on valid svg file with multiline attribute values', () => {
+        assert.doesNotThrow(() => {
+            getShape(new File({
+                path: __dirname,
+                contents: Buffer.from(`<svg viewBox="0 0 0 16
+                                     16"></svg>`)
+            }), spriter);
+        }, Error);
+    });
+
+    it('should not throw an error on valid svg file with multiline attribute values', () => {
+        assert.doesNotThrow(() => {
+            getShape(new File({
+                path: __dirname,
+                contents: Buffer.from(`<svg fill="r
+                                                            e
+                                                            d"
+                                                            viewBox="0 0 0 16
+                                                                                                 16"></svg>`)
+            }), spriter);
+        }, Error);
+    });
+
+    it('should not throw an error on valid svg file with mutliple multilined attritbutes values', () => {
+        assert.doesNotThrow(() => {
+            getShape(new File({
+                path: __dirname,
+                contents: Buffer.from(`<svg fill="r
+                                                            e
+                                                            d"
+                                                            viewBox="0
+                                                            0
+                                                            0
+                                                            16
+                                                            16"></svg>`)
+            }), spriter);
+        }, Error);
+    });
+
+    it('should throw an error on invalid file', () => {
+        assert.throws(() => {
+            getShape(new File({
+                path: __dirname,
+                contents: Buffer.from('<svg viewBox=></svg>')
+            }), spriter);
+        }, Error);
+    });
+
+    it('should throw an error on non-svg files', () => {
+        assert.throws(() => {
+            getShape(new File({
+                path: __dirname,
+                contents: Buffer.from('<div class="test">123</div>')
+            }), spriter);
+        }, Error);
+    });
+
+    it('should not throw an error on valid svg file with normal values', () => {
+        assert.doesNotThrow(() => {
+            getShape(new File({
+                path: __dirname,
+                contents: Buffer.from('<svg viewBox="0 0 0 16 16"></svg>')
+            }), spriter);
+        }, Error);
+    });
+
+    it('should not throw an error on actual valid svg files', () => {
+        const cwdWeather = path.join(__dirname, 'fixture/svg/single');
+        const weather = glob.sync('**/weather*.svg', { cwd: cwdWeather });
+
+        weather.forEach(weatherFile => {
+            const svgFileBuffer = fs.readFileSync(path.join(cwdWeather, weatherFile));
+            assert.doesNotThrow(() => {
+                getShape(new File({
+                    path: __dirname,
+                    contents: svgFileBuffer
+                }), spriter);
+            }, Error);
+        });
+    });
+});