Skip to content
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

No longer using defusedxml since it is not necessary. #1179

Merged
merged 2 commits into from Apr 11, 2021
Merged

No longer using defusedxml since it is not necessary. #1179

merged 2 commits into from Apr 11, 2021

Conversation

klaasjanelzinga
Copy link
Contributor

@klaasjanelzinga klaasjanelzinga commented Jan 6, 2021

Closes #956
Closes #1014

It is not necessary to use defused when using the lxml parser. The package defusedxml.lxml is deprecated because of this.

The lxml parser uses libxml2 that has the guards against the vulnerabilities build in (at least libxml2 > 2.6). Since zeep already uses the lxml parser, this pull request removes defusedxml as a dependency. Some functionality regarding forbid_dtd and forbid_entities that were present in defusedxml is now present in loader.py and exceptions were added to zeep with the same name as in defusedxml.

  • dependency on defusedxml removed
  • added two new exception classes DTDForbidden en EntitiesForbidden
  • when loading a document the docinfo is checked for entities and doctype.

- mimicked behaviour from defused into the loader.py
- bumped lxml version to > 4.6.0
- mimicked behaviour from defused into the loader.py
- bumped lxml version to > 4.6.0
@klaasjanelzinga
Copy link
Contributor Author

Zie blurb on https://github.com/tiran/defusedxml/blob/master/defusedxml/lxml.py for an explanation:

"""DEPRECATED Example code for lxml.etree protection
The code has NO protection against decompression bombs.
"""

@Yanonix
Copy link

Yanonix commented Mar 24, 2021

Have you an estimate date for a new release with this pull request ?
Thanks

@klaasjanelzinga
Copy link
Contributor Author

I do not know, I haven't had any feedback on this PR from the owner. The repository seems very quiet.

@mvantellingen mvantellingen merged commit 9ff4b5a into mvantellingen:master Apr 11, 2021
@mvantellingen
Copy link
Owner

Looks good, thanks! This was on my todo list for a while

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants