-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-46400: Update libexpat from 2.4.1 to 2.4.4 #31022
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.
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
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
Please sign up the CLA first |
sent it already , waiting for its validation :) |
@jouve I will take a look soon :) |
How did you create this PR? |
I imported code from https://github.com/libexpat/libexpat at R_2_4_4 tag I re-introduced the 3 lines here https://github.com/python/cpython/blob/main/Modules/expat/expat_external.h#L70:
the diffs about In both case by @corona10, and according libexpat/libexpat#513 for the same reason. I kept the one from libexpat. The rest of the changes are relevant to the diff/changelog between R_2_4_1 and R_2_4_4. I also arrived to the same result exporting the relevant patches in libexpat and re-applying them in cpython. |
@hartwork Can you please take a look? |
If you want/it's easier to review, I can regenerate this PR using the 2nd method I described (export the patches & re-apply), it will generate about 15 commits instead of one. |
@corona10 I just have, and it looks sane to me, but don't have CPython glasses. What I would suggest a person with CPython glasses does, is this: ## 1. Understand CPython's deviation from libexpat 2.4.1 upstream
# git clone --depth 1 https://github.com/python/cpython # i.e. main
# git clone --depth 1 --branch R_2_4_1 https://github.com/libexpat/libexpat libexpat_2_4_1
# diff --color=always -r -u libexpat_2_4_1/expat/lib/ cpython/Modules/expat/ | less
## 2. Understand jouve CPython's deviation from libexpat 2.4.4 upstream
# git clone --depth 1 --branch expat_R_2_4_4 https://github.com/jouve/cpython jouve_cpython
# git clone --depth 1 --branch R_2_4_4 https://github.com/libexpat/libexpat libexpat_2_4_4
# diff --color=always -r -u libexpat_2_4_4/expat/lib/ jouve_cpython/Modules/expat/ | less The diffs are small and the diff between the diffs is also small. Again, looks good to me, but someone with CPython glasses should check for themselves as well please.
@jouve my vote for sticking to status quo, because the approach above likely works for others too and I'm not sure all these commits (with different paths and different SHA1s) add value to the Git history of CPython. Just my 2 cents. |
Misc/NEWS.d/next/Library/2022-01-30-15-16-12.bpo-46400.vweUiO.rst
Outdated
Show resolved
Hide resolved
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.
lgtm with minor comment
@hartwork Thanks, I double-check also, it looks good to me too :) |
@@ -0,0 +1 @@ | |||
expat: Update libexpat from 2.4.1 to 2.4.4 to fix security issue (CVE-2021-45960) |
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 note that CVE-2021-45960 is only 1 of 10 CVEs addressed since 2.4.1, see https://github.com/libexpat/libexpat/blob/master/expat/Changes . I would either go all in and mention all 10 or not mention CVEs at all, just my 2 cents.
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.
Thanks!
Misc/NEWS.d/next/Library/2022-01-30-15-16-12.bpo-46400.vweUiO.rst
Outdated
Show resolved
Hide resolved
Sorry @jouve and @corona10, I had trouble checking out the |
GH-31295 is a backport of this pull request to the 3.9 branch. |
(cherry picked from commit 8aaaf7e) Co-authored-by: Cyril Jouve <[email protected]>
Sorry, @jouve and @corona10, I could not cleanly backport this to |
Sorry @jouve and @corona10, I had trouble checking out the |
GH-31296 is a backport of this pull request to the 3.10 branch. |
GH-31297 is a backport of this pull request to the 3.8 branch. |
GH-31298 is a backport of this pull request to the 3.7 branch. |
Co-authored-by: Cyril Jouve <[email protected]>
Co-authored-by: Cyril Jouve <[email protected]>
Co-authored-by: Cyril Jouve <[email protected]>
includes changes from 2.4.1 to 2.4.4 https://github.com/libexpat/libexpat/blob/R_2_4_4/expat/Changes
This will fix the CI issues like https://buildbot.python.org/all/#/builders/271/builds/1233
https://bugs.python.org/issue46400