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

Improved flexibility of XMP parsing #7274

Merged
merged 4 commits into from
Oct 6, 2023
Merged

Conversation

radarhere
Copy link
Member

@radarhere radarhere commented Jul 10, 2023

Resolves #7273
Resolves #7324

The JPEG XMP code was originally added in #5144. The image in #7273 differs in that it is padded at the end with b"\x00". I've updated the JpegImagePlugin code to allow for that.

The image in #7273 also has an attribute name that doesn't contain a semicolon. Normally, the tags do, and leads to the start of the tag being surrounded with curly brackets. Their absence causes an error at the moment, so I've also added a commit to allow for that.

I created the test files by copying the XMP data from Tests/images/xmp_test.jpg, modifying it to trigger these conditions, and saving new images with a modified Pillow.

def test_getxmp_no_prefix(self):
with Image.open("Tests/images/xmp_no_prefix.jpg") as im:
if ElementTree is None:
with pytest.warns(UserWarning):
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a UserWarning?

Copy link
Member Author

Choose a reason for hiding this comment

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

ElementTree is None if defusedxml is unavailable, triggering this warning.

Pillow/src/PIL/Image.py

Lines 1410 to 1411 in 1e7dcd3

if ElementTree is None:
warnings.warn("XMP data cannot be read without defusedxml dependency")

Copy link
Member

Choose a reason for hiding this comment

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

OK, let's also add a match= to pytest.warns to make it a bit clearer (this and the other one).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, done, and I've added it to other locations as well.

@hugovk hugovk merged commit 8280fba into python-pillow:main Oct 6, 2023
50 checks passed
@radarhere radarhere deleted the jpeg_xmp branch October 6, 2023 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants