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

'<' and '>' are not escaped in attribute values #198

Closed
zy-serguei opened this issue Mar 12, 2021 · 3 comments · Fixed by #199
Closed

'<' and '>' are not escaped in attribute values #198

zy-serguei opened this issue Mar 12, 2021 · 3 comments · Fixed by #199
Assignees
Labels
bug Something isn't working spec:XML https://www.w3.org/TR/xml11/ xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed
Milestone

Comments

@zy-serguei
Copy link

Serializer does not escape < and > characters in attribute values
const doc = new DOMParser().parseFromString('<a a="&lt;&gt;"/>');
const result = new XMLSerializer().serializeToString(doc);
result is <a a="<>"/>, which is invalid xml

@karfau karfau added bug Something isn't working spec:XML https://www.w3.org/TR/xml11/ xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed labels Mar 13, 2021
karfau added a commit to karfau/xmldom that referenced this issue Mar 13, 2021
to produce well formed XML.

> Well-formedness constraint: No `<` in Attribute Values
> The replacement text of any entity referred to directly or indirectly in an attribute value must not contain a `<`.

https://www.w3.org/TR/xml/#CleanAttrVals
https://www.w3.org/TR/xml/#NT-AttValue

fixes xmldom#198
@karfau
Copy link
Member

karfau commented Mar 13, 2021

@zy-serguei thx for reporting this.

Just some details about why I didn't also add escaping for >:

According to the specs I linked in the PR #199, the < not being part of attribute values is a well-formedness constraint (so it's not about validity of the XML).
But this is not true for > in attribute values, or at least I couldn't find anything about it.

We are dealing with similar issues in the same way, see #58 .

@karfau karfau added this to the 0.6.0 milestone Mar 13, 2021
@karfau karfau self-assigned this Mar 13, 2021
karfau added a commit that referenced this issue Mar 14, 2021
* fix: Escape `<` when serializing attribute values

to produce well formed XML.

> Well-formedness constraint: No `<` in Attribute Values
> The replacement text of any entity referred to directly or indirectly in an attribute value must not contain a `<`.

https://www.w3.org/TR/xml/#CleanAttrVals
https://www.w3.org/TR/xml/#NT-AttValue

fixes #198

Co-authored-by: Chris Brody <chris.brody+brodybits@gmail.com>
calebmshafer pushed a commit to iTwin/itwinjs-core that referenced this issue Aug 4, 2021
…1974)

* bump xmldom to 0.6.0 to avoid bug where it keeps "<" in attributes
see xmldom/xmldom#198

* add test which fails on version of 0.5.0 because of bad serialization
* add valid entity serialization test
* move serialization test down to serializer package instead of up at transformer level
* better test name not referencing round trip

Co-authored-by: Michael Belousov <MichaelBelousov@users.noreply.github.com>
mergify bot pushed a commit to iTwin/itwinjs-core that referenced this issue Aug 4, 2021
…1974)

* bump xmldom to 0.6.0 to avoid bug where it keeps "<" in attributes
see xmldom/xmldom#198

* add test which fails on version of 0.5.0 because of bad serialization
* add valid entity serialization test
* move serialization test down to serializer package instead of up at transformer level
* better test name not referencing round trip

Co-authored-by: Michael Belousov <MichaelBelousov@users.noreply.github.com>
(cherry picked from commit 97a229f)

# Conflicts:
#	common/config/rush/pnpm-lock.yaml
skirby1996 pushed a commit to iTwin/itwinjs-core that referenced this issue Aug 4, 2021
…ackport #1974) (#1994)

* bump xmldom to 0.6.0 to avoid bug where it keeps "<" in attributes (#1974)

* bump xmldom to 0.6.0 to avoid bug where it keeps "<" in attributes
see xmldom/xmldom#198

* add test which fails on version of 0.5.0 because of bad serialization
* add valid entity serialization test
* move serialization test down to serializer package instead of up at transformer level
* better test name not referencing round trip

Co-authored-by: Michael Belousov <MichaelBelousov@users.noreply.github.com>
(cherry picked from commit 97a229f)

# Conflicts:
#	common/config/rush/pnpm-lock.yaml

* pushed merged pnpm-lock

Co-authored-by: Michael Belousov <mike.belousov@bentley.com>
Co-authored-by: Michael Belousov <MichaelBelousov@users.noreply.github.com>
khanaffan pushed a commit to iTwin/itwinjs-core that referenced this issue Aug 10, 2021
…1974)

* bump xmldom to 0.6.0 to avoid bug where it keeps "<" in attributes
see xmldom/xmldom#198

* add test which fails on version of 0.5.0 because of bad serialization
* add valid entity serialization test
* move serialization test down to serializer package instead of up at transformer level
* better test name not referencing round trip

Co-authored-by: Michael Belousov <MichaelBelousov@users.noreply.github.com>
khanaffan pushed a commit to iTwin/itwinjs-core that referenced this issue Aug 10, 2021
…1974)

* bump xmldom to 0.6.0 to avoid bug where it keeps "<" in attributes
see xmldom/xmldom#198

* add test which fails on version of 0.5.0 because of bad serialization
* add valid entity serialization test
* move serialization test down to serializer package instead of up at transformer level
* better test name not referencing round trip

Co-authored-by: Michael Belousov <MichaelBelousov@users.noreply.github.com>
@retorquere
Copy link

According to the specs I linked in the PR #199, the < not being part of attribute values is a well-formedness constraint (so it's not about validity of the XML). But this is not true for > in attribute values, or at least I couldn't find anything about it.

Is there a way I can force escaping of > in attribute values? Right now the behavior of this package gives us different output than when we use the parser/serializer in Firefox.

@karfau
Copy link
Member

karfau commented Jan 24, 2022

@retorquere No, there is no such option at the moment.

Please add any relevant information that is not already part of the discussion to #58, since this issue has already been closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working spec:XML https://www.w3.org/TR/xml11/ xml:not well-formed https://www.w3.org/TR/xml11/#dt-wellformed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants