Throw errors if virtual tree traversal finds an unmatched ko closing …

…comment
knockout · Oct 13, 2013 · e69a5cb · e69a5cb
1 parent 8ed5f13
commit e69a5cb
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 8 deletions.
diff --git a/spec/bindingAttributeBehaviors.js b/spec/bindingAttributeBehaviors.js
@@ -266,6 +266,42 @@ describe('Binding attribute syntax', function() {
         expect(testNode).toContainText('Hello Bert, Goodbye');
     });
 
+    it('Should reject closing virtual bindings, when found as first child', function() {
+        testNode.innerHTML = "<!-- /ko -->";
+        expect(function() {
+            ko.applyBindings(null, testNode);
+        }).toThrow();
+    });
+
+    it('Should reject closing virtual bindings without matching open, when found as a sibling', function() {
+        testNode.innerHTML = "<div></div><!-- /ko -->";
+        expect(function() {
+            ko.applyBindings(null, testNode);
+        }).toThrow();
+    });
+
+    it('Should reject duplicated closing virtual bindings', function() {
+        testNode.innerHTML = "<!-- ko if: true --><div></div><!-- /ko --><!-- /ko -->";
+        expect(function() {
+            ko.applyBindings(null, testNode);
+        }).toThrow();
+    });
+
+
+    it('Should reject opening virtual bindings that are not closed', function() {
+        testNode.innerHTML = "<!-- ko if: true -->";
+        expect(function() {
+            ko.applyBindings(null, testNode);
+        }).toThrow();
+    });
+
+    it('Should reject virtual bindings that are nested incorrectly', function() {
+        testNode.innerHTML = "<!-- ko if: true --><div><!-- /ko --></div>";
+        expect(function() {
+            ko.applyBindings(null, testNode);
+        }).toThrow();
+    });
+
     it('Should be able to access virtual children in custom containerless binding', function() {
         var countNodes = 0;
         ko.bindingHandlers.test = {

diff --git a/src/virtualElements.js b/src/virtualElements.js
@@ -24,12 +24,19 @@
         return (node.nodeType == 8) && endCommentRegex.test(commentNodesHaveTextProperty ? node.text : node.nodeValue);
     }
 
+    function isUnmatchedEndComment(node) {
+        return isEndComment(node) && !(ko.utils.domData.get(node, matchedEndCommentDataKey));
+    }
+
+    var matchedEndCommentDataKey = "__ko_matchedEndComment__"
+
     function getVirtualChildren(startComment, allowUnbalanced) {
         var currentNode = startComment;
         var depth = 1;
         var children = [];
         while (currentNode = currentNode.nextSibling) {
             if (isEndComment(currentNode)) {
+                ko.utils.domData.set(currentNode, matchedEndCommentDataKey, true);
                 depth--;
                 if (depth === 0)
                     return children;
@@ -133,19 +140,30 @@
         },
 
         firstChild: function(node) {
-            if (!isStartComment(node))
+            if (!isStartComment(node)) {
+                if (node.firstChild && isUnmatchedEndComment(node.firstChild)) {
+                    throw new Error("Found end comment without opening comment, as first child of " + node.outerHTML);
+                }
                 return node.firstChild;
-            if (!node.nextSibling || isEndComment(node.nextSibling))
-                return null;
-            return node.nextSibling;
+            } else {
+                return node.nextSibling;
+            }
         },
 
         nextSibling: function(node) {
-            if (isStartComment(node))
+            if (isStartComment(node)) {
                 node = getMatchingEndComment(node);
-            if (node.nextSibling && isEndComment(node.nextSibling))
-                return null;
-            return node.nextSibling;
+            }
+
+            if (node.nextSibling && isEndComment(node.nextSibling)) {
+                if (isUnmatchedEndComment(node.nextSibling)) {
+                    throw Error("Found end comment without a matching opening comment, as next sibling of " + node.outerHTML);
+                } else {
+                    return null;
+                }
+            } else {
+                return node.nextSibling;
+            }
         },
 
         hasBindingValue: isStartComment,