Invalid file error while uploading different file format in personal Information admin page.

[aryan7081]

Description.

When attempting to upload an SVG (Scalable Vector Graphics) icon as a portrait in the personal information section via the admin page and clicking on the edit button, an error is encountered instead of the expected behavior of displaying a message indicating that the file format is not supported.

Steps to reproduce the behavior:

1. Navigate to the admin page.
2. Click on the "Edit" button in the personal information section.
3. In the form that appears, locate the portrait upload field.
4. Attempt to upload an SVG icon file.
5. Observe the error message instead of the expected notification about unsupported file format.

Expected Behavior:
When attempting to upload an invalid file format as a portrait, the system should display a message indicating that the file format is not supported, rather than throwing an error.

Screenshots

Software (please complete the following information):

• OS: Mac OS M2
• Browser: Chrome
• Volto Version: 18.0.0-alpha.10
• Plone Version 5.2.2
• Plone REST API Version e.g. 7.0.1

Additional context

• Providing proper feedback about unsupported file formats can enhance user experience and prevent confusion.
• Possible solutions may include implementing file format validation or updating error handling to provide clearer messaging to users.

[stevepiercy]

@uwaiszaki @aryan7081 has not yet signed the Plone Contributor Agreement, and until that happens, should not be assigned any issue. Assignment removed.

[stevepiercy]

The screenshot indicates Classic UI, not Volto. Transferring issue to the correct repo.

[rber474]

Confirmed in Plone 6.0.7:

Plone 6.0.7 (6018)
CMF 3.2
Zope 5.8.5
Python 3.11.2 (main, Oct 3 2023, 08:50:32) [Clang 12.0.5 (clang-1205.0.22.9)]
PIL 9.5.0 (Pillow)
WSGI: Activar
Servidor: waitress 2.1.2

The problem seems to be the lack of Pillow support for SVG files. When portrait is being set, scale_image method is called where the PIL tries to open it raising the error.
This is what is commented there:

    # Load up the image, don't try to catch errors, we want to fail miserably
    # on invalid images
    image = Image.open(image_file)

Possible solutions to this issue, at a first glance, could be:

1. Include a try/except in the form, catching IOError or PIL.UnidentifiedImageError which will allow to catch it and give a validation error.
2. Include a try/except in the membership.pytool, changeMemberPortraitmethod and not to scale de image in case of exception.
3. Include a portrait validation in the form, checking the blob contentType of file and don't allow image/svg+xml

I would choose the first option, as it'd scale the image if SVG support is included by Pillow.
What do yo think, @stevepiercy ?

Regards

Edit:
Tested 2nd solution. It works but it is not fancy to have an unscaled image...

[erral]

I think that the proper thing is to issue an error.

[stevepiercy]

Option 4

I think Plone should accept an SVG for profile images, as some people use SVG icons, not JPG photos.

I think that Plone should not attempt to resize an SVG through Pillow because its name itself implies it is a Scalable Vector Graphic and should fit within the space of its HTML container, defined by its CSS. One purpose for resizing is to reduce file size when serving, and that would not happen with SVG (except in the absurd, rare case where a bitmap image was put inside an SVG). It should merely accept it as is, and allow the editor user to specify its size attributes for display.

[rber474]

I also believe that Plone should support SVG for profile portraits. However, I disagree with letting the user choose the size of the portrait displayed in the author info. This could lead to unexpected layout issues for each user, necessitating overrides of styles or themes. The scale size and format for this feature are currently defined as static settings in Products.PlonePAS, with dimensions set to 75x100, anticipating a resizable image through PIL.

In addressing this issue, filtering allowed images should suffice to resolve it. In my opinion, a PLIP (Plone Improvement Proposal) or Enhancement should be initiated to refactor all portrait functionality.

Here are my personal thoughts on this:

• Introduce configuration options in the user option control panel or site control panel, enabling administrators to adjust image dimensions.
• Correspondingly, modify Products.PlonePAS/utils.py to fetch these settings and scale the image accordingly.
• Refactor Products.PlonePAS membership tool.py/changeMemberPortrait to verify whether the uploaded file is an image/svg+xml, thus avoiding scaling.
• Modify Products.CMFPlone @@author view to adapt container attributes based on the configured settings, ensuring consistent aspect ratios across all image types.
• Display the portrait in the @@personal-information view.
• Add a small portrait to the edit-bar toolbar on the left side, next to the username.

Advanced changes:

• Allow users to adjust the image (position, zoom) after it's uploaded.

What do you think?

Edit: Forgot to say I could work in these changes.

[stevepiercy]

@rber474 that, sir, is a PLIP worthy proposal.

The only question is in which repo should it be made? I think CMFPlone is the default, but it could touch many others as well.

[rber474]

@stevepiercy Good question!

• Products.PlonePAS:

  • memberdata_tool.py
  • utils.py with the scale_image function
  • config.py with the static settings

• Products.CMFPlone:

  • MemberGroup settings controlpanel
  • Author view

• plone.base:

  • Schema interface for MemberGroup settings

• plone.app.users:

  • personal-information view

• Barceloneta.Theme (??)

• mockup and plone.staticresources if any JS functionality is added (i.e: zoom or position change)

Yeah, a few repositories. 😄

Products.PlonePAS and ProductsCMFPlone has the most changes to do... For being my first PLIP not bad.
As commented here by @mauritsvanrees Products.CMFPlone could include plone.app.users, so I also will go for that repo.

I don't know if there is a way of reference all repositories being affected by a PLIP, if you agree we can create it.

[stevepiercy]

I would recommend only one primary repo for a PLIP, and cross-referencing other repos from the primary. Choose the repo in which the greatest amount of work or collaboration is needed. I did that for my event content type PLIP.