search

nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
26.91k stars 4.02k forks source link

Previews | libpng warning when png files are processed #27290

Open MichaIng opened 5 years ago

MichaIng commented 5 years ago

Steps to reproduce

  1. Upload some png file
  2. Run preview generator with -vvv to create previews for it.

Expected behaviour

Processing without warning.

Actual behaviour

libpng warning: Interlace handling should be turned on when using png_read_image

Server configuration

Operating system: Raspbian Buster

Web server: Apache2 2.4.38

Database: MariaDB 10.3.15

PHP version: PHP 7.3.4

Nextcloud version: 16.0.3

Updated from an older Nextcloud/ownCloud or fresh install: Updated several times

Where did you install Nextcloud from: https://download.nextcloud.com/server/releases/

Signing status:

Signing status ``` No errors have been found. ```

List of activated apps:

App list ``` Enabled: - activity: 2.9.1 - apporder: 0.7.1 - calendar: 1.7.0 - cloud_federation_api: 0.2.0 - contacts: 3.1.3 - dav: 1.9.2 - federatedfilesharing: 1.6.0 - files: 1.11.0 - files_rightclick: 0.13.0 - files_trashbin: 1.6.0 - files_versions: 1.9.0 - gallery: 18.3.0 - impersonate: 1.3.0 - logreader: 2.1.0 - lookup_server_connector: 1.4.0 - nextcloud_announcements: 1.5.0 - notes: 3.0.1 - notifications: 2.4.1 - oauth2: 1.4.2 - previewgenerator: 2.1.0 - provisioning_api: 1.6.0 - ransomware_protection: 1.4.0 - survey_client: 1.4.0 - tasks: 0.11.0 - twofactor_backupcodes: 1.5.0 - updatenotification: 1.6.0 - workflowengine: 1.6.0 ```

Nextcloud configuration:

Config report ``` { "system": { "passwordsalt": "***REMOVED SENSITIVE VALUE***", "secret": "***REMOVED SENSITIVE VALUE***", "trusted_domains": [ "localhost", "micha.gnoedi.org" ], "datadirectory": "***REMOVED SENSITIVE VALUE***", "dbtype": "mysql", "version": "16.0.3.0", "memcache.local": "\\OC\\Memcache\\APCu", "filelocking.enabled": true, "memcache.locking": "\\OC\\Memcache\\Redis", "redis": { "host": "***REMOVED SENSITIVE VALUE***", "port": 0 }, "dbname": "***REMOVED SENSITIVE VALUE***", "dbhost": "***REMOVED SENSITIVE VALUE***", "dbtableprefix": "oc_", "dbuser": "***REMOVED SENSITIVE VALUE***", "dbpassword": "***REMOVED SENSITIVE VALUE***", "installed": true, "instanceid": "***REMOVED SENSITIVE VALUE***", "loglevel": 1, "logtimezone": "Europe\/Berlin", "trashbin_retention_obligation": "disabled", "versions_retention_obligation": "disabled", "skeletondirectory": "", "defaultapp": "apporder", "maintenance": false, "overwrite.cli.url": "https:\/\/micha.gnoedi.org\/nextcloud", "htaccess.RewriteBase": "\/nextcloud", "mail_smtpmode": "smtp", "mail_smtpauthtype": "LOGIN", "mail_smtpsecure": "ssl", "mail_from_address": "***REMOVED SENSITIVE VALUE***", "mail_domain": "***REMOVED SENSITIVE VALUE***", "mail_smtpauth": 1, "mail_smtphost": "***REMOVED SENSITIVE VALUE***", "mail_smtpport": "465", "mail_smtpname": "***REMOVED SENSITIVE VALUE***", "mail_smtppassword": "***REMOVED SENSITIVE VALUE***", "theme": "", "mysql.utf8mb4": true, "updater.release.channel": "beta", "tempdirectory": "\/mnt\/sda\/ncdata\/tmp", "preview_max_x": "2048", "preview_max_y": "2048", "jpeg_quality": "60" } } ```

Are you using external storage, if yes which one: no

Are you using encryption: no

Are you using an external user-backend, if yes which one: no

Nextcloud log (data/nextcloud.log)

Nextcloud log nothing related

Additional info

ii  libpng16-16:armhf             1.6.37-1                    armhf        PNG library - runtime (version 1.6)
kesselb commented 5 years ago

Hmm. I'm not sure if we can fix this. This looks like a problem how php-gd calls libpng.

MichaIng commented 5 years ago

@kesselb Hmm the threads you linked are quite old and refer to a bug that was fixed with libpng 1.5.1 already.

I upgraded to PHP 7.3.8 meanwhile, but damn I didn't save/remember the exact files where this issue occurred. Tried some other .png file where previews were created well, I guess it is depends on the png format. Will try to check all pngs I have, those are not too many, hopefully.

Okay, so it is indeed a warning, no error. I went through all PNG files and preview have all been properly produced and no output in nextcloud.log when producing new ones. Was possible since I configured pre-generator to not generate the preview size that is requested when opening file details of an image in files app.

Since previews do now exist and and those are saved in files cache index directories (instead of file names), I am a bid stuck now about how to reproduce the warning with preview generator. From the links I think creating/uploading a PNG-24 image would be a start, but not sure how to convert/create one with usual tools 😉.

kesselb commented 3 years ago

@MichaIng you found the png file causing this error?

MichaIng commented 3 years ago

Found them. These images trigger the error:

I cannot see what makes them special currently. Windows lists them as 32-bit PNGs like all others. GIMP as well does not show any interesting differences 🤔.

wotography commented 3 years ago

I am having the same warning from my cron job. It does not always occur though. As I am not a professional its pretty difficult for me getting details information. Please help me get the right information to fix this issue.

Full waring message: usr/local/php72/bin/php -f 'path/nextcloud.tld/cron.php' > /dev/null libpng warning: Interlace handling should be turned on when using png_read_image

My first guess is, its caused by the not supported app "social". I disabled it for the moment. I will monitor this and give an update after a while.

MichaIng commented 3 years ago

It's definitely some PNG types/encodings that trigger it. In case of the "social" app it could be when certain images are sent or viewed.

I'll try to assure that it is still an issue with latest PHP and in case report to https://sourceforge.net/p/libpng/bugs/.

szaimen commented 3 years ago

I suppose this is still happening on NC21.0.4?

MichaIng commented 3 years ago

And 22.1.0 RC1, yes.

clinkadink commented 3 years ago

Same happens for me, when using the Preview Generator to generate thumbnails for my library. I am on release 22.1.1.

solracsf commented 1 year ago

Still happening on PHP8.1 / NC 25.0.7

Seems related to library, not NC/PHP code. https://mixable.blog/png-deactivate-interlace-to-avoid-libpng-warning-interlace-handling-should-be-turned-on-when-using-png_read_image/

stweil commented 1 year ago

This still occurs with Nextcloud 26.0.3. I observe these warnings related to libpng:

2023-06-26 more warning:

MichaIng commented 1 year ago

Some of these may be indeed due to bad PNG format or corrupted files, but the interlace handling warning may be a bug, according to the report at libpng SourceForge page. For this which are bothered/annoyed by this, it could make sense to contribute to that old bug report with example files.

Nextcloud likely could easily mute all libpng warnings. But not sure whether this is wanted, given that the resulting image may be broken or have errors, in which case the warning may serve valuable debug information. But probably they could be demoted to a lower log level?

stweil commented 1 year ago

Note that I don't see warnings in the Nextcloud log. But obviously the cron jobs print those warnings, so cron sends e-mails to all administrators.

MichaIng commented 1 year ago

Ah right, too long ago that I faced and reported this. However, I guess the same errors would end up in the Nextcloud log if the preview would be triggered via web UI instead of CLI. I guess one would need to catch the output of the php-gd function calls when loading/dealing with PNG images.

paramazo commented 3 months ago

Same here after upgrading from 27.x to 29.0.2. This is a result of php -f /var/www/nextcloud/cron.php

MichaIng commented 3 months ago

I checked for some possibly related reports at libpng:

PNGs can be stored interlaced, so that that they are not loaded line by line from top to bottom, but jumping over multiple lines in multiple loops. So you see the full sized picture earlier, just with "gaps" (not sharp), becoming sharper until loading finished. This was relevant on slow network connections in the past, today probably for very large PNGs on websites only.

The manual says this about the function: http://www.libpng.org/pub/png/libpng-manual.txt

Reading image data

After you've allocated memory, you can read the image data. The simplest way to do this is in one function call. If you are allocating enough memory to hold the whole image, you can just call png_read_image() and libpng will read in all the image data and put it in the memory area supplied. You will need to pass in an array of pointers to each row.

This function automatically handles interlacing, so you don't need to call png_set_interlace_handling() (unless you call png_read_update_info()) or call this function multiple times, or any of that other stuff necessary with png_read_rows().

png_read_image(png_ptr, row_pointers);

where row_pointers is:

png_bytep row_pointers[height];

You can point to void or char or whatever you use for pixels.

So it does interlace handling automatically, but there is no hint that it requires the image to be saved with interlacing, or that it would need to be called differently then. Also workaround posted above indicates that it does not throw the warning, when images are stored without interlacing explicitly. However, probably the images have some contradicting meta data or otherwise lead to libpng or php-gd detecting interlacing incorrectly, or the row pointer array passed by php-gd is passed incorrectly.

Having a look into the source code of the funtion:

/* Read the entire image.  If the image has an alpha channel or a tRNS
 * chunk, and you have called png_handle_alpha()[*], you will need to
 * initialize the image to the current image that PNG will be overlaying.
 * We set the num_rows again here, in case it was incorrectly set in
 * png_read_start_row() by a call to png_read_update_info() or
 * png_start_read_image() if png_set_interlace_handling() wasn't called
 * prior to either of these functions like it should have been.  You can
 * only call this function once.  If you desire to have an image for
 * each pass of a interlaced image, use png_read_rows() instead.
 *
 * [*] png_handle_alpha() does not exist yet, as of this version of libpng
 */
void PNGAPI
png_read_image(png_structrp png_ptr, png_bytepp image)
{
   png_uint_32 i, image_height;
   int pass, j;
   png_bytepp rp;

   png_debug(1, "in png_read_image");

   if (png_ptr == NULL)
      return;

#ifdef PNG_READ_INTERLACING_SUPPORTED
   if ((png_ptr->flags & PNG_FLAG_ROW_INIT) == 0)
   {
      pass = png_set_interlace_handling(png_ptr);
      /* And make sure transforms are initialized. */
      png_start_read_image(png_ptr);
   }
   else
   {
      if (png_ptr->interlaced != 0 &&
          (png_ptr->transformations & PNG_INTERLACE) == 0)
      {
         /* Caller called png_start_read_image or png_read_update_info without
          * first turning on the PNG_INTERLACE transform.  We can fix this here,
          * but the caller should do it!
          */
         png_warning(png_ptr, "Interlace handling should be turned on when "
             "using png_read_image");
         /* Make sure this is set correctly */
         png_ptr->num_rows = png_ptr->height;
      }

      /* Obtain the pass number, which also turns on the PNG_INTERLACE flag in
       * the above error case.
       */
      pass = png_set_interlace_handling(png_ptr);
   }

According to comment and code, the warning is thrown when not only png_read_image() is called, but when other functions were called first, without setting interlace handling explicitly, while the image is actually interlaced. The PNG_FLAG_ROW_INIT flag seems to be set, when the image/info is read, in which case interlace handling should have been set already, prior to those calls. This matches the manual, which says that interlacing is handled automatically "unless you call png_read_update_info()".

Probably php-gd calls png_start_read_image() or png_read_update_info() first, without setting the PNG_INTERLACE flag/option, respectively calling png_set_interlace_handling() first (which sets PNG_INTERLACE based on image data).

Now checking php-gd => libgd source code, this very thing has been fixed already in 2022: https://github.com/libgd/libgd/pull/823

Checking releases, the last v2.3.3 was in 2021. So we need to wait for v2.3.4 to be released, and in case distros to ship libgd/php-gd packages based on this/compiled against this version. On point release distros, this can take some years.

@kesselb @szaimen do you think it makes sense to mute this very particular warning, given that we now know exactly where it is coming from, that it is gracefully handled by libpng internally, and that users/admins are neither responsible (image files, setup), nor can do anything about it? We can add a comment to the code, stating that this special handling/mute can be removed, once libgd 2.3.4 or newer has been released and widely adopted among distros. Motivation is that admins do not worry or waste time to check these log entries unnecessarily, and allowing them to focus on actually relevant log entries, when investigating issues.

EDIT: It seems to have been backported into php-gd from PHP 8.0.17 already: https://github.com/php/php-src/commit/1d48da6 But I guess this is only effective when building php-gd with/against their internal libgd sources, while e.g. on Debian, it is compiled against dedicated libgd2 sources, which are surely not based on these PHP-internal ones, but upstream. So likely not relevant, unless one compiles some sort of standalone PHP.

TheRealTripleB commented 3 weeks ago

I came to this thread pretty quickly after I got this error after updating the PHP on my Debian host to 8.1 and then updating the Nextcloud from 25 to 27. Now the host is running under debian12 with PHP 8.2, Nextcloud 29 and I still get the message, so the fix doesn't seem to have arrived in debian yet.

MichaIng commented 3 weeks ago

First of all there needs to be a new libgd upstream release: https://github.com/libgd/libgd/releases

Once it is there, you can see the libgd version for Debian Sid/unstable here: https://packages.debian.org/sid/libgd3 Once that once raised as well, the next Debian release will likely be shipped with that version, at least if it does not happen within feature freeze stage the months before a new Debian release.

So it will take some years before it landed in a stable Debian version. of in case of upstream release if Debian backports this particular fix as patch into their older versions.

Best we/Nextcloud can do is muting this particular warning, and admins can just ignore it.