Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse attribute "constructor" crash #359

Closed
Particle60 opened this issue Dec 24, 2021 · 1 comment · Fixed by #315
Closed

parse attribute "constructor" crash #359

Particle60 opened this issue Dec 24, 2021 · 1 comment · Fixed by #315
Assignees
Labels
duplicate This issue or pull request already exists

Comments

@Particle60
Copy link

if a node has an attribute "constructor", xmldom will crash. For Example:

<?xml version="1.0" encoding="UTF-8"?>
<cofluent:Application >
    <refinement>
        <models constructor="printf(&quot;hello\n&quot;);" >
        </models>
    </refinement>
</cofluent:Application>

The bug is in lib/sax.js

function parseElementStartPart(source,start,el,currentNSMap,entityReplacer,errorHandler){

	/**
	 * @param {string} qname
	 * @param {string} value
	 * @param {number} startIndex
	 */
	function addAttribute(qname, value, startIndex) {
		if (qname in el.attributeNames) errorHandler.fatalError('Attribute ' + qname + ' redefined')
		el.addValue(qname, value, startIndex)
	}

(qname in el.attributeNames) will return true even el.attributeNames === {}.
"constructor" in {} will return true.

@karfau karfau self-assigned this Dec 24, 2021
@karfau karfau added bug Something isn't working duplicate This issue or pull request already exists labels Dec 24, 2021
@karfau
Copy link
Member

karfau commented Dec 24, 2021

Thank you for reporting it, but I wonder what version of xmldom (or rather @xmldom/xmldom) you are using, since this was fixed in #315 and released in version 0.7.4 (4 months ago).

See the current version of the code

And I verified that your XML can be parsed in 0.7.4: https://stackblitz.com/edit/js-xmldom359?devtoolsheight=33&file=index.js
And checked that the issue is present in xmldom@>=0.5.0 and @xmldom/xmldom@<0.7.4.

@karfau karfau closed this as completed Dec 24, 2021
@karfau karfau linked a pull request Dec 24, 2021 that will close this issue
@karfau karfau modified the milestone: 0.7.4 Dec 24, 2021
@karfau karfau removed the bug Something isn't working label Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants