Skip to content

Commit 795ee3f

Browse files
authored
fix: compare tag and attribute names case insensitively (#1937)
HTML tag and attribute names are ASCII case insensitive, and the parser hands them over exactly as they were written. Two default-on rules compare them raw, so the identical violation spelled in a different case is silently unreported. src-not-empty tests the tag with /^(img|script|embed|bgsound|iframe)$/, with no i flag, and the attribute with attr.name === 'src'. So <img src=""> is reported and <img SRC="">, <IMG src=""> and <IMG SRC=""> are not. The same applies to link/href and object/data. attr-no-duplication keys its seen-set on the raw attribute name, so <div id="a" id="b"> is reported and <div ID="a" id="b"> is not. 28 of the 44 rules already normalise with toLowerCase; this brings these two into line. The reported message still echoes the attribute as the author wrote it. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
1 parent dc6e3cd commit 795ee3f

6 files changed

Lines changed: 42 additions & 12 deletions

File tree

‎dist/core/rules/attr-no-duplication.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/core/rules/src-not-empty.js‎

Lines changed: 6 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/core/rules/attr-no-duplication.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ export default {
1414

1515
for (let i = 0, l = attrs.length; i < l; i++) {
1616
attr = attrs[i]
17-
attrName = attr.name
17+
// HTML attribute names are ASCII case insensitive, so id and ID on the
18+
// same element are a duplicate.
19+
attrName = attr.name.toLowerCase()
1820

1921
if (mapAttrName[attrName] === true) {
2022
reporter.error(

‎src/core/rules/src-not-empty.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,23 @@ export default {
55
description: 'The src attribute of an img(script,link) must have a value.',
66
init(parser, reporter) {
77
parser.addListener('tagstart', (event) => {
8-
const tagName = event.tagName
8+
// HTML tag and attribute names are ASCII case insensitive, so compare
9+
// against the lowercased form. The parser hands them over exactly as they
10+
// were written.
11+
const tagName = event.tagName.toLowerCase()
912
const attrs = event.attrs
1013
let attr
1114
const col = event.col + tagName.length + 1
1215

1316
for (let i = 0, l = attrs.length; i < l; i++) {
1417
attr = attrs[i]
18+
const attrName = attr.name.toLowerCase()
1519

1620
if (
1721
((/^(img|script|embed|bgsound|iframe)$/.test(tagName) === true &&
18-
attr.name === 'src') ||
19-
(tagName === 'link' && attr.name === 'href') ||
20-
(tagName === 'object' && attr.name === 'data')) &&
22+
attrName === 'src') ||
23+
(tagName === 'link' && attrName === 'href') ||
24+
(tagName === 'object' && attrName === 'data')) &&
2125
attr.value === ''
2226
) {
2327
reporter.error(

‎test/rules/attr-no-duplication.spec.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@ describe(`Rules: ${ruleId}`, () => {
1515
expect(messages[0].col).toBe(12)
1616
})
1717

18+
it('Attribute name duplicated in a different case should result in an error', () => {
19+
// HTML attribute names are ASCII case insensitive, so href and HREF on the
20+
// same element are the same attribute.
21+
const code = '<a href="a" HREF="b">bbb</a>'
22+
const messages = HTMLHint.verify(code, ruleOptions)
23+
expect(messages.length).toBe(1)
24+
expect(messages[0].rule.id).toBe(ruleId)
25+
})
26+
1827
it('Attribute name not been duplication should not result in an error', () => {
1928
const code = '<a href="a">bbb</a>'
2029
const messages = HTMLHint.verify(code, ruleOptions)

‎test/rules/src-not-empty.spec.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,20 @@ describe(`Rules: ${ruleId}`, () => {
2020
expect(messages.length).toBe(0)
2121
})
2222

23+
it('Src be empty should result in an error whatever the case', () => {
24+
// HTML tag and attribute names are ASCII case insensitive, so these are four
25+
// spellings of one bug and all four have to be reported.
26+
const code = '<img src="" /><img SRC="" /><IMG src="" /><IMG SRC="" />'
27+
const messages = HTMLHint.verify(code, ruleOptions)
28+
expect(messages.length).toBe(4)
29+
})
30+
31+
it('Uppercase href and data should result in an error', () => {
32+
const code = '<LINK HREF="" type="text/css" /><OBJECT DATA="">'
33+
const messages = HTMLHint.verify(code, ruleOptions)
34+
expect(messages.length).toBe(2)
35+
})
36+
2337
it('Src be not set value should not result in an error', () => {
2438
const code =
2539
'<img width="200" /><script></script><link type="text/css" /><embed width="200"><bgsound /><iframe width="200"><object width="200">'

0 commit comments

Comments
 (0)