Skip to content

fix: report fatalError when doctype is inside elements#550

Merged
karfau merged 9 commits into
masterfrom
fuzz/prevent-doctype-inside-elements
Oct 4, 2023
Merged

fix: report fatalError when doctype is inside elements#550
karfau merged 9 commits into
masterfrom
fuzz/prevent-doctype-inside-elements

Conversation

@karfau

@karfau karfau commented Oct 4, 2023

Copy link
Copy Markdown
Member

which is a regression compared to 0.8.x, most likely introduced as part of #498

  • add check to parseDoctypeCommentOrCData
  • drop redundant and broken Element.appendChild implementation
  • hasInsertableNodeType now checks for CharacterData nodes instead of only text nodes
  • align ParseError and DOMException in how they are extending Error
  • wrap DOMExceptions in ParseError in sax parser
  • move custom errors to own module
    • and allow current and modern constructor arguments for DOMException

@codecov

codecov Bot commented Oct 4, 2023

Copy link
Copy Markdown

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (99602b4) 92.77% compared to head (9665f1c) 93.88%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #550      +/-   ##
==========================================
+ Coverage   92.77%   93.88%   +1.10%     
==========================================
  Files           7        8       +1     
  Lines        2035     2044       +9     
  Branches      530      531       +1     
==========================================
+ Hits         1888     1919      +31     
+ Misses        147      125      -22     
Files Coverage Δ
lib/conventions.js 100.00% <ø> (ø)
lib/dom-parser.js 93.56% <100.00%> (+0.03%) ⬆️
lib/errors.js 100.00% <100.00%> (ø)
lib/index.js 100.00% <100.00%> (ø)
lib/sax.js 98.18% <100.00%> (+0.01%) ⬆️
lib/dom.js 90.06% <84.61%> (+1.57%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

karfau added 5 commits October 4, 2023 14:53
- `hasInsertableNodeType` now checks for `CharacterData` nodes instead of only text nodes
- wrap DOMExceptions in ParseError in sax parser
and allow current and modern constructor arguments for DOMException
and cover more DOMException cases
Comment thread test/dom/document.test.js Fixed
Comment thread test/dom/node.test.js Fixed
Comment thread test/dom/node.test.js Fixed
Comment thread test/dom/node.test.js Fixed
Comment thread test/errors/expectDOMException.js Fixed
Comment thread test/errors/expectDOMException.js Fixed
@karfau karfau marked this pull request as ready for review October 4, 2023 20:21

@karfau karfau left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self review completed

@karfau karfau merged commit 642c9e8 into master Oct 4, 2023
@karfau karfau deleted the fuzz/prevent-doctype-inside-elements branch October 4, 2023 20:27
@karfau karfau added this to the 0.9.0 milestone Oct 4, 2023
@karfau karfau added xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed testing error handling fuzzer-finding labels Oct 4, 2023
karfau added a commit that referenced this pull request Oct 13, 2024
as part of #550
("drop redundant and broken `Element.appendChild` implementation")
the `_onUpdateChild` was not passed the added child,
so the `childNodes` were re-indexed from the `firstChild` each time a node was appended.

Now that the added child is being passed,
the performance in 0.9.0 is still not as good as in 0.8.*,
but this is because of the higher amount of checks being executed before adding a node.

fixes #748
karfau added a commit that referenced this pull request Oct 13, 2024
as part of #550 ("drop redundant
and broken `Element.appendChild` implementation")
the `_onUpdateChild` was not passed the added child, so the `childNodes`
were "re-indexed" from the `firstChild` each time a node was appended.
This is why the performance degraded significantly, when there are a lot
of childNodes to be appended.

fixes #748
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

error handling fuzzer-finding testing xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants