Skip to content

Commit

Permalink
XMLSerializer.serializeToString() doesn't properly escape \n, \n and \t
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=227844

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Rebaseline WPT test now that one more subtest is passing.

* web-platform-tests/domparsing/XMLSerializer-serializeToString-expected.txt:

Source/WebCore:

XMLSerializer.serializeToString() doesn't properly escape \n, \n and \t.

This is causing the "check XMLSerializer.serializeToString escapes attribute values for roundtripping" subtest to fail in WebKit on:
http://wpt.live/domparsing/XMLSerializer-serializeToString.html

Chrome and Firefox both escape these and pass this WPT subtest.

The specification does not indicate we should escape those:
- https://w3c.github.io/DOM-Parsing/#dfn-serializing-an-attribute-value
But there is an open bug about this:
- w3c/DOM-Parsing#59

No new tests, rebaselined existing test.

* editing/MarkupAccumulator.cpp:
(WebCore::elementCannotHaveEndTag):
* editing/MarkupAccumulator.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@279815 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
[email protected] committed Jul 11, 2021
1 parent bab137f commit a8eeda9
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 3 deletions.
11 changes: 11 additions & 0 deletions LayoutTests/imported/w3c/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
2021-07-11 Chris Dumez <[email protected]>

XMLSerializer.serializeToString() doesn't properly escape \n, \n and \t
https://bugs.webkit.org/show_bug.cgi?id=227844

Reviewed by Darin Adler.

Rebaseline WPT test now that one more subtest is passing.

* web-platform-tests/domparsing/XMLSerializer-serializeToString-expected.txt:

2021-07-10 Chris Dumez <[email protected]>

document.readyState should be "complete" after calling DOMParser.parseFromString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ FAIL Check if an attribute with namespace and no prefix is serialized with the n
PASS Check if the prefix of an attribute is replaced with another existing prefix mapped to the same namespace URI.
FAIL Check if the prefix of an attribute is NOT preserved in a case where neither its prefix nor its namespace URI is not already used. assert_equals: expected "<r xmlns:xx=\"uri\" xmlns:ns1=\"uri2\" ns1:name=\"value\"/>" but got "<r xmlns:xx=\"uri\" p:name=\"value\" xmlns:p=\"uri2\"/>"
FAIL Check if the prefix of an attribute is replaced with a generated one in a case where the prefix is already mapped to a different namespace URI. assert_equals: expected "<r xmlns:xx=\"uri\" xmlns:ns1=\"uri2\" ns1:name=\"value\"/>" but got "<r xmlns:xx=\"uri\" NS1:name=\"value\" xmlns:NS1=\"uri2\"/>"
FAIL check XMLSerializer.serializeToString escapes attribute values for roundtripping assert_in_array: value "<root attr=\"\t\"/>" not in array ["<root attr=\"&#9;\"/>", "<root attr=\"&#x9;\"/>"]
PASS check XMLSerializer.serializeToString escapes attribute values for roundtripping
FAIL Check if attribute serialization takes into account of following xmlns:* attributes assert_equals: expected "<root xmlns:ns1=\"uri1\" ns1:foobar=\"value1\" xmlns:p=\"uri2\"/>" but got "<root p:foobar=\"value1\" xmlns:p=\"uri1\" xmlns:p=\"uri2\"/>"
FAIL Check if attribute serialization takes into account of the same prefix declared in an ancestor element assert_equals: expected "<root xmlns:p=\"uri1\"><child xmlns:ns1=\"uri2\" ns1:foobar=\"v\"/></root>" but got "<root xmlns:p=\"uri1\"><child NS1:foobar=\"v\" xmlns:NS1=\"uri2\"/></root>"
FAIL Check if start tag serialization drops element prefix if the namespace is same as inherited default namespace. assert_equals: expected "<root xmlns=\"u1\"><child xmlns:p=\"u1\"/></root>" but got "<root xmlns=\"u1\"><p:child xmlns:p=\"u1\"/></root>"
Expand Down
25 changes: 25 additions & 0 deletions Source/WebCore/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,28 @@
2021-07-11 Chris Dumez <[email protected]>

XMLSerializer.serializeToString() doesn't properly escape \n, \n and \t
https://bugs.webkit.org/show_bug.cgi?id=227844

Reviewed by Darin Adler.

XMLSerializer.serializeToString() doesn't properly escape \n, \n and \t.

This is causing the "check XMLSerializer.serializeToString escapes attribute values for roundtripping" subtest to fail in WebKit on:
http://wpt.live/domparsing/XMLSerializer-serializeToString.html

Chrome and Firefox both escape these and pass this WPT subtest.

The specification does not indicate we should escape those:
- https://w3c.github.io/DOM-Parsing/#dfn-serializing-an-attribute-value
But there is an open bug about this:
- https://github.com/w3c/DOM-Parsing/issues/59

No new tests, rebaselined existing test.

* editing/MarkupAccumulator.cpp:
(WebCore::elementCannotHaveEndTag):
* editing/MarkupAccumulator.h:

2021-07-10 Chris Dumez <[email protected]>

document.readyState should be "complete" after calling DOMParser.parseFromString()
Expand Down
13 changes: 12 additions & 1 deletion Source/WebCore/editing/MarkupAccumulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ static const EntityDescription entitySubstitutionList[] = {
{ "&gt;", 4, EntityGt },
{ "&quot;", 6, EntityQuot },
{ "&nbsp;", 6, EntityNbsp },
{ "&#9;", 4, EntityTab },
{ "&#10;", 5, EntityLineFeed },
{ "&#13;", 5, EntityCarriageReturn },
};

enum EntitySubstitutionIndex {
Expand All @@ -69,11 +72,19 @@ enum EntitySubstitutionIndex {
EntitySubstitutionGtIndex = 3,
EntitySubstitutionQuotIndex = 4,
EntitySubstitutionNbspIndex = 5,
EntitySubstitutionTabIndex = 6,
EntitySubstitutionLineFeedIndex = 7,
EntitySubstitutionCarriageReturnIndex = 8,
};

static const unsigned maximumEscapedentityCharacter = noBreakSpace;
static const uint8_t entityMap[maximumEscapedentityCharacter + 1] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0,
EntitySubstitutionTabIndex, // '\t'.
EntitySubstitutionLineFeedIndex, // '\n'.
0, 0,
EntitySubstitutionCarriageReturnIndex, // '\r'.
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
EntitySubstitutionQuotIndex, // '"'.
0, 0, 0,
EntitySubstitutionAmpIndex, // '&'.
Expand Down
5 changes: 4 additions & 1 deletion Source/WebCore/editing/MarkupAccumulator.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,16 @@ enum EntityMask {
EntityGt = 0x0004,
EntityQuot = 0x0008,
EntityNbsp = 0x0010,
EntityTab = 0x0020,
EntityLineFeed = 0x0040,
EntityCarriageReturn = 0x0080,

// Non-breaking space needs to be escaped in innerHTML for compatibility reason. See http://trac.webkit.org/changeset/32879
// However, we cannot do this in a XML document because it does not have the entity reference defined (See the bug 19215).
EntityMaskInCDATA = 0,
EntityMaskInPCDATA = EntityAmp | EntityLt | EntityGt,
EntityMaskInHTMLPCDATA = EntityMaskInPCDATA | EntityNbsp,
EntityMaskInAttributeValue = EntityAmp | EntityLt | EntityGt | EntityQuot,
EntityMaskInAttributeValue = EntityAmp | EntityLt | EntityGt | EntityQuot | EntityTab | EntityLineFeed | EntityCarriageReturn,
EntityMaskInHTMLAttributeValue = EntityAmp | EntityQuot | EntityNbsp,
};

Expand Down

0 comments on commit a8eeda9

Please sign in to comment.