Skip to content

gh-135661: Fix CDATA section parsing in HTMLParser #135665

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.

Sign up for GitHub

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

gh-135661: Fix CDATA section parsing in HTMLParser #135665

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f7f9f56
gh-135661: Fix CDATA section parsing in HTMLParser
serhiy-storchaka Jun 18, 2025
816f34e
Merge branch 'main' into htmlparser-cdata
serhiy-storchaka Jul 3, 2025
cf918e3
Move to Security.
serhiy-storchaka Jul 3, 2025
d346c10
Update 2025-06-18-13-34-55.gh-issue-135661.NZlpWf.rst
serhiy-storchaka Jul 4, 2025
9e1ae33
Merge branch 'main' into htmlparser-cdata
serhiy-storchaka Jul 4, 2025
524cac5
* Make CDATA section parsing context depending.
serhiy-storchaka Jul 5, 2025
2a1bb46
Merge branch 'main' into htmlparser-cdata
serhiy-storchaka Jul 22, 2025
8cdbc95
Enable support of CDATA sections by default.
serhiy-storchaka Jul 22, 2025
e4f13a8
Merge branch 'main' into htmlparser-cdata
serhiy-storchaka Aug 3, 2025
50fd4b3
Make the setter method private.
serhiy-storchaka Aug 3, 2025
165fd1e
Update NEWS.
serhiy-storchaka Aug 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
gh-135661: Fix CDATA section parsing in HTMLParser 
"] ]>" and "]] >" no longer end the CDATA section.
  • Loading branch information
@serhiy-storchaka
serhiy-storchaka committed Jun 18, 2025
commit f7f9f562f1b31c2130e26269cf4f196f378d80f2
6 changes: 5 additions & 1 deletion Lib/html/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,11 @@ def parse_html_declaration(self, i):
# this case is actually already handled in goahead()
return self.parse_comment(i)
elif rawdata[i:i+9] == '<![CDATA[':
return self.parse_marked_section(i)
j = rawdata.find(']]>')
if j < 0:
return -1
self.unknown_decl(rawdata[i+3: j])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.unknown_decl(rawdata[i+3: j])
self.unknown_decl(rawdata[i+3:j])

return j + 3
Copy link
Member Author

Choose a reason for hiding this comment

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

According to the HTML5 standard (https://html.spec.whatwg.org/multipage/parsing.html#markup-declaration-open-state), it should be either data or bogus comment (which ends with >, not ]]>), but this depends on the context. It may be that I incorrectly understand the HTML5 standard, because this part is difficult to implement.

Copy link
Member

@ezio-melotti ezio-melotti Jul 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I tried copying the content of the tests in the following file:

<!DOCTYPE html>
<html>
<body>
<![CDATA[just some plain text]]><hr>
<![CDATA[<!-- not a comment -->]]><hr>
<![CDATA[&not-an-entity-ref;]]><hr>
<![CDATA[<not a='start tag'>]]><hr>
<![CDATA[]]><hr>
<![CDATA[[[I have many brackets]]]]><hr>
<![CDATA[I have a > in the middle]]><hr>
<![CDATA[I have a ]] in the middle]]><hr>
<![CDATA[] ]>]]><hr>
<![CDATA[]] >]]><hr>
<![CDATA[
    if (a < b && a > b) {
        printf("[<marquee>How?</marquee>]");
    }
]]><hr>

</body>
</html>

and this was the result on Firefox:

<html><head></head><body>
<!--[CDATA[just some plain text]]--><hr>
<!--[CDATA[<!-- not a comment ---->]]&gt;<hr>
<!--[CDATA[&not-an-entity-ref;]]--><hr>
<!--[CDATA[<not a='start tag'-->]]&gt;<hr>
<!--[CDATA[]]--><hr>
<!--[CDATA[[[I have many brackets]]]]--><hr>
<!--[CDATA[I have a --> in the middle]]&gt;<hr>
<!--[CDATA[I have a ]] in the middle]]--><hr>
<!--[CDATA[] ]-->]]&gt;<hr>
<!--[CDATA[]] -->]]&gt;<hr>
<!--[CDATA[
    if (a < b && a --> b) {
        printf("[<marquee>How?</marquee>]");
    }
]]&gt;<hr>



</body></html>
Image

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and if you try <svg><text y="100"><![CDATA[foo<br>bar]]></text></svg>, you will see that content between <![CDATA[ and ]]> is interpreted as a raw data.

This is context dependent.

HTMLParser is actually just a tokenizer. To determine the context automatically, it needs to support the stack of open elements and to know what elements are in the HTML namespace. This is all in the specification, and we will implement this in future. But this is a different level of complexity. So I solved the issue by letting the user to determine the context. New method support_cdata() sets how HTMLParser will parse CDATA. This is not good, but perhaps better than the current state.

elif rawdata[i:i+9].lower() == '<!doctype':
# find the closing >
gtpos = rawdata.find('>', i+9)
Expand Down
42 changes: 21 additions & 21 deletions Lib/test/test_htmlparser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,27 +686,27 @@ def test_broken_condcoms(self):
]
self._run_check(html, expected)

def test_cdata_declarations(self):
# More tests should be added. See also "8.2.4.42. Markup
# declaration open state", "8.2.4.69. CDATA section state",
# and issue 32876
html = ('<![CDATA[just some plain text]]>')
expected = [('unknown decl', 'CDATA[just some plain text')]
self._run_check(html, expected)

def test_cdata_declarations_multiline(self):
html = ('<code><![CDATA['
' if (a < b && a > b) {'
' printf("[<marquee>How?</marquee>]");'
' }'
']]></code>')
expected = [
('starttag', 'code', []),
('unknown decl',
'CDATA[ if (a < b && a > b) { '
'printf("[<marquee>How?</marquee>]"); }'),
('endtag', 'code')
]
@support.subTests('content', [
'just some plain text',
'<!-- not a comment -->',
'&not-an-entity-ref;',
"<not a='start tag'>",
'',
'[[I have many brackets]]',
'I have a > in the middle',
'I have a ]] in the middle',
'] ]>',
']] >',
('\n'
' if (a < b && a > b) {\n'
' printf("[<marquee>How?</marquee>]");\n'
' }\n'),
])
def test_cdata_section(self, content):
# See "13.2.5.42 Markup declaration open state",
# "13.2.5.69 CDATA section state", and issue bpo-32876.
html = f'<![CDATA[{content}]]>'
expected = [('unknown decl', 'CDATA[' + content)]
self._run_check(html, expected)

def test_convert_charrefs_dropped_text(self):
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2025-06-18-13-34-55.gh-issue-135661.NZlpWf.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix CDATA section parsing in :class:`html.parser.HTMLParser`: ``] ]>`` and
``]] >`` no longer end the CDATA section.
Loading