fix: avoid reading element-specific node properties of non-element node types (#4317)

Updates VirtualNode's `props` initialization path to avoid reading
properties that we know based on `nodeType` won't be present anyway.
This should mitigate #4316 by avoiding reading the problematic `value`
prop present on certain text nodes.

I also cleaned up some missing test coverage in the impacted code.

Closes: #4316

Author: dbjorge

lib/core/base/virtual-node/virtual-node.js

@@ -51,30 +51,25 @@ class VirtualNode extends AbstractVirtualNode {
   // add to the prototype so memory is shared across all virtual nodes
   get props() {
     if (!this._cache.hasOwnProperty('props')) {
-      const {
-        nodeType,
-        nodeName,
-        id,
-        multiple,
-        nodeValue,
-        value,
-        selected,
-        checked,
-        indeterminate
-      } = this.actualNode;
+      const { nodeType, nodeName, id, nodeValue } = this.actualNode;
 
       this._cache.props = {
         nodeType,
         nodeName: this._isXHTML ? nodeName : nodeName.toLowerCase(),
         id,
         type: this._type,
-        multiple,
-        nodeValue,
-        value,
-        selected,
-        checked,
-        indeterminate
+        nodeValue
       };
+
+      // We avoid reading these on node types where they won't be relevant
+      // to work around issues like #4316.
+      if (nodeType === 1) {
+        this._cache.props.multiple = this.actualNode.multiple;
+        this._cache.props.value = this.actualNode.value;
+        this._cache.props.selected = this.actualNode.selected;
+        this._cache.props.checked = this.actualNode.checked;
+        this._cache.props.indeterminate = this.actualNode.indeterminate;
+      }
     }
 
     return this._cache.props;

test/core/base/virtual-node/virtual-node.js

@@ -37,15 +37,45 @@ describe('VirtualNode', () => {
       assert.equal(vNode.props.type, 'text');
     });
 
-    it('should reflect selected property', () => {
-      node = document.createElement('option');
-      let vNode = new VirtualNode(node);
-      assert.equal(vNode.props.selected, false);
-
-      node.selected = true;
-      vNode = new VirtualNode(node);
-      assert.equal(vNode.props.selected, true);
-    });
+    for (const [prop, tagName, examplePropValue] of [
+      ['value', 'input', 'test value'],
+      ['selected', 'option', true],
+      ['checked', 'input', true],
+      ['indeterminate', 'input', true],
+      ['multiple', 'select', true]
+    ]) {
+      describe(`props.${prop}`, () => {
+        it(`should reflect a ${tagName} element's ${prop} property`, () => {
+          node = document.createElement(tagName);
+          let vNode = new VirtualNode(node);
+          assert.equal(vNode.props[prop], '');
+
+          node[prop] = examplePropValue;
+          vNode = new VirtualNode(node);
+          assert.equal(vNode.props[prop], examplePropValue);
+        });
+
+        it('should be undefined for a text node', () => {
+          node = document.createTextNode('text content');
+          let vNode = new VirtualNode(node);
+          assert.equal(vNode.props[prop], undefined);
+        });
+
+        // Regression test for #4316
+        it(`should be resilient to text node with un-gettable ${prop} property`, () => {
+          node = document.createTextNode('text content');
+          Object.defineProperty(node, prop, {
+            get() {
+              throw new Error('Unqueryable value');
+            }
+          });
+          let vNode = new VirtualNode(node);
+          assert.throws(() => node[prop]);
+          assert.doesNotThrow(() => vNode.props[prop]);
+          assert.equal(vNode.props[prop], undefined);
+        });
+      });
+    }
 
     it('should lowercase type', () => {
       node = document.createElement('input');