docker
docker copied to clipboard
nextcloud
Fix warning about svg support missing
This fixes #1414 by adding the appropriate package in all of the supported flavors.
Thanks. This kind of PR has been submitted before and closed with a "wontfix" but then they close the issue topic... not sure why there's little communication here about what's happening but i hope they can agree to either accept this or just remove the dang warning.
Hm, it sounded like @pierreozoux would be a part of it and finally allow the change... This has been really annoying for quite a long time. The team should totally decide if they see it as a security risk or not...
@CorneliousJD Did you follow this discussion? https://github.com/nextcloud/docker/issues/1414
I think the community of people using this docker image is in favor of this change.
I also tried to change the warning message, and this was the response: https://github.com/nextcloud/server/pull/31742#discussion_r837407416 So basically it looks like imagemagick is only used to generate initials icon. So it looks to me that it is not used for user images, so the atack surface is low.
I'm in favor of this change, from what I know and understand.
But people keep coming here, on this issue, so, we as a community have to fix it. (Either change this warning, or add it to our package)
Call me stupid - but why even bother generating previews for SVG containing a circle and one or two letters? The SVG should already be very small. Is that worth making this controversial change? 'Security' is one of the core values of Nextcloud and this is not only a compromise - it's kompromat.
We usually accept longer load times, higher CPU load, and higher use of bandwidth when it comes to security. Is there anyone not using TLS here? I'd much rather see the Nextcloud server remove the warning and maybe even all references to Imagemagick altogether.
why even bother generating previews for SVG containing a circle and one or two letters?
@yogo1212 From https://github.com/nextcloud/server/pull/31742#discussion_r854874347 it seems that it's not just for the avatars (initials-in-circles), but also "themed favicons and app icons when the base logo is an svg". I'd agree with you for the 'avatars', but if this improves things for themed favicons & app icons, I reckon it makes sense to merge this still.
In the image 25.0.1-fpm:latest, built one day ago, the warning is still there. As far as I have understood, it should be gone after merging this now. Am I wrong here?
Would be really nice. Or at least it should be realized through an option which can be enabled by a container variable. I mean, how many issues should be opened?
In the image 25.0.1-fpm:latest, built one day ago, the warning is still there. As far as I have understood, it should be gone after merging this now. Am I wrong here?
I am also puzzled.
I am also still seeing the warning. Im not sure why if this was actually merged and associated issues were closed as resolved?
Same here, whats wrong here? Im using nextcloud:latest, so even same on the apache version xD
With the most recent image from 3 or 4 hours ago the same again. The weird thing is, if I build the image myself, using the Dockerfile from this repo, the warning is not shown anymore.
I don't know really how such things work, and apologies if I'm using the wrong terminology. But did this change 'trickle down' to the docker-hosted image yet? And did anyone notice a difference between updating an existing installation/container vs creating a new container from scratch?
And did anyone notice a difference between updating an existing installation/container vs creating a new container from scratch?
This is the same in the container world.
This is the same in the container world
Configuration & data is persistent, no? But I guess from your reply that that would not affects this warning.
Configuration & data is persistent
Each container uses specific paths for user config and data files. They are located "outside" of a container. Every update means a full fresh install while reusing those user files. For Nextcloud this path is /var/www/html as mentioned in the documentation in the paragraph "Persistent data": https://hub.docker.com/_/nextcloud
I've stumbled upon something, which might be the reason, that the most recent image at Docker Hub is still warning for the missing libimagick. If you look at the results of the last run of GitHub Actions of the nextcloud/docker repository: https://github.com/nextcloud/docker/actions/runs/3579183960/jobs/6020125259#step:6:13 There you can see, that libmagickcore-6.q16-6-extra is installed via apt:
RUN set -ex; apt-get update; apt-get install -y --no-install-recommends rsync bzip2 busybox-static libldap-common libmagickcore-6.q16-6-extra ; rm -rf /var/lib/apt/lists/*; mkdir -p /var/spool/cron/crontabs; echo '*/5 * * * * php -f /var/www/html/cron.php' > /var/spool/cron/crontabs/www-data```
On the other hand, if you look at the content of the respective image 25.0.1-apache at the Docker Hub: https://hub.docker.com/layers/library/nextcloud/25.0.1-apache/images/sha256-7d38401f5a893f17fd1c1bf38bc81519f243cf914af6177f9b85e93a7837415d?context=explore There you can see in Layer 31, that libmagickcore-6.q16-6-extra is not installed via apt:
/bin/sh -c set -ex; apt-get update; apt-get install -y --no-install-recommends rsync bzip2 busybox-static libldap-common ; rm -rf /var/lib/apt/lists/*; mkdir -p /var/spool/cron/crontabs; echo '*/5 * * * * php -f /var/www/html/cron.php' > /var/spool/cron/crontabs/www-data
I don't know, if this difference is intended or not. Maybe someone, who knows the infrastructure and how GitHub and Docker Hub play together here, can check if this behaviour is intended and if not fix it.
I've stumbled upon something, which might be the reason, that the most recent image at Docker Hub is still warning for the missing libimagick. If you look at the results of the last run of GitHub Actions of the nextcloud/docker repository: https://github.com/nextcloud/docker/actions/runs/3579183960/jobs/6020125259#step:6:13 There you can see, that libmagickcore-6.q16-6-extra is installed via apt:
RUN set -ex; apt-get update; apt-get install -y --no-install-recommends rsync bzip2 busybox-static libldap-common libmagickcore-6.q16-6-extra ; rm -rf /var/lib/apt/lists/*; mkdir -p /var/spool/cron/crontabs; echo '*/5 * * * * php -f /var/www/html/cron.php' > /var/spool/cron/crontabs/www-data```On the other hand, if you look at the content of the respective image 25.0.1-apache at the Docker Hub: https://hub.docker.com/layers/library/nextcloud/25.0.1-apache/images/sha256-7d38401f5a893f17fd1c1bf38bc81519f243cf914af6177f9b85e93a7837415d?context=explore There you can see in Layer 31, that libmagickcore-6.q16-6-extra is not installed via apt:
/bin/sh -c set -ex; apt-get update; apt-get install -y --no-install-recommends rsync bzip2 busybox-static libldap-common ; rm -rf /var/lib/apt/lists/*; mkdir -p /var/spool/cron/crontabs; echo '*/5 * * * * php -f /var/www/html/cron.php' > /var/spool/cron/crontabs/www-dataI don't know, if this difference is intended or not. Maybe someone, who knows the infrastructure and how GitHub and Docker Hub play together here, can check if this behaviour is intended and if not fix it.
Well this certainly seems to be the answer as to why our nextcloud:latest containers don't have it - it's not actually present in the image. Someone will need to address/correct this build it would seem.
The actual Dockerfile: https://github.com/nextcloud/docker/blob/master/25/apache/Dockerfile
And what the dockerhub uses: https://github.com/nextcloud/docker/blob/739d6996409825bc61000f0c901ea05fbe3debf7/25/apache/Dockerfile
The dockerhub needs an update it seems, cause its fixed somewhat to this tag. however, i checked the differences, because i was worried that more "pull requests" are missing...
root@Docker:~# diff Dockerlatest Dockerhub
13d12
< libmagickcore-6.q16-6-extra \
root@Docker:~#
But no, only libmagickcore is missing :-)
The build from today is again without ImageMagick.
The builds are broken, docker hub just builds always the same image 😂 Idk who maintains this dockerhub/docker image thing. That one should be tagged here.
This here isn't modzillas fault, he can't anything for. But whoever maintains this container need to update inside dockerhub the link to the docker file.
That's nothing anyone of us can do with a PR or sth like that. Cheers
The build from today is again without ImageMagick.
The builds are broken, docker hub just builds always the same image 😂 Idk who maintains this dockerhub/docker image thing. That one should be tagged here.
This here isn't modzillas fault, he can't anything for. But whoever maintains this container need to update inside dockerhub the link to the docker file.
That's nothing anyone of us can do with a PR or sth like that. Cheers
Absolutely right! I did not want to blame anyone, but only to draw attention to it again, because unfortunately I also do not know whom to address.
@J0WI might be able to help here.
This user (could also be a bot) is the one pushing changes to the official docker-library/official-images . Also his username somewhat resembles the one generic user shown on Dockerhub (?) (just a wild guess):
Maybe @tianon could assist here too / tell us who's in charge of the Dockerfile (sorry for tagging you if you are the wrong person!). He was once involved with a different Nextcloud Docker issue: https://github.com/docker-library/official-images/issues/13135
doijanky is the CI user that is used in the infosiftr pipelines which push the official docker libraries.
The docker hub image gets updated when a PR like https://github.com/docker-library/official-images/pull/13475 is created and merged.
The just built image for 25.0.2 on Docker Hub now contains ImageMagick! Perfect, thanks to the maintainers.
The just built image for 25.0.2 on Docker Hub now contains ImageMagick! Perfect, thanks to the maintainers.
I have not seen this in a very long time. Thanks everyone involved in fixing this, especially @dkadioglu :heart:
