Skip to content

gh-140875: Fix handling of unclosed charrefs before EOF in HTMLParser #140904

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

Merged
serhiy-storchaka merged 4 commits into from
Nov 19, 2025
Merged

gh-140875: Fix handling of unclosed charrefs before EOF in HTMLParser #140904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fe8bcb2
gh-140875: Fix handling of unclosed charrefs before EOF in HTMLParser
serhiy-storchaka Nov 2, 2025
8105625
Update a NEWS entry.
serhiy-storchaka Nov 2, 2025
ebc287b
Merge branch 'main' into htmlparser-eof-in-entityref
serhiy-storchaka Nov 19, 2025
5dedffc
Update tests.
serhiy-storchaka Nov 19, 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
29 changes: 19 additions & 10 deletions Lib/html/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

entityref = re.compile('&([a-zA-Z][-.a-zA-Z0-9]*)[^a-zA-Z0-9]')
charref = re.compile('&#(?:[0-9]+|[xX][0-9a-fA-F]+)[^0-9a-fA-F]')
incomplete_charref = re.compile('&#(?:[0-9]|[xX][0-9a-fA-F])')
attr_charref = re.compile(r'&(#[0-9]+|#[xX][0-9a-fA-F]+|[a-zA-Z][a-zA-Z0-9]*)[;=]?')

starttagopen = re.compile('<[a-zA-Z]')
Expand Down Expand Up @@ -304,10 +305,20 @@ def goahead(self, end):
k = k - 1
i = self.updatepos(i, k)
continue
match = incomplete_charref.match(rawdata, i)
if match:
if end:
self.handle_charref(rawdata[i+2:])
i = self.updatepos(i, n)
break
# incomplete
break
elif i + 3 < n: # larger than "&#x"
# not the end of the buffer, and can't be confused
# with some other construct
self.handle_data("&#")
Copy link
Member

@ezio-melotti ezio-melotti Nov 19, 2025

Choose a reason for hiding this comment

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

What's the reason for emitting &# as data now, instead of emitting it later with the rest of the data? IOW, in the case of '&x y', this will emit handle_data('&#') + handle_data(' y') instead of a single handle_data('&# y').

Having multiple handle_data is not wrong per se, but if we remove the elif block and let the else break, we can simplify the code and emit a single handle_data.

The same might apply below, where a single & is emitted. Also note that for ' z &x y', the first handle_data gets called with only 'z ', even if followed by one or more additional handle_data.

Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Nov 19, 2025

Choose a reason for hiding this comment

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

Try &#x &lt;. With this PR it emits handle_data('&#'), handle_data('x '), handle_entityref('lt') which is not optimal, but correct. If remove this elif, it will emit handle_data('&#x &lt;'), which is incorrect.

i = self.updatepos(i, i + 2)
else:
if ";" in rawdata[i:]: # bail by consuming &#
self.handle_data(rawdata[i:i+2])
i = self.updatepos(i, i+2)
break
elif startswith('&', i):
match = entityref.match(rawdata, i)
Expand All @@ -321,15 +332,13 @@ def goahead(self, end):
continue
match = incomplete.match(rawdata, i)
if match:
# match.group() will contain at least 2 chars
if end and match.group() == rawdata[i:]:
k = match.end()
if k <= i:
k = n
i = self.updatepos(i, i + 1)
if end:
self.handle_entityref(rawdata[i+1:])
i = self.updatepos(i, n)
break
# incomplete
break
elif (i + 1) < n:
elif i + 1 < n:
# not the end of the buffer, and can't be confused
# with some other construct
self.handle_data("&")
Expand Down
110 changes: 87 additions & 23 deletions Lib/test/test_htmlparser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,13 @@ def get_events(self):

class TestCaseBase(unittest.TestCase):

def get_collector(self):
return EventCollector(convert_charrefs=False)
def get_collector(self, convert_charrefs=False):
return EventCollector(convert_charrefs=convert_charrefs)

def _run_check(self, source, expected_events, collector=None):
def _run_check(self, source, expected_events,
*, collector=None, convert_charrefs=False):
if collector is None:
collector = self.get_collector()
collector = self.get_collector(convert_charrefs=convert_charrefs)
parser = collector
for s in source:
parser.feed(s)
Expand All @@ -128,7 +129,7 @@ def _run_check(self, source, expected_events, collector=None):

def _run_check_extra(self, source, events):
self._run_check(source, events,
EventCollectorExtra(convert_charrefs=False))
collector=EventCollectorExtra(convert_charrefs=False))


class HTMLParserTestCase(TestCaseBase):
Expand Down Expand Up @@ -187,10 +188,87 @@ def test_malformatted_charref(self):
])

def test_unclosed_entityref(self):
self._run_check("&entityref foo", [
("entityref", "entityref"),
("data", " foo"),
])
self._run_check('&gt &lt;', [('entityref', 'gt'), ('data', ' '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('&gt &lt;', [('data', '> <')], convert_charrefs=True)

self._run_check('&undefined &lt;',
[('entityref', 'undefined'), ('data', ' '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('&undefined &lt;', [('data', '&undefined <')],
convert_charrefs=True)

self._run_check('&gtundefined &lt;',
[('entityref', 'gtundefined'), ('data', ' '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('&gtundefined &lt;', [('data', '>undefined <')],
convert_charrefs=True)

self._run_check('& &lt;', [('data', '& '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('& &lt;', [('data', '& <')], convert_charrefs=True)

def test_eof_in_entityref(self):
self._run_check('&gt', [('entityref', 'gt')], convert_charrefs=False)
Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Nov 2, 2025

Choose a reason for hiding this comment

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

It was data before this change.

self._run_check('&gt', [('data', '>')], convert_charrefs=True)

self._run_check('&g', [('entityref', 'g')], convert_charrefs=False)
Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Nov 2, 2025

Choose a reason for hiding this comment

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

Ampersand was only swallowed in the case of 1-character name before EOF.

self._run_check('&g', [('data', '&g')], convert_charrefs=True)

self._run_check('&undefined', [('entityref', 'undefined')],
convert_charrefs=False)
self._run_check('&undefined', [('data', '&undefined')],
convert_charrefs=True)

self._run_check('&gtundefined', [('entityref', 'gtundefined')],
convert_charrefs=False)
self._run_check('&gtundefined', [('data', '>undefined')],
convert_charrefs=True)

self._run_check('&', [('data', '&')], convert_charrefs=False)
self._run_check('&', [('data', '&')], convert_charrefs=True)

def test_unclosed_charref(self):
self._run_check('&#123 &lt;', [('charref', '123'), ('data', ' '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('&#123 &lt;', [('data', '{ <')], convert_charrefs=True)
self._run_check('&#xab &lt;', [('charref', 'xab'), ('data', ' '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('&#xab &lt;', [('data', '\xab <')], convert_charrefs=True)

self._run_check('&#123456789 &lt;',
[('charref', '123456789'), ('data', ' '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('&#123456789 &lt;', [('data', '\ufffd <')],
convert_charrefs=True)
self._run_check('&#x123456789 &lt;',
[('charref', 'x123456789'), ('data', ' '), ('entityref', 'lt')],
convert_charrefs=False)
self._run_check('&#x123456789 &lt;', [('data', '\ufffd <')],
convert_charrefs=True)

self._run_check('&# &lt;', [('data', '&# '), ('entityref', 'lt')], convert_charrefs=False)
self._run_check('&# &lt;', [('data', '&# <')], convert_charrefs=True)
self._run_check('&#x &lt;', [('data', '&#x '), ('entityref', 'lt')], convert_charrefs=False)
self._run_check('&#x &lt;', [('data', '&#x <')], convert_charrefs=True)

def test_eof_in_charref(self):
self._run_check('&#123', [('charref', '123')], convert_charrefs=False)
self._run_check('&#123', [('data', '{')], convert_charrefs=True)
self._run_check('&#xab', [('charref', 'xab')], convert_charrefs=False)
self._run_check('&#xab', [('data', '\xab')], convert_charrefs=True)

self._run_check('&#123456789', [('charref', '123456789')],
convert_charrefs=False)
self._run_check('&#123456789', [('data', '\ufffd')], convert_charrefs=True)
self._run_check('&#x123456789', [('charref', 'x123456789')],
convert_charrefs=False)
self._run_check('&#x123456789', [('data', '\ufffd')], convert_charrefs=True)

self._run_check('&#', [('data', '&#')], convert_charrefs=False)
self._run_check('&#', [('data', '&#')], convert_charrefs=True)
self._run_check('&#x', [('data', '&#x')], convert_charrefs=False)
self._run_check('&#x', [('data', '&#x')], convert_charrefs=True)

def test_bad_nesting(self):
# Strangely, this *is* supposed to test that overlapping
Expand Down Expand Up @@ -762,20 +840,6 @@ def test_correct_detection_of_start_tags(self):
]
self._run_check(html, expected)

def test_EOF_in_charref(self):
# see #17802
# This test checks that the UnboundLocalError reported in the issue
# is not raised, however I'm not sure the returned values are correct.
# Maybe HTMLParser should use self.unescape for these
data = [
('a&', [('data', 'a&')]),
('a&b', [('data', 'ab')]),
Copy link
Member Author

@serhiy-storchaka serhiy-storchaka Nov 2, 2025

Choose a reason for hiding this comment

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

This is the reported bug. It was in the tests!

('a&b ', [('data', 'a'), ('entityref', 'b'), ('data', ' ')]),
('a&b;', [('data', 'a'), ('entityref', 'b')]),
]
for html, expected in data:
self._run_check(html, expected)

def test_eof_in_comments(self):
data = [
('<!--', [('comment', '')]),
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2025-11-02-10-44-23.gh-issue-140875.wt6B37.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix handling of unclosed character references (named and numerical)
followed by the end of file in :class:`html.parser.HTMLParser` with
``convert_charrefs=False``.
Loading