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

Unexpected behaviour with ElementTree.ParseError #63

Closed
pmezard opened this issue Mar 8, 2021 · 3 comments · Fixed by #64
Closed

Unexpected behaviour with ElementTree.ParseError #63

pmezard opened this issue Mar 8, 2021 · 3 comments · Fixed by #64

Comments

@pmezard
Copy link

pmezard commented Mar 8, 2021

Hello,

No sure if it is a bug or feature, but this is certainly a regression in 0.7.0. To reproduce:

import defusedxml.ElementTree
import xml.etree.ElementTree

try:
    defusedxml.ElementTree.fromstring("")
    print("OK")
except xml.etree.ElementTree.ParseError as e:
    print("PARSEERROR", e.__class__)
except Exception as e:
    print("NOT PARSEERROR", e.__class__)

which outputs:

NOT PARSEERROR <class 'xml.etree.ElementTree.ParseError'>

Unsurprisingly, it can be bisected to:

commit 3a48453780793c3e98251b0139e9e89e310fb617 (HEAD, refs/bisect/bad)
Author: Christian Heimes <[email protected]>
Date:   Tue Jan 12 16:37:17 2021 +0100

    Restore xml.etree.ElementTree after patch
    
    Restore ``ElementTree`` attribute of ``xml.etree`` module after patching

Is this a bug or should we do this in a different manner? Regardless, this is very confusing.

Thanks.

@tiran
Copy link
Owner

tiran commented Mar 8, 2021

This is an unforeseen side-effect of module reloading code. The exception classes look to be the same, but they are actually different classes.

>>> import defusedxml.ElementTree
>>> import xml.etree.ElementTree
>>> defusedxml.ElementTree.ParseError is xml.etree.ElementTree.ParseError
False
>>> defusedxml.ElementTree.ParseError
<class 'xml.etree.ElementTree.ParseError'>
>>> id(defusedxml.ElementTree.ParseError)
94084011057936
>>> xml.etree.ElementTree.ParseError
<class 'xml.etree.ElementTree.ParseError'>
>>> id(xml.etree.ElementTree.ParseError)
94084011051104

Looking into a fix now...

tiran added a commit that referenced this issue Mar 8, 2021
The ``defusedxml.ElementTree.ParseError`` exception is now the same class object
as ``xml.etree.ElementTree.ParseError`` again. The regression was
introduced by new patching code as part of PR #60.

See: #60
Fixes: #63
Signed-off-by: Christian Heimes <[email protected]>
@tiran tiran closed this as completed in #64 Mar 8, 2021
tiran added a commit that referenced this issue Mar 8, 2021
The ``defusedxml.ElementTree.ParseError`` exception is now the same class object
as ``xml.etree.ElementTree.ParseError`` again. The regression was
introduced by new patching code as part of PR #60.

See: #60
Fixes: #63
Signed-off-by: Christian Heimes <[email protected]>
tiran added a commit that referenced this issue Mar 8, 2021
The ``defusedxml.ElementTree.ParseError`` exception is now the same class object
as ``xml.etree.ElementTree.ParseError`` again. The regression was
introduced by new patching code as part of PR #60.

See: #60
Fixes: #63
Signed-off-by: Christian Heimes <[email protected]>
tiran added a commit that referenced this issue Mar 8, 2021
The ``defusedxml.ElementTree.ParseError`` exception is now the same class object
as ``xml.etree.ElementTree.ParseError`` again. The regression was
introduced by new patching code as part of PR #60.

See: #60
Fixes: #63
Signed-off-by: Christian Heimes <[email protected]>
tiran added a commit that referenced this issue Mar 8, 2021
The ``defusedxml.ElementTree.ParseError`` exception is now the same class object
as ``xml.etree.ElementTree.ParseError`` again. The regression was
introduced by new patching code as part of PR #60.

See: #60
Fixes: #63
Signed-off-by: Christian Heimes <[email protected]>
@tiran
Copy link
Owner

tiran commented Mar 8, 2021

Fixed in release 0.7.1.

Thanks for the report!

@pmezard
Copy link
Author

pmezard commented Mar 8, 2021

Merci !

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

Successfully merging a pull request may close this issue.

2 participants