Skip to content

gh-149017: Upgrade bundled Expat to 2.8.0#149020

Merged
StanFromIreland merged 5 commits intopython:mainfrom
StanFromIreland:expat-2.8.0
Apr 27, 2026
Merged

gh-149017: Upgrade bundled Expat to 2.8.0#149020
StanFromIreland merged 5 commits intopython:mainfrom
StanFromIreland:expat-2.8.0

Conversation

@StanFromIreland
Copy link
Copy Markdown
Member

@StanFromIreland StanFromIreland commented Apr 26, 2026

Comment thread Modules/expat/refresh.sh
@hartwork
Copy link
Copy Markdown
Contributor

@StanFromIreland I think I should note that CPython defines XML_POOR_ENTROPY and passes entropy using an explicit call to XML_SetHashSalt (related to #149018). The new _pyexpat_random.c wrapper is in some conflict with that approach, I believe.

@StanFromIreland StanFromIreland marked this pull request as draft April 26, 2026 17:10
@StanFromIreland StanFromIreland marked this pull request as ready for review April 26, 2026 17:48
@StanFromIreland StanFromIreland requested a review from picnixz April 26, 2026 17:49
// bpo-30947: Python uses best available entropy sources to
// call XML_SetHashSalt(), expat entropy sources are not needed
#define XML_POOR_ENTROPY 1
#undef HAVE_ARC4RANDOM
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So we actually are using a poor entropy source and later we will use the entropy source defined by Expat? The BPO issue seems also unsure on whether we wanted this for the C accelerated module. If we still want a poor entropy, I wonder why we need to undef those HAVE_* constants.

Because we would hit (in xmlparse.c):

#if ! defined(HAVE_ARC4RANDOM_BUF) && ! defined(HAVE_ARC4RANDOM)

on Windows I think? I'm confused here about (1) why we want poor entropy (2) why we undef those macros.

Copy link
Copy Markdown
Member Author

@StanFromIreland StanFromIreland Apr 26, 2026

Choose a reason for hiding this comment

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

So we actually are using a poor entropy source and later we will use the entropy source defined by Expat?

IIUC, not exactly. We skip all the "good" Expat entropy sources as we override it anyway with XML_SetHashSalt.

Because we would hit (in xmlparse.c):

It only uses gettimeofday/GetSystemTimeAsFileTime which are in the respective system libraries.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But XML_SetHashSalt has poor entropy according to #149018 and we need to use the new interface, which uses the non-poor sources?

Copy link
Copy Markdown
Member Author

@StanFromIreland StanFromIreland Apr 26, 2026

Choose a reason for hiding this comment

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

XML_SetHashSalt16Bytes doesn’t use the new sources? It is just a setter.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh ok! so we produce the entropy and set it afterwards, om makes sense.

@StanFromIreland StanFromIreland requested a review from picnixz April 26, 2026 18:44
@StanFromIreland StanFromIreland merged commit 005555a into python:main Apr 27, 2026
66 checks passed
@StanFromIreland StanFromIreland deleted the expat-2.8.0 branch April 27, 2026 20:22
@miss-islington-app
Copy link
Copy Markdown

Thanks @StanFromIreland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11, 3.12, 3.13, 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link
Copy Markdown

Sorry, @StanFromIreland, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 005555a3f0ae20ee8154eb4ee172e1e355144c8c 3.14

@miss-islington-app
Copy link
Copy Markdown

Sorry, @StanFromIreland, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 005555a3f0ae20ee8154eb4ee172e1e355144c8c 3.13

@miss-islington-app
Copy link
Copy Markdown

Sorry, @StanFromIreland, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 005555a3f0ae20ee8154eb4ee172e1e355144c8c 3.12

@miss-islington-app
Copy link
Copy Markdown

Sorry, @StanFromIreland, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 005555a3f0ae20ee8154eb4ee172e1e355144c8c 3.11

@miss-islington-app
Copy link
Copy Markdown

Sorry, @StanFromIreland, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 005555a3f0ae20ee8154eb4ee172e1e355144c8c 3.10

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Apr 27, 2026

GH-149073 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Apr 27, 2026
@StanFromIreland StanFromIreland removed needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Apr 27, 2026
@StanFromIreland
Copy link
Copy Markdown
Member Author

3.12/3.13 should hopefully be the same as the 3.14 one, so moving the labels over.

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Apr 27, 2026

GH-149074 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Apr 27, 2026
@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented Apr 27, 2026

GH-149075 is a backport of this pull request to the 3.10 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.10 only security fixes label Apr 27, 2026
hugovk pushed a commit that referenced this pull request Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

5 participants