Skip to content

Commit

Permalink
fix: Prevent error when using <select name="attributes"> [#1397] (#1432)
Browse files Browse the repository at this point in the history
  • Loading branch information
straker authored and WilcoFiers committed Mar 28, 2019
1 parent ce77fa6 commit b477e0d
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 11 deletions.
6 changes: 6 additions & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@
{
"selector": "MemberExpression[property.name=tagName]",
"message": "Don't use node.tagName, use node.nodeName instead."
},
{
// node.attributes can be clobbered so is unsafe to use
// @see https://github.com/dequelabs/axe-core/pull/1432
"selector": "MemberExpression[object.name=node][property.name=attributes]",
"message": "Don't use node.attributes, use node.hasAttributes() or axe.utils.getNodeAttributes(node) instead."
}
]
}
Expand Down
2 changes: 1 addition & 1 deletion lib/checks/aria/allowed-attr.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var attr,
attrName,
allowed,
role = node.getAttribute('role'),
attrs = node.attributes;
attrs = axe.utils.getNodeAttributes(node);

if (!role) {
role = axe.commons.aria.implicitRole(node);
Expand Down
2 changes: 1 addition & 1 deletion lib/checks/aria/unsupportedattr.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const nodeName = node.nodeName.toUpperCase();
const lookupTable = axe.commons.aria.lookupTable;
const role = axe.commons.aria.getRole(node);

const unsupportedAttrs = Array.from(node.attributes)
const unsupportedAttrs = Array.from(axe.utils.getNodeAttributes(node))
.filter(({ name }) => {
const attribute = lookupTable.attributes[name];

Expand Down
2 changes: 1 addition & 1 deletion lib/checks/aria/valid-attr-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ var invalid = [],

var attr,
attrName,
attrs = node.attributes;
attrs = axe.utils.getNodeAttributes(node);

var skipAttrs = ['aria-errormessage'];

Expand Down
2 changes: 1 addition & 1 deletion lib/checks/aria/valid-attr.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var invalid = [],
aria = /^aria-/;

var attr,
attrs = node.attributes;
attrs = axe.utils.getNodeAttributes(node);

for (var i = 0, l = attrs.length; i < l; i++) {
attr = attrs[i].name;
Expand Down
2 changes: 1 addition & 1 deletion lib/commons/aria/roles.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ aria.implicitRole = function(node) {
return null;
}

var nodeAttributes = node.attributes;
var nodeAttributes = axe.utils.getNodeAttributes(node);
var ariaAttributes = [];

/* Get all aria-attributes defined for this node */
Expand Down
21 changes: 21 additions & 0 deletions lib/core/utils/get-node-attributes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/* global axe */

/**
* Return the list of attributes of a node.
* @method getNodeAttributes
* @memberof axe.utils
* @param {Element} node
* @returns {NamedNodeMap}
*/
axe.utils.getNodeAttributes = function getNodeAttributes(node) {
// eslint-disable-next-line no-restricted-syntax
if (node.attributes instanceof window.NamedNodeMap) {
// eslint-disable-next-line no-restricted-syntax
return node.attributes;
}

// if the attributes property is not of type NamedNodeMap then the DOM
// has been clobbered. E.g. <form><input name="attributes"></form>.
// We can clone the node to isolate it and then return the attributes
return node.cloneNode(false).attributes;
};
8 changes: 4 additions & 4 deletions lib/core/utils/get-selector.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ axe.utils.getSelectorData = function(domTree) {
}

// count all the filtered attributes
if (node.attributes) {
Array.from(node.attributes)
if (node.hasAttributes()) {
Array.from(axe.utils.getNodeAttributes(node))
.filter(filterAttributes)
.forEach(at => {
let atnv = getAttributeNameValue(node, at);
Expand Down Expand Up @@ -238,8 +238,8 @@ function uncommonAttributes(node, selectorData) {
let attData = selectorData.attributes;
let tagData = selectorData.tags;

if (node.attributes) {
Array.from(node.attributes)
if (node.hasAttributes()) {
Array.from(axe.utils.getNodeAttributes(node))
.filter(filterAttributes)
.forEach(at => {
const atnv = getAttributeNameValue(node, at);
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/aria-allowed-attr-matches.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const aria = /^aria-/;
if (node.hasAttributes()) {
let attrs = node.attributes;
let attrs = axe.utils.getNodeAttributes(node);
for (let i = 0, l = attrs.length; i < l; i++) {
if (aria.test(attrs[i].name)) {
return true;
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/aria-has-attr-matches.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
var aria = /^aria-/;
if (node.hasAttributes()) {
var attrs = node.attributes;
var attrs = axe.utils.getNodeAttributes(node);
for (var i = 0, l = attrs.length; i < l; i++) {
if (aria.test(attrs[i].name)) {
return true;
Expand Down
26 changes: 26 additions & 0 deletions test/core/utils/get-node-attributes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
describe('axe.utils.getNodeAttributes', function() {
'use strict';

it('should return the list of attributes', function() {
var node = document.createElement('div');
node.setAttribute('class', 'foo bar');
var actual = axe.utils.getNodeAttributes(node);
assert.isTrue(actual instanceof window.NamedNodeMap);
assert.equal(actual.length, 1);
assert.equal(actual[0].name, 'class');
});

it('should return the list of attributes when the DOM is clobbered', function() {
var node = document.createElement('form');
node.setAttribute('id', '123');
node.innerHTML = '<select name="attributes"></select>';

// eslint-disable-next-line no-restricted-syntax
assert.isFalse(node.attributes instanceof window.NamedNodeMap);

var actual = axe.utils.getNodeAttributes(node);
assert.isTrue(actual instanceof window.NamedNodeMap);
assert.equal(actual.length, 1);
assert.equal(actual[0].name, 'id');
});
});

0 comments on commit b477e0d

Please sign in to comment.