Skip to content

gh-149017: Upgrade bundled Expat to 2.8.0#149020

Open
StanFromIreland wants to merge 5 commits intopython:mainfrom
StanFromIreland:expat-2.8.0
Open

gh-149017: Upgrade bundled Expat to 2.8.0#149020
StanFromIreland wants to merge 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants