Skip to content

Commit

Permalink
fix(parser): make attribute re-declaration a warning
Browse files Browse the repository at this point in the history
Technically a strict XML fail. However due to the way we expose
attributes via a lazy getter it may be hard for library users to
properly deal with that error (as it is being logged out-of-bounds).
  • Loading branch information
nikku committed Jan 9, 2018
1 parent 27eb48f commit 75b6bb2
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
8 changes: 6 additions & 2 deletions parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ function Saxen(options) {
alias,
name,
attrs = {},
seenAttrs = {},
w,
j;

Expand Down Expand Up @@ -385,11 +386,14 @@ function Saxen(options) {
// advance cursor to next attribute
i = j + 1;

if (attrs[name]) {
handleError('attribute <' + name + '> already defined');
// check attribute re-declaration
if (name in seenAttrs) {
handleWarning('attribute <' + name + '> already defined');
return cachedAttrs = false;
}

seenAttrs[name] = true;

if (!isNamespace) {
attrs[name] = value;
continue;
Expand Down
32 changes: 27 additions & 5 deletions test/elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ test({
xml: '<e:root xmlns:e="http://extensions" xmlns:e="http://other" />',
ns: true,
expect: [
['error', 'attribute <xmlns:e> already defined'],
['warn', 'attribute <xmlns:e> already defined'],
['openTag', 'e:root', false],
['closeTag', 'e:root']
],
Expand All @@ -990,18 +990,40 @@ test({
xml: '<root xmlns="http://extensions" xmlns="http://other" />',
ns: true,
expect: [
['error', 'attribute <xmlns> already defined'],
['warn', 'attribute <xmlns> already defined'],
['openTag', 'ns0:root', false],
['closeTag', 'ns0:root']
],
});

// local attribute re-declaration / default namespace
// local attribute re-declaration / no ns
test({
xml: '<root a="A" a="B" />',
expect: [
['error', 'attribute <a> already defined'],
['warn', 'attribute <a> already defined'],
['openTag', 'root', false],
['closeTag', 'root']
],
});
});

// local attribute re-declaration / with namespace
test({
xml: '<root xmlns="http://extensions" a="A" a="B" />',
ns: true,
expect: [
['warn', 'attribute <a> already defined'],
['openTag', 'ns0:root', false],
['closeTag', 'ns0:root']
],
});

// local attribute re-declaration / with other namespace
test({
xml: '<root xmlns="http://extensions" xmlns:bar="http://extensions" bar:a="A" bar:a="B" />',
ns: true,
expect: [
['warn', 'attribute <bar:a> already defined'],
['openTag', 'ns0:root', false],
['closeTag', 'ns0:root']
],
});

0 comments on commit 75b6bb2

Please sign in to comment.