Skip to content

Commit

Permalink
fix: avoid problems from element IDs that exist on object prototype (#…
Browse files Browse the repository at this point in the history
…4060)

* fix: avoid problems from element IDs that exist on object prototype

* 🤖 Automated formatting fixes

* Fixed tests
  • Loading branch information
WilcoFiers authored Jun 22, 2023
1 parent 173f29d commit 8d135dd
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 21 deletions.
25 changes: 14 additions & 11 deletions lib/commons/aria/get-accessible-refs.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,26 @@ function cacheIdRefs(node, idRefs, refAttrs) {
if (node.hasAttribute) {
if (node.nodeName.toUpperCase() === 'LABEL' && node.hasAttribute('for')) {
const id = node.getAttribute('for');
idRefs[id] = idRefs[id] || [];
idRefs[id].push(node);
if (!idRefs.has(id)) {
idRefs.set(id, [node]);
} else {
idRefs.get(id).push(node);
}
}

for (let i = 0; i < refAttrs.length; ++i) {
const attr = refAttrs[i];
const attrValue = sanitize(node.getAttribute(attr) || '');

if (!attrValue) {
continue;
}

const tokens = tokenList(attrValue);
for (let k = 0; k < tokens.length; ++k) {
idRefs[tokens[k]] = idRefs[tokens[k]] || [];
idRefs[tokens[k]].push(node);
for (const token of tokenList(attrValue)) {
if (!idRefs.has(token)) {
idRefs.set(token, [node]);
} else {
idRefs.get(token).push(node);
}
}
}
}
Expand All @@ -50,22 +54,21 @@ function getAccessibleRefs(node) {
let root = getRootNode(node);
root = root.documentElement || root; // account for shadow roots

const idRefsByRoot = cache.get('idRefsByRoot', () => new WeakMap());
const idRefsByRoot = cache.get('idRefsByRoot', () => new Map());

let idRefs = idRefsByRoot.get(root);
if (!idRefs) {
idRefs = {};
idRefs = new Map();
idRefsByRoot.set(root, idRefs);

const refAttrs = Object.keys(standards.ariaAttrs).filter(attr => {
const { type } = standards.ariaAttrs[attr];
return idRefsRegex.test(type);
});

cacheIdRefs(root, idRefs, refAttrs);
}

return idRefs[node.id] || [];
return idRefs.get(node.id) ?? [];
}

export default getAccessibleRefs;
8 changes: 7 additions & 1 deletion lib/core/utils/find-by.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@
*/
function findBy(array, key, value) {
if (Array.isArray(array)) {
return array.find(obj => typeof obj === 'object' && obj[key] === value);
return array.find(
obj =>
obj !== null &&
typeof obj === 'object' &&
Object.hasOwn(obj, key) &&
obj[key] === value
);
}
}

Expand Down
13 changes: 9 additions & 4 deletions lib/core/utils/selector-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,12 @@ function findMatchingNodes(expression, selectorMap, shadowId) {
nodes = selectorMap['*'];
} else {
if (exp.id) {
// a selector must match all parts, otherwise we can just exit
// early
if (!selectorMap[idsKey] || !selectorMap[idsKey][exp.id]?.length) {
// a selector must match all parts, otherwise we can just exit early
if (
!selectorMap[idsKey] ||
!Object.hasOwn(selectorMap[idsKey], exp.id) ||
!selectorMap[idsKey][exp.id]?.length
) {
return;
}

Expand Down Expand Up @@ -176,7 +179,9 @@ function getSharedValues(a, b) {
* @param {Object} map
*/
function cacheSelector(key, vNode, map) {
map[key] = map[key] || [];
if (!Object.hasOwn(map, key)) {
map[key] = [];
}
map[key].push(vNode);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/standards/html-elms.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ const htmlElms = {
'menuitem',
'menuitemcheckbox',
'menuitemradio',
'meter',
'meter',
'option',
'progressbar',
'radio',
Expand Down
28 changes: 28 additions & 0 deletions test/commons/aria/get-accessible-refs.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,34 @@ describe('aria.getAccessibleRefs', function () {
assert.deepEqual(getAccessibleRefs(node), [ref]);
});

describe('when JavaScript object names are used as IDs', function () {
const ids = [
'prototype',
'constructor',
'__proto__',
'Element',
'nodeName',
'valueOf',
'toString'
];
for (const id of ids) {
it(`does not break with id="${id}"`, function () {
setLookup({ 'aria-bar': { type: 'idrefs' } });
fixture.innerHTML = `<div id="ref" aria-bar="${ids.join(
' '
)}"></div><i id="${id}"></i>`;

var node = document.getElementById(id);
var ref = document.getElementById('ref');
assert.deepEqual(
getAccessibleRefs(node),
[ref],
`Not equal for ID ${id}`
);
});
}
});

(shadowSupport ? it : xit)('works inside shadow DOM', function () {
setLookup({ 'aria-bar': { type: 'idref' } });
fixture.innerHTML = '<div id="foo"></div>';
Expand Down
17 changes: 17 additions & 0 deletions test/core/utils/find-by.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,21 @@ describe('axe.utils.findBy', function () {
it('should not throw if passed falsey first parameter', function () {
assert.isUndefined(axe.utils.findBy(null, 'id', 'macaque'));
});

it('ignores any non-object elements in the array', function () {
const obj = {
id: 'monkeys',
foo: 'bar'
};
const array = ['bananas', true, null, 123, obj];

assert.equal(axe.utils.findBy(array, 'id', 'monkeys'), obj);
});

it('only looks at owned properties', function () {
const obj1 = { id: 'monkeys', eat: 'bananas' };
const obj2 = Object.create(obj1);
obj2.id = 'gorillas';
assert.equal(axe.utils.findBy([obj2, obj1], 'eat', 'bananas'), obj1);
});
});
23 changes: 23 additions & 0 deletions test/core/utils/selector-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,29 @@ describe('utils.selector-cache', function () {

assert.lengthOf(Object.keys(map), 0);
});

describe('with javascripty attribute selectors', function () {
const terms = [
'prototype',
'constructor',
'__proto__',
'Element',
'nodeName',
'valueOf',
'toString'
];
for (const term of terms) {
it(`works with ${term}`, function () {
fixture.innerHTML = `<div id="${term}" class="${term}" aria-label="${term}"></div>`;
const vNode = new axe.VirtualNode(fixture.firstChild);
const map = {};
cacheNodeSelectors(vNode, map);
assert.deepEqual(map['[id]'], [vNode]);
assert.deepEqual(map['[class]'], [vNode]);
assert.deepEqual(map['[aria-label]'], [vNode]);
});
}
});
});

describe('getNodesMatchingExpression', function () {
Expand Down
6 changes: 3 additions & 3 deletions test/integration/full/all-rules/all-rules.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
>
<banner></banner>
<main>
<div accesskey="B"></div>
<div accesskey="B" id="__proto__"></div>
<map>
<area href="#" id="pass1" alt="monkeys" />
</map>
<div aria-label="foo">Foo</div>
<div aria-label="foo" id="constructor">Foo</div>
<div role="contentinfo"></div>
<div role="link">Home</div>
<div role="dialog" aria-label="Cookies"></div>
Expand All @@ -51,7 +51,7 @@
<div role="treeitem">Item</div>
</div>
<audio id="caption"><track kind="captions" /></audio>
<input autocomplete="username" />
<input autocomplete="username" id="toString" />
<p id="fail1" style="line-height: 1.5 !important">Banana error</p>
<p><blink>text</blink></p>
<button id="text">Name</button>
Expand Down
10 changes: 9 additions & 1 deletion test/integration/rules/aria-allowed-role/aria-allowed-role.html
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,15 @@ <h1 id="pass-h1-valid-role" role="none"></h1>
<div id="fail-dpub-4" role="doc-noteref">ok</div>
<!-- images -->
<div id="fail-dpub-5" role="doc-cover">ok</div>
<img src="#" role="meter" aria-valuenow="0" aria-valuemin="0" aria-valuemax="100" alt="test" id="pass-img-valid-role-meter">
<img
src="#"
role="meter"
aria-valuenow="0"
aria-valuemin="0"
aria-valuemax="100"
alt="test"
id="pass-img-valid-role-meter"
/>
<img role="doc-cover" aria-label="foo" id="pass-img-valid-role-aria-label" />
<img role="menuitem" title="bar" id="pass-img-valid-role-title" />
<div id="image-baz">hazaar</div>
Expand Down

0 comments on commit 8d135dd

Please sign in to comment.