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

[Bug]: Single letter avatars in Nextcloud 24 have become all white icons in Nextcloud 25 on Alpine Linux with imagemagick installed #34755

Closed
6 of 9 tasks
Tracked by #1280
e-caste opened this issue Oct 23, 2022 · 73 comments · Fixed by #35891, #37378 or #38143

Comments

@e-caste
Copy link

e-caste commented Oct 23, 2022

⚠️ This issue respects the following points: ⚠️

  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • This issue is not already reported on Github (I've searched it).
  • Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
  • Nextcloud Server is running on 64bit capable CPU, PHP and OS.
  • I agree to follow Nextcloud's Code of Conduct.

Bug description

Before updating to Nextcloud 25, some of my cloud users had letter avatars (e.g. an A on a colorful background if the name was Abcde). Now all avatars have this white image (512x512):
avatar 512

I have:

  • checked that I have a correct installation of imagemagick (I'm using this Dockerfile)
  • run occ preview:reset-rendered-texts (this command was introduced with this commit, but is there any opposite command to regenerate the avatars?), occ preview:generate-all, occ preview:pre-generate, and occ preview:repair
  • run occ files:scan --all, occ files:scan-app-data, occ files:repair-tree

Steps to reproduce

  1. update Nextcloud 24 to Nextcloud 25 (my journey: [Bug]: Upgrade from 24 to 25 can't connect to Postgres DB #34679)
  2. wait a few hours
  3. text avatars are gone
  4. try steps above

Expected behavior

The text avatars are shown correctly.

Installation method

Community Docker image

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.1

Web server

Nginx

Database engine version

PostgreSQL

Is this bug present after an update or on a fresh install?

Updated to a major version (ex. 22.2.3 to 23.0.1)

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

{
    "system": {
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            ***REMOVED SENSITIVE VALUE***
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "pgsql",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "log_type": "file",
        "logfile": "\/var\/log\/nextcloud\/nextcloud.log",
        "loglevel": "2",
        "mail_smtpmode": "sendmail",
        "remember_login_cookie_lifetime": "7200",
        "log_rotate_size": "10485760",
        "trashbin_retention_obligation": "auto, 180",
        "versions_retention_obligation": "auto, 365",
        "simpleSignUpLink.shown": "false",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "filelocking.enabled": true,
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "password": "***REMOVED SENSITIVE VALUE***",
            "port": ***REMOVED SENSITIVE VALUE***
        },
        "logtimezone": "Europe\/Rome",
        "htaccess.RewriteBase": "\/",
        "enable_previews": true,
        "enabledPreviewProviders": {
            "11": "OC\\Preview\\PNG",
            "12": "OC\\Preview\\JPEG",
            "13": "OC\\Preview\\GIF",
            "14": "OC\\Preview\\BMP",
            "15": "OC\\Preview\\MarkDown",
            "16": "OC\\Preview\\MP3",
            "17": "OC\\Preview\\TXT",
            "18": "OC\\Preview\\Movie"
        },
        "preview_max_x": "2048",
        "preview_max_y": "2048",
        "jpeg_quality": "60",
        "maintenance": false,
        "app_install_overwrite": [
            "drawio",
            "end_to_end_encryption",
            "files_external_dropbox",
            "unsplash",
            "groupfolders",
            "onlyoffice",
            "twofactor_webauthn",
            "breezedark",
            "deck",
            "fulltextsearch"
        ],
        "overwrite.cli.url": "***REMOVED SENSITIVE VALUE***",
        "overwriteprotocol": "https",
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",
                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "default_phone_region": "IT",
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "theme": "",
        "version": "25.0.0.18",
        "mail_sendmailmode": "pipe",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "updater.secret": "***REMOVED SENSITIVE VALUE***",
        "data-fingerprint": "***REMOVED SENSITIVE VALUE***"
    }
}

List of activated Apps

Enabled:
  - activity: 2.17.0
  - admin_audit: 1.15.0
  - breezedark: 24.0.2
  - calendar: 4.0.1
  - circles: 25.0.0
  - cloud_federation_api: 1.8.0
  - comments: 1.15.0
  - contacts: 5.0.1
  - contactsinteraction: 1.6.0
  - dashboard: 7.5.0
  - dav: 1.24.0
  - federatedfilesharing: 1.15.0
  - files: 1.20.1
  - files_external: 1.17.0
  - files_pdfviewer: 2.6.0
  - files_rightclick: 1.4.0
  - files_sharing: 1.17.0
  - files_trashbin: 1.15.0
  - files_versions: 1.18.0
  - forms: 3.0.0
  - fulltextsearch: 24.0.0
  - groupfolders: 13.0.0
  - integration_dropbox: 1.0.5
  - integration_github: 1.0.12
  - integration_google: 1.0.8
  - integration_reddit: 1.0.5
  - logreader: 2.10.0
  - lookup_server_connector: 1.13.0
  - maps: 0.2.1
  - nextcloud_announcements: 1.14.0
  - notes: 4.6.0
  - notifications: 2.13.1
  - oauth2: 1.13.0
  - onlyoffice: 7.6.6
  - password_policy: 1.15.0
  - photos: 2.0.0
  - previewgenerator: 5.1.0
  - privacy: 1.9.0
  - provisioning_api: 1.15.0
  - recommendations: 1.4.0
  - related_resources: 1.0.1
  - serverinfo: 1.15.0
  - settings: 1.7.0
  - spreed: 15.0.0
  - support: 1.8.0
  - systemtags: 1.15.0
  - text: 3.6.0
  - theming: 2.0.0
  - twofactor_backupcodes: 1.14.0
  - twofactor_totp: 7.0.0
  - unsplash: 2.0.1
  - updatenotification: 1.15.0
  - user_status: 1.5.0
  - viewer: 1.9.0
  - weather_status: 1.5.0
  - workflowengine: 2.7.0
Disabled:
  - bruteforcesettings
  - deck: 1.8.0
  - drawio: 1.0.3
  - encryption
  - external: 5.0.0
  - extract: 1.3.5
  - federation: 1.11.0
  - files_external_dropbox: 1.4.3
  - files_fulltextsearch: 24.0.1
  - firstrunwizard: 2.13.0
  - fulltextsearch_elasticsearch: 24.0.1
  - issuetemplate: 0.7.0
  - mail: 2.0.3
  - metadata: 0.16.0
  - news: 18.3.0
  - podcast: 0.3.1
  - sharebymail: 1.12.0
  - survey_client: 1.12.0
  - suspicious_login
  - twofactor_webauthn: 1.0.0
  - user_ldap

Nextcloud Signing status

Technical information
=====================
The following list covers which files have failed the integrity check. Please read
the previous linked documentation to learn more about the errors and how to fix
them.

Results
=======
- related_resources
	- EXCEPTION
		- OC\IntegrityCheck\Exceptions\InvalidSignatureException
		- Signature data not found.

Raw output
==========
Array
(
    [related_resources] => Array
        (
            [EXCEPTION] => Array
                (
                    [class] => OC\IntegrityCheck\Exceptions\InvalidSignatureException
                    [message] => Signature data not found.
                )

        )

)

Nextcloud Logs

No response

Additional info

The white text avatar is downloaded from Nextcloud Talk iOS and Nextcloud Talk web independently. I can also check the avatar picture by going to https://my.cloud.tld/index.php/avatar//96.

@e-caste e-caste added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Oct 23, 2022
@e-caste
Copy link
Author

e-caste commented Oct 24, 2022

Update: it's not the exact same image, each image is slightly different. 4 examples:

avatar 512-2

avatar 512-3

avatar 512-4

avatar 512-5

Looks like the avatar generation process completes, but does not work correctly. Is there any occ command that I can use to re-trigger the avatar generation?

@e-caste
Copy link
Author

e-caste commented Oct 24, 2022

Dark avatars (same URL but adding /dark at the end):

avatar-dark 512-4

avatar-dark 512-3

avatar-dark 512-2

avatar-dark 512

They are slightly different like the white ones, but still wrong since they don't have the foreground letter.

@jared2876
Copy link

I am also having this issue when upgrading to Nextcloud 25.

@phillipplum
Copy link

I have the same issue. Here an example image for darkmode.

avatar-dark 512

@piiskop
Copy link

piiskop commented Oct 25, 2022

All the icons are white and not distinguishable. Maps sometimes displays an error message and if that appears i cannot remove that white modal window. Did you guys miss integration testing before launching 25?

@serverless83
Copy link

Same issue with text avatars since upgrading to 25 RC3, can’t find any solution.

@piiskop
Copy link

piiskop commented Oct 26, 2022

The reason is that the icons are white on a transparent background which has a white background. If i change the background color for header in header.scss, line 31 to another color then i see icons. Please fix it as soon as possible. i am really wondering how has that not been discovered before launching a major version. The one who caused it please speak out loud so that we can get your reasons.

@PVince81 PVince81 added 1. to develop Accepted and waiting to be taken care of regression ui-refresh-feedback and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Oct 28, 2022
@PVince81 PVince81 added this to the Nextcloud 25.0.2 milestone Oct 28, 2022
@Pytal
Copy link
Member

Pytal commented Nov 1, 2022

Can't reproduce

The related change is likely #33752 but the code doesn't look problematic, any idea what could cause this @CarlSchwan?

@Pytal
Copy link
Member

Pytal commented Nov 1, 2022

Commenting out https://github.com/nextcloud/server/blob/v25.0.0/lib/private/Avatar/Avatar.php#L136 seems to produce the same results as in #34755 (comment) so this seems like an issue with fonts

Could you provide your nextcloud.log contents @e-caste?

@phillipplum
Copy link

I have tried a few things and it seems to be a problem with ImageMagick. For me, the problem can be solved quite easily if I don't have the image converted to a PNG. For this I disabled the following line: https://github.com/nextcloud/server/blob/v25.0.0/lib/private/Avatar/Avatar.php#L138

My result:
avatar-dark 512

If you search the internet there are always many different problems with ImageMagick and SVGs.

@szaimen
Copy link
Contributor

szaimen commented Nov 1, 2022

@PVince81 @Pytal what happens on instances that dont have imagick installed? Will it completely bug out?

@phillipplum
Copy link

@szaimen if the function returning null (https://github.com/nextcloud/server/blob/v25.0.0/lib/private/Avatar/Avatar.php#L129), then the function generateAvatar (https://github.com/nextcloud/server/blob/v25.0.0/lib/private/Avatar/Avatar.php#L150) is used instead of generateAvatarFromSvg.

@szaimen
Copy link
Contributor

szaimen commented Nov 2, 2022

Ah right, thanks!

@piiskop
Copy link

piiskop commented Nov 3, 2022

i saw lately that a user on Mac had the grey background and the icons were visible. i use Ubuntu and another user had Windows - both having a white background with no icons displayed.

@serverless83
Copy link

Not resolved in NC 25.0.1

@PVince81 PVince81 assigned skjnldsv and unassigned Pytal Nov 14, 2022
@skjnldsv
Copy link
Member

Can everyone confirmed they have imagick enabled?
WITH svg support?

You can check that with the admin settings page
image

@elandorr
Copy link

elandorr commented Apr 21, 2023

I really don't understand the nextcloud modus operandi. This is not fixed. Unless an admin finds this thread, then reads @schleyk's comment, and then comes up with a way of accomplishing this in a reasonable manner, he will wonder until nextcloud v99.

A proper fix means nextcloud informs the admin of the problem and offers to regenerate all relevant avatars. It's just a small, inconsequential bug, but the way this lingers more than half a year, across a (broken) v26, and still isn't proper, all while adding buzzwords upon buzzwords in marketing, is maddening.

Since that's not going to happen judging from the record-breaking breakage since owncloud days, here's a script for admins who want to get this over with:

#!/usr/bin/env bash

# run from data/appdata_xxx/avatar folder

set -euo pipefail

while read -r folder
do
  if [[ -f "${folder}/generated" ]]; then
    rm -rf "${folder}"
  fi
done <<<"$(find . ! -path .  -type d)"

Thankfully the 'generated' file exists, so you don't have to manually fuck around. This leaves user-uploaded avatars alone.

Even this is not a 'fix'. On the next login, nc will generate only one of the required files. The small one in the upper right won't be regenerated until the user attempts to change his name.

There doesn't seem to be an occ command that would trigger it, so you have to tell the users who care to edit their name for a second.

image

Neither files:scan-app-data nor changing the user's name back and forth via occ regenerate both, before anyone wastes time trying. It has to be inside the user's own account panel.

For reference, the files that need to exist:
avatar.512.png
avatar.64.png
avatar.png

@icedream
Copy link

icedream commented Apr 21, 2023

I think the issue got closed mainly because the merge request mentions "Fix #34755" as a trigger keyword and GitHub automates on that. I agree though, I did mention in my post that I had to clear the avatar cache, otherwise Nextcloud will just continue to use the broken avatar renders until a user change triggers a rerender. I was acting on a from-scratch test setup so I was able to just wipe everything instead of selectively.

I propose that this issue is reopened and the reuse of broken avatars be solved before closing it again. The solution can range anywhere from directly having a check in the code for avatars that are known to be generated with faulty code and automatically rerendering or notifying the admin based on this (some flag in the database similar to oc_migrations, or checking the modified date on avatar files, ...), all the way down to working around this by mentioning the fact you need to empty the avatar cache in a central place such as the changelog just for admins actively on the lookout for a fix (maybe implementing a command to clear the avatar cache for only users without avatar?). I'm not enough into the code to confidently just call a single particular solution here.

@provokateurin provokateurin reopened this Apr 28, 2023
@provokateurin
Copy link
Member

@szaimen This is still a problem, as users reported that the avatars are not regenerated.

@szaimen
Copy link
Contributor

szaimen commented Apr 28, 2023

The problems is technically solved (at least for new users). After updating people need to regenerate the avatars.

@provokateurin
Copy link
Member

Yes, but how can they do that? As far as I understand the server just caches the avatars and the cache would need to cleared to fix this.

@szaimen
Copy link
Contributor

szaimen commented Apr 28, 2023

A migration step could be added that checks if imagick rsvg is present and clean all avatars in appdata once. But I dont feel capable to implement that.

@szaimen
Copy link
Contributor

szaimen commented Apr 28, 2023

After running the migration step, cleaning the browser cache will automatically regenerate the avatars.

@provokateurin
Copy link
Member

Telling all users to just clear their browser cache is not an option. We need some solution for this.

@szaimen
Copy link
Contributor

szaimen commented Apr 28, 2023

This wont work as the migration step is missing as well.

@provokateurin
Copy link
Member

I adapted @elandorr's script because it would delete the whole folder breaking the avatar generation process instead of only deleting the pngs:

#!/bin/bash
set -euxo pipefail
while read -r folder
do
  if [[ -f "${folder}/generated" ]]; then
    rm "${folder}"/*.png
  fi
done <<<"$(find . ! -path . -type d)"

@elandorr
Copy link

@provokateurin For the record: that wasn't necessary here. It creates the folders just fine. Just not when you want it.

@ the previous comments: NC could 'cache-bust' these no problem; write a few lines to regen, add a timestamp to the request. In a later update, this 'one-use-code' could be removed, but who am I kidding, it's 2023, who cares.

If it weren't a multi-million $ business, I'd fix it... But really, it's just avatars. Be happy, if your core works, and don't update too soon lest you challenge the beast.

@provokateurin
Copy link
Member

@szaimen I found https://github.com/nextcloud/server/blob/master/lib/private/Repair/ClearGeneratedAvatarCache.php that already clears the avatar cache given a version. If we bump this version to the currently released version it should regenerated all avatars right? We would have to backport it to 25 and 26 to fix it everywhere, right?

@szaimen
Copy link
Contributor

szaimen commented May 8, 2023

I am not sure if this works. In the end the repair job comes here and runs only one time iirc:

$users = $this->config->getUsersForUserValue('avatar', 'generated', 'true');

@provokateurin
Copy link
Member

To my understanding it selects the users that have a generated avatar and just deletes their whole avatar folder. Afterwards it sets the generated value to false. I assume this will cause the avatar to be regenerated? I mean at that point they have no avatar anymore, so the server should create one for them, right?

@szaimen
Copy link
Contributor

szaimen commented May 8, 2023

I think the clearcache only runs once for each user globally since it sets the value for each user to false at the end of the job, no?
See

$this->config->setUserValue($userId, 'avatar', 'generated', 'false');

@szaimen
Copy link
Contributor

szaimen commented May 8, 2023

But maybe I am wrong?

@provokateurin
Copy link
Member

Judging from

$this->config->setUserValue($this->user->getUID(), 'avatar', 'generated', 'true');
the value is set when no avatar was found and one was automatically generated. If it is set to false again the server will try to check again if an avatar file exists and if that isn't the case it will generate one.

@szaimen
Copy link
Contributor

szaimen commented May 8, 2023

Ah that makes sense then. So it will run whenever the maintenance:repair is executed? In that case, what is missing then? Only clearing the avatars in the browser cache?

@provokateurin
Copy link
Member

Wouldn't the avatars in the browser cache get invalidated once the avatars on the server are regenerated since they will have a different etag? Not sure how this works. I think to fix it we just need to adapt the job to run when upgrading from any of the currently supported versions so that on the next release all generated avatars are cleared.

@szaimen
Copy link
Contributor

szaimen commented May 8, 2023

Based on my testing it looks like there is some version applied. However not sure how and when it gets increased.

The link looks like this: https://mydomain.com/avatar/admin/512/dark?v=0

@elandorr
Copy link

elandorr commented May 8, 2023

You guys could use setDisplayName to look for clues. If a user changes his name in settings, it reliably recreates both/all pics. increaseCacheBuster also sounds related.

@provokateurin
Copy link
Member

provokateurin commented May 9, 2023

The ?v= parameter comes from

$this->config->setUserValue($this->user->getUID(), 'avatar', 'version',
(string)((int)$this->config->getUserValue($this->user->getUID(), 'avatar', 'version', '0') + 1));
, meaning that we just need to bump the avatar version for affected users by one.

@provokateurin provokateurin added 2. developing Work in progress and removed 1. to develop Accepted and waiting to be taken care of labels May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment