[FIX] Diff algorithm exception (#110)

Prevents access to undefined objects in case the base stylesheet contains more rules than the compare stylesheet. Fixes: #109 Co-authored-by: Matthias Oßwald <1410947+matz3@users.noreply.github.com>
SAP · Jan 7, 2020 · 9628a6c · 9628a6c
1 parent 7b2eefa
commit 9628a6c
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 6 deletions.
diff --git a/lib/diff.js b/lib/diff.js
@@ -63,13 +63,13 @@ Diffing.prototype.diffRules = function(oBaseRules, oCompareRules) {
 		let oDiffNode = null;
 
 		// Add all different compare nodes to stack and check for next one
-		while (oBaseNode.type !== oCompareNode.type) {
+		while (oCompareNode && oBaseNode.type !== oCompareNode.type) {
 			this.oStack.stylesheet.rules.push(oCompareNode);
 			iCompareNode++;
 			oCompareNode = oCompareRules[iCompareNode];
 		}
 
-		if (oBaseNode.type === "comment") {
+		if (oCompareNode && oBaseNode.type === "comment") {
 			const sBaseComment = oBaseNode.comment;
 			const sCompareComment = oCompareNode.comment;
 
@@ -87,12 +87,12 @@ Diffing.prototype.diffRules = function(oBaseRules, oCompareRules) {
 			}
 
 			const aBaseDeclarations = oBaseNode.declarations;
-			const aCompareDeclarations = oCompareNode.declarations;
+			const aCompareDeclarations = oCompareNode && oCompareNode.declarations;
 			for (let j = 0; j < aBaseDeclarations.length; j++) {
 				const oBaseDeclaration = aBaseDeclarations[j];
-				const oCompareDeclaration = aCompareDeclarations[j];
+				const oCompareDeclaration = aCompareDeclarations && aCompareDeclarations[j];
 
-				if (oBaseDeclaration.type === "declaration") {
+				if (oCompareDeclaration && oBaseDeclaration.type === "declaration") {
 					// TODO: Also check for different node and add to stack???
 					if (oBaseDeclaration.type === oCompareDeclaration.type) {
 						if (oBaseDeclaration.property === oCompareDeclaration.property) {
@@ -111,7 +111,7 @@ Diffing.prototype.diffRules = function(oBaseRules, oCompareRules) {
 					}
 				}
 			}
-		} else if (oBaseNode.type === "media") {
+		} else if (oCompareNode && oBaseNode.type === "media") {
 			const aMediaDiffRules = this.diffRules(oBaseNode.rules, oCompareNode.rules);
 
 			if (aMediaDiffRules.length > 0) {

diff --git a/test/fixtures/diff/css/library2/base.css b/test/fixtures/diff/css/library2/base.css
@@ -0,0 +1,11 @@
+a {
+  color: blue;
+}
+/* one */
+b {
+  color: green;
+}
+b.test {
+  color: yellow;
+}
+/* two */
diff --git a/test/fixtures/diff/css/library2/compare.css b/test/fixtures/diff/css/library2/compare.css
@@ -0,0 +1,7 @@
+a {
+  color: blue;
+}
+/* one */
+b {
+  color: green;
+}
diff --git a/test/test-diff.js b/test/test-diff.js
@@ -40,6 +40,24 @@ describe("Diff algorithm", function() {
 
 		const oResult = diff(oBase, oEmbedded);
 
+		assert.deepStrictEqual(oResult.diff.stylesheet.rules.map(convertRuleToComparableString), [
+			{
+				"type": "rule",
+				"value": "a"
+			},
+			{
+				"type": "rule",
+				"value": "b"
+			},
+			{
+				"type": "comment",
+				"value": " mine "
+			},
+			{
+				"type": "rule",
+				"value": "b.test"
+			}
+		]);
 		assert.deepStrictEqual(oResult.stack.stylesheet.rules.map(convertRuleToComparableString), [
 			{
 				"type": "rule",
@@ -55,6 +73,28 @@ describe("Diff algorithm", function() {
 			}
 		]);
 	});
+	it("should create a diff with more rules in base than in compare", function() {
+		const compareCSS = fs.readFileSync(path.join(cssPath, "library2/compare.css"), options);
+		const baseCSS = fs.readFileSync(path.join(cssPath, "library2/base.css"), options);
+
+		// Create diff object between embeddedCompare and embedded
+		const oBase = css.parse(baseCSS);
+		const oEmbedded = css.parse(compareCSS);
+
+		const oResult = diff(oBase, oEmbedded);
+
+		assert.deepStrictEqual(oResult.diff.stylesheet.rules.map(convertRuleToComparableString), [
+			{
+				"type": "rule",
+				"value": "a"
+			},
+			{
+				"type": "rule",
+				"value": "b"
+			}
+		]);
+		assert.deepStrictEqual(oResult.stack.stylesheet.rules.map(convertRuleToComparableString), []);
+	});
 });