phoenixlan / phoenixapi-v1

Python-based api server for a self-hosted event hosting system
GNU General Public License v3.0
2 stars 0 forks source link

PNG avatars #51

Closed petterroea closed 2 months ago

petterroea commented 3 months ago
api_1                      | [2024-07-07 09:31:41 +0000] [19] [ERROR] Socket error processing request.
api_1                      | Traceback (most recent call last):
api_1                      |   File "/usr/local/lib/python3.10/site-packages/gunicorn/workers/sync.py", line 135, in handle
api_1                      |     self.handle_request(listener, req, client, addr)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/gunicorn/workers/sync.py", line 193, in handle_request
api_1                      |     util.reraise(*sys.exc_info())
api_1                      |   File "/usr/local/lib/python3.10/site-packages/gunicorn/util.py", line 641, in reraise
api_1                      |     raise value
api_1                      |   File "/usr/local/lib/python3.10/site-packages/gunicorn/workers/sync.py", line 178, in handle_request
api_1                      |     respiter = self.wsgi(environ, resp.start_response)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/integrations/pyramid.py", line 132, in sentry_patched_wsgi_call
api_1                      |     return SentryWsgiMiddleware(sentry_patched_inner_wsgi_call)(
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/integrations/wsgi.py", line 115, in __call__
api_1                      |     reraise(*_capture_exception(hub))
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/_compat.py", line 99, in reraise
api_1                      |     raise value
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/integrations/wsgi.py", line 108, in __call__
api_1                      |     rv = self.app(
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/integrations/pyramid.py", line 130, in sentry_patched_inner_wsgi_call
api_1                      |     reraise(*einfo)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/_compat.py", line 99, in reraise
api_1                      |     raise value
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/integrations/pyramid.py", line 126, in sentry_patched_inner_wsgi_call
api_1                      |     return old_wsgi_call(self, environ, start_response)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/router.py", line 270, in __call__
api_1                      |     response = self.execution_policy(environ, self)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/router.py", line 276, in default_execution_policy
api_1                      |     return router.invoke_request(request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/router.py", line 245, in invoke_request
api_1                      |     response = handle_request(request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid_tm/__init__.py", line 178, in tm_tween
api_1                      |     raise exc from None
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid_tm/__init__.py", line 145, in tm_tween
api_1                      |     response = handler(request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/tweens.py", line 43, in excview_tween
api_1                      |     response = _error_handler(request, exc)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/tweens.py", line 17, in _error_handler
api_1                      |     reraise(*exc_info)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/util.py", line 733, in reraise
api_1                      |     raise value
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/tweens.py", line 41, in excview_tween
api_1                      |     response = handler(request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/router.py", line 143, in handle_request
api_1                      |     response = _call_view(
api_1                      |   File "/usr/local/lib/python3.10/site-packages/sentry_sdk/integrations/pyramid.py", line 91, in sentry_patched_call_view
api_1                      |     return old_call_view(registry, request, *args, **kwargs)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/view.py", line 674, in _call_view
api_1                      |     response = view_callable(context, request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/config/views.py", line 170, in attr_view
api_1                      |     return view(context, request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/config/views.py", line 196, in predicate_wrapper
api_1                      |     return view(context, request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/viewderivers.py", line 319, in secured_view
api_1                      |     return view(context, request)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/pyramid/viewderivers.py", line 427, in rendered_view
api_1                      |     result = view(context, request)
api_1                      |   File "/srv/phoenixRest/phoenixRest/utils/__init__.py", line 33, in inner
api_1                      |     return func(*args, **kwargs)
api_1                      |   File "/srv/phoenixRest/phoenixRest/views/user/instance/__init__.py", line 385, in upload_avatar
api_1                      |     im.resize((avatar_thumb_w,avatar_thumb_h), Image.LANCZOS).save(avatar_thumb_path, "JPEG", quality=80)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/PIL/Image.py", line 2413, in save
api_1                      |     save_handler(self, fp, filename)
api_1                      |   File "/usr/local/lib/python3.10/site-packages/PIL/JpegImagePlugin.py", line 642, in _save
api_1                      |     raise OSError(msg) from e
api_1                      | OSError: cannot write mode P as JPEG
simsine commented 3 months ago

https://github.com/phoenixlan/phoenixapi-v1/blob/master/phoenixRest/views/user/instance/__init__.py#L491C44-L491C48

petterroea commented 3 months ago

@simsine That isn't the cause of the problem. We want to always store avatars as JPG to save disk space. The problem is that imagemagick doesn't like when the source image is PNG.

simsine commented 3 months ago

@simsine That isn't the cause of the problem. We want to always store avatars as JPG to save disk space. The problem is that imagemagick doesn't like when the source image is PNG.

Is it PIL.ImageOps that uses imagemagick?

petterroea commented 3 months ago

@simsine my bad, i was mixing imagemagick and pillow.

I am talking about PIL. Basically the goal is this:

simsine commented 3 months ago

So as the stacktrace implies: OSError: cannot write mode P as JPEG. This means that the image has not been converted to RGB mode. This is supposed to happen on line 500. I was able to reproduce this locally by exporting a png with palettised colors as described here. Uploading this png failed as expected, this is because the parsed image with mode P does not contain the "transparency" key in im.info. Changing the "P" and "transparency" to or fixes this and allows us to upload the png normally.

@petterroea Do you know in which cases im.info contains "transparency"? As either using or or removing the check outright would fix this issue.

petterroea commented 3 months ago

@simsine Good catch!

Many PNGs contain transparency, so i believe we need to support it. Are you able to make PIL convert the image to RGB(with image.convert('RGB') or something?)?

For unit tests, can we ensure we test uploading with:

It seems we currently test RGB and RGBA PNGs: https://github.com/phoenixlan/phoenixapi-v1/tree/master/phoenixRest/tests/assets

petterroea commented 3 months ago

Note that since we store as JPEG, the avatar already automatically strips away the transparency. We just need to support it because users may find themselves not being able to upload a photo due to transparency, but not being good enough at computers to strip it away.