GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-146169: Prevent re-entrant Parse() calls in pyexpat by okiemute04 · Pull Request #146179 · python/cpython

Conversation

okiemute04 commented Mar 19, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Fixes #146169

Add a check in Parse() to prevent calls when in_callback is true,
as this violates expat's requirements and can cause crashes. Now raises
RuntimeError with a clear message.

Added tests to test_pyexpat.py to verify the fix:

test_reentrant_parse_crash: Verifies re-entrant calls raise RuntimeError
test_parse_normal: Ensures normal parsing still works

Issue: Segfault/UB from expat when re-entering the XML Parser #146169

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added the awaiting review label

Mar 19, 2026

okiemute04 mentioned this pull request

Mar 19, 2026

gh-145866 : Update JIT contributor list (GH-146170) #146177

Closed

okiemute04 force-pushed the fix-pyexpat-reentrant-crash-final branch 3 times, most recently from 2368c9d to 4557157 Compare

March 19, 2026 17:10

okiemute04 changed the title ~~gh-146174: Prevent re-entrant Parse() calls in pyexpat~~ gh-146169: Prevent re-entrant Parse() calls in pyexpat

Mar 19, 2026

bedevere-app Bot mentioned this pull request

Mar 19, 2026

Segfault/UB from expat when re-entering the XML Parser #146169

Open

okiemute04 force-pushed the fix-pyexpat-reentrant-crash-final branch 2 times, most recently from afe297f to 4146ff1 Compare

March 19, 2026 17:22

aisk reviewed

Mar 19, 2026

View reviewed changes

Comment thread

Misc/NEWS.d/next/Library/2026-03-19-14-45-00.gh-issue-146169.XXXXXX.rst Outdated

Comment on lines +1 to +4

		.. gh-issue: 146169

		.. section: Library

aisk Mar 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change

	.. gh-issue: 146169

	.. section: Library

The news entry format is invalid, these should be removed. The filename is also wrong. I think you can delete this file, and re-generate it by blurb-it or the blurb command line tool.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

okiemute04 force-pushed the fix-pyexpat-reentrant-crash-final branch 2 times, most recently from 00844fd to b6c2df5 Compare

March 19, 2026 17:32


          pythongh-146174: Prevent re-entrant Parse() calls in pyexpat

c924f57

Add a check in Parse() to prevent calls when in_callback is true,
as this violates expat's requirements and can cause crashes. Now raises
RuntimeError with a clear message.

Add tests to verify the fix and ensure normal parsing still works.

okiemute04 force-pushed the fix-pyexpat-reentrant-crash-final branch from b6c2df5 to c924f57 Compare

March 19, 2026 17:35

aisk commented Mar 19, 2026

Copy link

Copy Markdown

Member

Hi @okiemute04 please avoid force push in the future, which will make the review work harder, see the Dev Guide: https://devguide.python.org/getting-started/pull-request-lifecycle/#don-t-force-push

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aisk reviewed

Mar 19, 2026

View reviewed changes

Comment thread

Lib/test/test_pyexpat.py Outdated

		expat.errors.XML_ERROR_FINISHED)

		def test_reentrant_parse_crash(self):
		from xml.parsers import expat

aisk Mar 19, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this is already imported at the begaining of this file: https://github.com/python/cpython/blob/main/Lib/test/test_pyexpat.py#L18

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

okiemute04 Mar 19, 2026

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yes thats true, Thanks @aisk i already updated the pr with a new push to remove the import.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

okiemute04 added 3 commits

March 19, 2026 10:43


          Add NEWS entry for pyexpat re-entrant fix

0e67052


          Add NEWS entry for pyexpat re-entrant fix

029e408


          Merge remote branch to resolve history divergence

57faef4

aisk mentioned this pull request

Mar 20, 2026

gh-146194: Fix nested KeyboardInterrupt handling in asyncio #146203

Closed

picnixz requested changes

Mar 20, 2026

View reviewed changes

picnixz left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please address the comments.

I suspect that this has been generated by an LLM. If an LLM replies to me I will not consider this PR as being valid as this goes against our ToS about LLMs. So carefully address those comments and properly review any LLM output before committing anything.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_pyexpat.py

		p = expat.ParserCreate()
		data = "<root><child/></root>".encode('utf-8')
		try:
		p.Parse(data, 1)

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Don't use 0 or 1 for the second argument of parse but real booleans.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_pyexpat.py

		data = "<root><child/></root>".encode('utf-8')
		try:
		p.Parse(data, 1)
		except RuntimeError:

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Remove this. If the test fails it must fail with the RuntimeError or any other exception that was unexpected

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_pyexpat.py

		self.assertEqual(str(cm.exception),
		"cannot call Parse() from within a handler")

		def test_parse_normal(self):

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why do we even need this test in the first place? we already test this somewhere don't we?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Well yes that is what I am asking.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_pyexpat.py

Comment on lines +288 to +292

		data = b"\xff\xfe<\x00a\x00>\x00x\x00"
		with self.assertRaises(RuntimeError) as cm:
		for i in range(len(data)):
		p.Parse(data[i:i+1], i == len(data) - 1)
		self.assertEqual(str(cm.exception),

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why this complex test? just have p.Parse(data, False) and that's it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_pyexpat.py

		p.CharacterDataHandler = handler

		p.StartElementHandler = start
		data = b"\xff\xfe<\x00a\x00>\x00x\x00"

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why this payload?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

But you do not need such payload or? It should be triggered by just reparsing the same data in the handler.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_pyexpat.py

		expat.errors.XML_ERROR_FINISHED)

		def test_reentrant_parse_crash(self):
		p = expat.ParserCreate(encoding="utf-16")

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

why using utf16?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It is wrong. We just want to check for reentrancy. Handler doesn't care about the payload encoding as long as it is being called.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/pyexpat.c


		if (self->in_callback) {
		PyErr_SetString(PyExc_RuntimeError,
		"cannot call Parse() from within a handler");

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Are other re-entrant calls raising a RuntimeError or something else?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

picnixz Mar 20, 2026

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ok this is clearly an LLM answer. Since it will be faster if I were to address this I am closing this PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot added awaiting changes and removed awaiting review labels

Mar 20, 2026

bedevere-app Bot commented Mar 20, 2026

Copy link

Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

picnixz closed this

Mar 20, 2026

picnixz commented Mar 20, 2026

Copy link

Copy Markdown

Member

If you want to contribute with an LLM agent:

read our policy in the devguide
always review the code
never let an agent reply in your stead

If you do not follow those guidelines we will restrict your access.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Reviewers

aisk

aisk left review comments

picnixz

picnixz requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Segfault/UB from expat when re-entering the XML Parser

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

okiemute04 commented Mar 19, 2026 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aisk commented Mar 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app Bot commented Mar 20, 2026

Uh oh!

picnixz commented Mar 20, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

okiemute04 commented Mar 19, 2026 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.