SVG imagemagick Nextcloud 21.0 Docker #1414
Comments
|
Agreed. Also, there's a warning regarding the default phone region code. |
|
Yes, of course. But I thought it's a good idea to provide an ENV VAR for docker users. I really don't want to modify php files while using docker. |
Your config file is in your volume/mount. |
Surely I know that. But what I expect to see is that I provide a set of Dockerfile, docker-compose.yml, and .env, I copy them to the server, run docker-compose up --build -d, and that's it. |
|
I feel setting a default phone region in the image isn't a one size fits all solution and might even confuse users. An env variable or even a graphical setting in the nextcloud UI might be a good solution for that. |
|
I also have solved all warnings but the one related to php-imagick with the container for version 21. |
Were you able to resolve this warning? I am using ngnix and not apache. |
Yes, I'm also using nginx (with the nextcloud-fpm container) and you need to recreate your configuration according to new new configuration as the redirects changed. |
I didn't find this file to edit, did it exist or did you have to create it? |
In .\web\nginx.conf is your configuration. But it seems to have some modifications applied to it, so you have to create a new one by combining what you need from your current config and what's new in the nextcloud docs. |
ok, thanks, i'll take a look. |
|
Hi, same problem here, should we just leave showing up this svg warning? I think exactly the same as @ThxAndBye |
|
Same here |
|
Same for me, it really bugs my mind to see that warning that I can't easily fix. |
|
This should be corrected in the image of the docker, it does not make sense to install something that is made to work in a way and create workarounds to correct a problem that was not solved in the cause. |
|
Here is the part of the admin manual that mentioned the dependency: https://docs.nextcloud.com/server/21/admin_manual/configuration_server/theming.html?highlight=libmagickcore%20q16%20extra#theming-of-icons
But the official docker image based on Debian 10, so it's |
|
And for those who use docker with the nextcloud:fpm-alpine image? Create Dockerfile The problem is which package to install. |
Try it and see :) I guess |
I get that you can install the packages required, I mention it in the original post. |
|
Just make a pull request ;) |
|
Ha, never thought that I would have to go into a container and install a package manually to solve a dependency for a software that is supposed to have it all included in the container image. But running |
|
@ostefek99, that sadly does not work for my Debian 10 container. Did you execute any other commands as well? |
|
@mat-l did you https://packages.debian.org/buster/libmagickcore-6.q16-6-extra this is the package you want. |
|
Consider that installing dependencies in a running container is not a persistent fix. |
|
@mat-l, as Snuupy sugested, you need to run However as markuman said, it's a workaround for a current container. It won't persist. I am not willing to build my own container. I would expect to be fixed in the image. |
|
@Snuupy @ostefek99 woops, forget the update, thought I had executed it before 🙈 @markuman in my optinion it must be integrated into the upstream image. Dunno why Nextcloud didn't do this yet. |
diff --git a/21.0/apache/Dockerfile b/21.0/apache/Dockerfile
index 52fab6e..fc548ac 100644
--- a/21.0/apache/Dockerfile
+++ b/21.0/apache/Dockerfile
@@ -9,6 +9,7 @@ RUN set -ex; \
rsync \
bzip2 \
busybox-static \
+ libmagickcore-6.q16-6-extra \
; \
rm -rf /var/lib/apt/lists/*; \
\that's the fix for the apache container |
Testing this fix on my machine, I'll edit this comment with the results. EDIT: yes it fixes it (bogs my mind a bit that it's in the "entrypoint.sh and cron.sh dependencies" part |
|
Also @markuman I would suggest you not to edit in the |
|
Yes there is a potential security issue, I assume not fixed upstream yet though or if it can be. The message in the UI says I would recommend adding Edit: For any wondering, I have not enabled SVG support, because of the security issue. |
|
Please read past just the last 5 comments or so. This has been discussed at length. Imagemagick should not be added to this image, and the warning should be changed or flipped so you get a warning for using imagemagick. Open a PR to nextcloud if you want to move along the discussion. Re-confirming that you also have this problem just adds to the noise that makes people ignore prior discussion, and pings a lot of people's inboxes. Yes, we all have this problem, and anyone installing this image will. There are a number of hacks in this thread for "fixing" it already, too, likely including yours. None of them should probably be used, unless you really know you want to ignore upstream advice. Here's the relevant bits of info: Should this issue be locked to prevent more sharing of footguns? |
|
I think the root cause is that it takes more than a year to change the warning (or remove it). Also, keeping this issue open makes people think the issue is actually in the Docker image, not with the warning... |
|
As a workaround one could add a "patch" to the own Dockerfile to simply remove a few lines after 427 |
This tempts admins into installing insecure packages to make the warning go away, even going so far as to [impurely modify running containers](nextcloud/docker#1414 (comment)). Changing the warning to trigger if the php-imagick module is loaded is more in line with the actual upstream recommendation, and hopefully helps unsuspecting users who have hacked around the warning in the past realize this. Draft because: - [ ] Untested (will try building this all this evening) - [ ] Translations are missing Band-aid for nextcloud#1414, while we wait for a proper solution with libvips or somesuch.
This tempts admins into installing insecure packages to make the warning go away, even going so far as to [impurely modify running containers](nextcloud/docker#1414 (comment)). Changing the warning to trigger if the php-imagick module is loaded is more in line with the actual upstream recommendation, and hopefully helps unsuspecting users who have hacked around the warning in the past realize this. Draft because: - [ ] Untested (will try building this all this evening) - [ ] Translations are missing, covered are: - [x] English - [x] German Band-aid for nextcloud#1414, while we wait for a proper solution with libvips or somesuch.
This tempts admins into installing insecure packages to make the warning go away, even going so far as to [impurely modify running containers](nextcloud/docker#1414 (comment)). Changing the warning to trigger if the php-imagick module is loaded is more in line with the actual upstream recommendation, and hopefully helps unsuspecting users who have hacked around the warning in the past realize this. Draft because: - [ ] Untested (will try building this all this evening) - [ ] Translations are missing, covered are: - [x] English - [x] German Band-aid for nextcloud#13099, while we wait for a proper solution with libvips or somesuch.
Refers to this lengthy discussion: nextcloud/docker#1414
|
Those running the |
|
Please read this comment: #1414 (comment) |
|
Oh, this is interesting. @pierreozoux' PR to upstream brought up this comment from @tcitworld: nextcloud/server#31742 (comment) Seems to me like the security concerns I had are actually unfounded, given imagemagick is in fact only used to handle built-in (and therefore non-malicious) svgs? The default In other words, this warning is about lacking imagemagick for the purposes of rendering some icons, which is much more reasonable than what was represented in this thread. If that's true, I'd much rather see imagemagick actually included in the image, after all. |
|
On docker-compose install based on nextcloud:latest (currently 24.0.1): When I tried to install the "extra" lib, I got: So it looks like it was delibarately blocked... And it adds some libs too: |
|
do you run |
Not at first, to understand why it was not there. With the message |
so without |
Which is strange, since there are two apt-get update in the Dockerfile, lines 7 and 27 |
and the info wiped out at line89 and 152 |
|
Ha, you're right. |
|
is this going to get added? i still see the warning with the newest version of 24.x |
|
I found this discussion while looking for a solution that allows the theming app to generate theme icons from SVG images. At first when I saw Nextcloud's warning in the overview page about php-imagick lacking SVG support, I just left it because no one really cares about preview images of SVG files. That feature isn't even enabled by default, so I'm not sure why Nextcloud's overview page needs to warn about it. Maybe that warning should only appear if someone goes through the effort of enabling that preview feature. The theming app does warn about the missing support as well, and there it's arguably more useful. So why is the official image still missing a complete imagemagick? On the concern of security, surely there's a big difference between the Docker image having the support installed, and Nextcloud being configured to actually use it? It's not enabled by default, so why is the Docker image still lacking imagemagick? This makes no sense. |
End-users-advocate: |
This in my opinion is exactly the problem. This is the official Nextcloud container, and it comes with a warning that says the recommend installing it for better suppport, but really they recommend NOT installing it due to a security issue, which is why it's not bundled with the container image. The real fix here is to remove the warning. I'm curious why this has not yet been done and over a year later the same warning is still present and nothing has changed? Warning should just be removed. :/ |
My understanding is that adding SVG support poses no security threat, because SVG previews are disabled by default. And having SVG support allows the theming module to generate rasters from SVG files. Therefore the warning should exist to inform users that without SVG support there is missing functionality. And the Docker image should install the SVG support because it poses no security threat and allows all of Nextcloud's functionality to work, which would also prevent the warning from displaying. |
I kinda agree here, the toggle can (and is) made on many levels that do not inherently block functionality or bother users by their defaults. But either or: Everyone is kinda in agreement that something should be done, so it's kinda weird nothing is being done. |
|
Ok, I'm in favor of adding svg support, then this discussion is over. Can anybody submit a PR to add svg on each flavor of the container? Thanks! |
and others
With the recent update to the vanilla library Nextcloud docker image on x86_64, i.e docker pull nextcloud, bumping the version of nextcloud to 21.0, some warnings appear,of which all but one were trivial to solve.
Whilst I know that the issue of svg compatibility and imagemagick has been raised several times, and at risk of this being marked as duplicate etc and closed , I feel now is the time to actually address it by either adding whatever is required to the image or removing this new warning.
And I understand that current advice is to modify a local Dockerfile to add the required packages etc, this isn't always an easy solution, and that (at least for now) there is a specific warning appearing about this, to suggest that people effectively create their own image locally seems a little silly.
The text was updated successfully, but these errors were encountered: