Skip to content

bpo-29591: Upgrade Modules/expat to libexpat 2.2 #2164

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

Merged
merged 4 commits into from
Jun 14, 2017
Merged

bpo-29591: Upgrade Modules/expat to libexpat 2.2 #2164

merged 4 commits into from
Jun 14, 2017

Conversation

vstinner
Copy link
Member

[Security] bpo-29591: Update expat copy from 2.1.1 to 2.2.0 to get fixes of CVE-2016-0718 and CVE-2016-4472. See https://sourceforge.net/p/expat/bugs/537/ for more information.

@@ -342,7 +342,7 @@ XML_SetEntityDeclHandler(XML_Parser parser,
XML_EntityDeclHandler handler);

/* OBSOLETE -- OBSOLETE -- OBSOLETE
This handler has been superseded by the EntityDeclHandler above.
This handler has been superceded by the EntityDeclHandler above.
Copy link
Member Author

Choose a reason for hiding this comment

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

wait, superseded is the right spelling no?

Copy link
Member

Choose a reason for hiding this comment

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

it is... but lets not correct spelling in third party code.

Copy link
Member

Choose a reason for hiding this comment

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

oh, i see, this was a python tree only change? weird. keep it as superseded.

Copy link
Member Author

Choose a reason for hiding this comment

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

it is... but lets not correct spelling in third party code.

Right, I prefer to reduce the downstream changes: so I reverted this change as well.

@@ -7,6 +7,9 @@
/* Define to 1 if you have the `bcopy' function. */
#define HAVE_BCOPY 1

/* Define to 1 if you have the <check.h> header file. */
#undef HAVE_CHECK_H
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand this change, I just tried to reduce the changes.

Copy link
Member

Choose a reason for hiding this comment

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

we don't even need to carry this file around given the #elif defined(__amigaos__) it is included under.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure that I understood. Do you want to remove amigaconfig.h?

Copy link
Member Author

Choose a reason for hiding this comment

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

In the meanwhile, I reverted this change.

@@ -7,6 +7,10 @@

/* External API definitions */

/* Namespace external symbols to allow multiple libexpat version to
co-exist. */
#include "pyexpatns.h"
Copy link
Member Author

Choose a reason for hiding this comment

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

I would be nice to upstream pyexpatns.h namespace, it seems useful to build a clean DLL on Windows.

Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

I believe we can drop amigaconfig.h, watcomconfig.h, and macconfig.h (used for MacOS classic on PPC32).

@matrixise
Copy link
Member

Duplicate of #2021 ?

@vstinner
Copy link
Member Author

@matrixise: "Duplicate of #2021 ?"

I chose a different approach. I wanted to recreate Modules/expat/ directory "from scratch", so I wrote a shell script that I attached http://bugs.python.org/issue29591

I tried to restore Python changes reverted by my approach, to more easily identify downstream (Python) changes.

I compare my PR with your (rebased) PR:

  • you didn't update COPYING
  • you removed: #include "pyexpatns.h"
  • you removed: #define XML_HAS_SET_HASH_SALT
  • my PR now also removes amigaconfig.h, macconfig.h and watcomconfig.h of Modules/expat/

@gpshead
Copy link
Member

gpshead commented Jun 13, 2017

FWIW I agree with recreating the directory from scratch and applying the few actually relevant local changes on top as you have done.

@gpshead
Copy link
Member

gpshead commented Jun 13, 2017

We can remove the XML_HAS_SET_HASH_SALT define and remove that from the ifdef in Modules/pyexpat.c as that was a custom Python tree only addition I did back when I backported those changes to the old version of expat we had at the time.

vstinner added 4 commits June 14, 2017 11:58
Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom
The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).
@vstinner
Copy link
Member Author

@gpshead: "We can remove the XML_HAS_SET_HASH_SALT define and remove that from the ifdef in Modules/pyexpat.c (...)"

Right, done.

@vstinner vstinner dismissed gpshead’s stale review June 14, 2017 10:05

" I believe we can drop amigaconfig.h, watcomconfig.h, and macconfig.h (used for MacOS classic on PPC32)." => done

@vstinner
Copy link
Member Author

Does it look good to you now @gpshead?

Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

Your patch on top of expat upstream looks good to me. I don't have enough resources to properly verify all changes in expat, though.

@vstinner vstinner merged commit 23ec4b5 into python:master Jun 14, 2017
@vstinner vstinner deleted the expat_22 branch June 14, 2017 22:54
@vstinner
Copy link
Member Author

Thanks for the reviews @tiran and @gpshead!

vstinner added a commit that referenced this pull request Jun 14, 2017
* bpo-29591: Upgrade Modules/expat to libexpat 2.2

* bpo-29591: Restore Python changes on expat

* bpo-29591: Remove expat config of unsupported platforms

Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom

* bpo-29591: Remove useless XML_HAS_SET_HASH_SALT

The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).

(cherry picked from commit 23ec4b5)
vstinner added a commit that referenced this pull request Jun 15, 2017
* bpo-29591: Upgrade Modules/expat to libexpat 2.2

* bpo-29591: Restore Python changes on expat

* bpo-29591: Remove expat config of unsupported platforms

Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom

* bpo-29591: Remove useless XML_HAS_SET_HASH_SALT

The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).

(cherry picked from commit 23ec4b5)
vstinner added a commit that referenced this pull request Jun 15, 2017
* bpo-29591: Upgrade Modules/expat to libexpat 2.2

* bpo-29591: Restore Python changes on expat

* bpo-29591: Remove expat config of unsupported platforms

Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom

* bpo-29591: Remove useless XML_HAS_SET_HASH_SALT

The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).

(cherry picked from commit 23ec4b5)
larryhastings pushed a commit that referenced this pull request Jul 12, 2017
…2164) (#2203)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2 (#2164)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2

* bpo-29591: Restore Python changes on expat

* bpo-29591: Remove expat config of unsupported platforms

Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom

* bpo-29591: Remove useless XML_HAS_SET_HASH_SALT

The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).

(cherry picked from commit 23ec4b5)

* bpo-30694: Upgrade Modules/expat/ to libexpat 2.2.1 (#2300)

New file: Modules/expat/siphash.h.
(cherry picked from commit 5ff7132)

* bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)

bpo-30726, bpo-29591: libexpat 2.2.1 of Modules/expat/ now uses
a winconfig.h configuration file which already defines:

* XML_NS
* XML_DTD
* BYTEORDER=1234
* XML_CONTEXT_BYTES=1024
* HAVE_MEMMOVE

Remove these defines from PCbuild/_elementtree.vcxproj to prevent
compiler warnings.

Co-Authored-By: Jeremy Kloth <[email protected]>
(cherry picked from commit c8fb58b)

* bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (#2319)

* bpo-30726: Fix elementtree warnings on Windows

Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.

* bpo-30726: Don't ignore libexpat warnings which haypo intends to fix upstream

(cherry picked from commit 87c6555)
ned-deily pushed a commit that referenced this pull request Jul 16, 2017
…2164) (#2204)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2 (#2164)

* bpo-29591: Upgrade Modules/expat to libexpat 2.2

* bpo-29591: Restore Python changes on expat

* bpo-29591: Remove expat config of unsupported platforms

Remove the configuration (Modules/expat/*config.h) of unsupported
platforms:

* Amiga
* MacOS Classic on PPC32
* Open Watcom

* bpo-29591: Remove useless XML_HAS_SET_HASH_SALT

The XML_HAS_SET_HASH_SALT define of Modules/expat/expat.h became
useless since our local expat copy was upgrade to expat 2.1 (it's now
expat 2.2.0).

(cherry picked from commit 23ec4b5)

* bpo-30694: Upgrade Modules/expat/ to libexpat 2.2.1 (#2300)

New file: Modules/expat/siphash.h.
(cherry picked from commit 5ff7132)

* bpo-30726: PCbuild _elementtree: remove duplicate defines (#2348)

bpo-30726, bpo-29591: libexpat 2.2.1 of Modules/expat/ now uses
a winconfig.h configuration file which already defines:

* XML_NS
* XML_DTD
* BYTEORDER=1234
* XML_CONTEXT_BYTES=1024
* HAVE_MEMMOVE

Remove these defines from PCbuild/_elementtree.vcxproj to prevent
compiler warnings.

Co-Authored-By: Jeremy Kloth <[email protected]>
(cherry picked from commit c8fb58b)

* bpo-30726: Fix elementtree warnings on Windows due to expat upgrade (#2319)

* bpo-30726: Fix elementtree warnings on Windows

Caused by usage of `getenv` which should be safe. And a few integer
truncations which should also be ok.

* bpo-30726: Don't ignore libexpat warnings which haypo intends to fix upstream

(cherry picked from commit 87c6555)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-security A security issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants